cfg: Eliminate the need for 'root' directory in configuration

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

conftest.py

@@ -53,9 +53,9 @@ def fixture_load_cfg():
     root_dir = pathlib.Path(__file__).resolve().parent
     test_data_dir = root_dir / 'plinth' / 'tests' / 'data'
     cfg_file = test_data_dir / 'etc' / 'plinth' / 'plinth.config'
-    cfg.read_file(str(cfg_file), str(root_dir))
+    cfg.read_file(str(cfg_file))
     yield cfg
-    cfg.read_file(str(cfg_file), str(root_dir))
+    cfg.read_file(str(cfg_file))
 
 
 @pytest.fixture(name='develop_mode')

plinth.config

@@ -1,6 +1,6 @@
 [Path]
 # directory locations
-file_root = %(root)s
+file_root = %(parent_dir)s
 config_dir = %(file_root)s/data/etc/plinth
 data_dir = %(file_root)s/data/var/lib/plinth
 server_dir = /plinth

plinth/__main__.py

@@ -124,9 +124,8 @@ def main():
 
     cfg.read()
     if arguments.develop:
-        # use the root and plinth.config of the current working directory
+        # Use the config in the current working directory
-        config_path, root_directory = cfg.get_develop_config_paths()
+        cfg.read_file(cfg.get_develop_config_path())
-        cfg.read_file(config_path, root_directory)
 
     adapt_config(arguments)
 

plinth/actions.py

@@ -169,7 +169,7 @@ def _run(action, options=None, input=None, run_in_background=False,
     if cfg.develop and sudo_call:
         # Passing 'env' does not work with sudo, so append the PYTHONPATH
         # as part of the command
-        sudo_call += ['PYTHONPATH=%s' % cfg.root]
+        sudo_call += ['PYTHONPATH=%s' % cfg.file_root]
 
     if sudo_call:
         cmd = sudo_call + cmd
@@ -186,7 +186,7 @@ def _run(action, options=None, input=None, run_in_background=False,
     }
     if cfg.develop:
         # In development mode pass on local pythonpath to access Plinth
-        kwargs['env'] = {'PYTHONPATH': cfg.root}
+        kwargs['env'] = {'PYTHONPATH': cfg.file_root}
 
     proc = subprocess.Popen(cmd, **kwargs)
 

plinth/cfg.py

@@ -6,11 +6,11 @@ Configuration parser and default values for configuration options.
 import configparser
 import logging
 import os
+import pathlib
 
 logger = logging.getLogger(__name__)
 
 # [Path] section
-root = None
 file_root = '/usr/share/plinth'
 config_dir = '/etc/plinth'
 data_dir = '/var/lib/plinth'
@@ -50,33 +50,28 @@ develop = False
 config_files = []
 
 
-def get_develop_config_paths():
+def get_develop_config_path():
-    """Return config paths of current source folder for development mode."""
+    """Return config path of current source folder for development mode."""
     root_directory = os.path.dirname(os.path.realpath(__file__))
     root_directory = os.path.join(root_directory, '..')
     root_directory = os.path.realpath(root_directory)
     config_path = os.path.join(root_directory, 'plinth.config')
-    return config_path, root_directory
+    return config_path
 
 
-def get_config_paths():
+def get_config_path():
     """Get default config paths."""
-    return '/etc/plinth/plinth.config', '/'
+    return '/etc/plinth/plinth.config'
 
 
 def read():
     """Read all configuration files."""
-    config_path, root_directory = get_config_paths()
+    config_path = get_config_path()
-    read_file(config_path, root_directory)
+    read_file(config_path)
 
 
-def read_file(config_path, root_directory):
+def read_file(config_path):
-    """Read and merge into defaults a single configuration file.
+    """Read and merge into defaults a single configuration file."""
-
-    - config_path: path of plinth.config file
-    - root_directory: path of plinth root folder
-
-    """
     if not os.path.isfile(config_path):
         # Ignore missing configuration files
         return
@@ -85,12 +80,11 @@ def read_file(config_path, root_directory):
     config_files.append(config_path)
 
     parser = configparser.ConfigParser(defaults={
-        'root': os.path.realpath(root_directory),
+        'parent_dir': pathlib.Path(config_path).parent.resolve(),
     })
     parser.read(config_path)
 
     config_items = (
-        ('Path', 'root', 'string'),
         ('Path', 'file_root', 'string'),
         ('Path', 'config_dir', 'string'),
         ('Path', 'data_dir', 'string'),

plinth/tests/data/etc/plinth/plinth.config

@@ -1,6 +1,6 @@
 [Path]
 # directory locations
-file_root = %(root)s
+file_root = %(parent_dir)s
 config_dir = %(file_root)s/data/etc/plinth
 data_dir = %(file_root)s/data/var/lib/plinth
 server_dir = /plinth

plinth/tests/test_actions.py

@@ -167,7 +167,7 @@ def test_action_path(monkeypatch):
     monkeypatch.setitem(os.environ, 'PYTHONPATH', '')
     plinth_path = run('test_path').strip()
     su_plinth_path = superuser_run('test_path').strip()
-    assert plinth_path.startswith(cfg.root)
+    assert plinth_path.startswith(cfg.file_root)
     assert plinth_path == su_plinth_path
 
 

plinth/tests/test_cfg.py

@@ -6,6 +6,7 @@ Test module for configuration module.
 import configparser
 import logging
 import os
+import pathlib
 from unittest.mock import patch
 
 import pytest
@@ -26,58 +27,54 @@ pytestmark = pytest.mark.usefixtures('load_cfg')
 
 def test_read_default_config_file():
     """Verify that the default config file can be read correctly."""
-    config_file, config_dir = cfg.get_develop_config_paths()
+    config_file = cfg.get_develop_config_path()
 
     # Read the plinth.config file directly
-    parser = configparser.ConfigParser(defaults={'root': config_dir})
+    parser = configparser.ConfigParser(
+        defaults={'parent_dir': pathlib.Path(config_file).parent})
     parser.read(config_file)
 
     # Read the plinth.config file via the cfg module
-    cfg.read_file(config_file, config_dir)
+    cfg.read_file(config_file)
 
     # Compare the two results
     compare_configurations(parser)
 
 
-@patch('plinth.cfg.get_config_paths')
+@patch('plinth.cfg.get_config_path')
-def test_read_primary_config_file(get_config_paths):
+def test_read_primary_config_file(get_config_path):
     """Verify that the primary config file is used by default."""
-    expected_config_path = CONFIG_FILE_WITH_MISSING_OPTIONS
+    config_path = CONFIG_FILE_WITH_MISSING_OPTIONS
-    root_directory = 'x-default-root'
+    get_config_path.return_value = config_path
-    expected_root_directory = os.path.realpath(root_directory)
-    get_config_paths.return_value = (expected_config_path, root_directory)
-
     cfg.read()
-
+    assert cfg.config_files[-1] == config_path
-    assert cfg.config_files[-1] == expected_config_path
-    assert cfg.root == expected_root_directory
 
 
-def test_read_fallback_config_file():
+def test_read_develop_config_file():
-    """Verify that the correct fallback config file is used"""
+    """Verify that the correct develop config file is used."""
     test_dir = os.path.dirname(os.path.realpath(__file__))
-    fallback_root = os.path.realpath(os.path.join(test_dir, '..', '..'))
+    develop_root = os.path.realpath(os.path.join(test_dir, '..', '..'))
-    fallback_config_file = os.path.join(fallback_root, 'plinth.config')
+    develop_config_file = os.path.join(develop_root, 'plinth.config')
-    config_path, root_directory = cfg.get_develop_config_paths()
+    config_path = cfg.get_develop_config_path()
-    cfg.read_file(config_path, root_directory)
+    cfg.read_file(config_path)
-    assert cfg.config_files[-1] == fallback_config_file
+    assert cfg.config_files[-1] == develop_config_file
-    assert cfg.root == fallback_root
+    assert cfg.file_root == develop_root
 
 
 def test_read_missing_config_file():
     """Verify that an exception is raised when there's no config file."""
-    cfg.read_file('x-non-existant-file', 'x-root-directory')
+    cfg.read_file('x-non-existant-file')
 
 
 def test_read_config_file_with_missing_sections():
     """Verify that missing configuration sections can be detected."""
-    cfg.read_file(CONFIG_FILE_WITH_MISSING_SECTIONS, TEST_CONFIG_DIR)
+    cfg.read_file(CONFIG_FILE_WITH_MISSING_SECTIONS)
     assert cfg.box_name == 'FreedomBoxTestMissingSections'
 
 
 def test_read_config_file_with_missing_options():
     """Verify that missing configuration options can be detected."""
-    cfg.read_file(CONFIG_FILE_WITH_MISSING_OPTIONS, TEST_CONFIG_DIR)
+    cfg.read_file(CONFIG_FILE_WITH_MISSING_OPTIONS)
     assert cfg.box_name == 'FreedomBoxTestMissingOptions'
 
 
@@ -86,7 +83,6 @@ def compare_configurations(parser):
     # Note that the count of items within each section includes the number
     # of default items (1, for 'root').
     assert len(parser.items('Path')) == 9
-    assert parser.get('Path', 'root') == cfg.root
     assert parser.get('Path', 'file_root') == cfg.file_root
     assert parser.get('Path', 'config_dir') == cfg.config_dir
     assert parser.get('Path', 'custom_static_dir') == cfg.custom_static_dir