Fix issue 50: use relative paths in config file (#78)

* test Repository.from_config with both relative and absolute paths

* explicitly test path args for Repository.__init__

* extend test for Repository.save_config to ensure relative paths

* save relative paths to config file, if possible

Although using paths that are *not* relative to the current working directory (cwd) is discouraged,
this *should* still be possible. So, if the path is not relative to cwd, we just save the absolute path.

* try to fix github actions for windows/mac

see this issue: actions/runner-images#712

* handle 8.3 filenames in Repository.save_config()

* add _patched_resolve() as workaround for cpython #82852

for python <3.10 on windows

* ruff formatting

src/tufup/repo/__init__.py

@@ -38,6 +38,7 @@
 from tuf.api.serialization.json import JSONSerializer
 
 from tufup.common import Patcher, SUFFIX_ARCHIVE, SUFFIX_PATCH, TargetMeta
+from tufup.utils.platform_specific import _patched_resolve
 
 logger = logging.getLogger(__name__)
 
@@ -505,9 +506,9 @@ def __init__(
         thresholds: Optional[RolesDict] = None,
     ):
         if repo_dir is None:
-            repo_dir = pathlib.Path.cwd() / DEFAULT_REPO_DIR_NAME
+            repo_dir = DEFAULT_REPO_DIR_NAME
         if keys_dir is None:
-            keys_dir = pathlib.Path.cwd() / DEFAULT_KEYS_DIR_NAME
+            keys_dir = DEFAULT_KEYS_DIR_NAME
         if key_map is None:
             key_map = deepcopy(DEFAULT_KEY_MAP)
         if encrypted_keys is None:
@@ -519,8 +520,8 @@ def __init__(
         self.app_name = app_name
         self.app_version_attr = app_version_attr
         # force path object and resolve, in case of relative paths
-        self.repo_dir = pathlib.Path(repo_dir).resolve()
-        self.keys_dir = pathlib.Path(keys_dir).resolve()
+        self.repo_dir = _patched_resolve(pathlib.Path(repo_dir))
+        self.keys_dir = _patched_resolve(pathlib.Path(keys_dir))
         self.key_map = key_map
         self.encrypted_keys = encrypted_keys
         self.expiration_days = expiration_days
@@ -557,14 +558,34 @@ def app_version(self) -> str:
 
     @classmethod
     def get_config_file_path(cls) -> pathlib.Path:
+        # config must be stored in current working directory
         return pathlib.Path.cwd() / cls.config_filename
 
     def save_config(self):
         """Save current configuration."""
-        # todo: write directories relative to config file dir?
-        file_path = self.get_config_file_path()
-        file_path.write_text(
-            data=json.dumps(self.config_dict, default=str, sort_keys=True, indent=4),
+        config_file_path = self.get_config_file_path()
+        # make paths relative to current working directory (cwd),
+        # if possible, otherwise keep absolute paths (note, to avoid
+        # confusion, using paths other than cwd is discouraged)
+        temp_config_dict = self.config_dict  # note self.config_dict is a property
+        for key in ['repo_dir', 'keys_dir']:
+            try:
+                temp_config_dict[key] = temp_config_dict[key].relative_to(
+                    # resolve() is necessary on windows, to handle "short"
+                    # path components (a.k.a. "8.3 filename" or "8.3 alias"),
+                    # which are truncated with a tilde,
+                    # e.g. c:\Users\RUNNER~1\...
+                    pathlib.Path.cwd().resolve()
+                )
+            except ValueError:
+                logger.warning(
+                    f'Saving *absolute* path to config, because the path'
+                    f' ({temp_config_dict[key]}) is not relative to cwd'
+                    f' ({pathlib.Path.cwd()})'
+                )
+        # write file
+        config_file_path.write_text(
+            data=json.dumps(temp_config_dict, default=str, sort_keys=True, indent=4),
             encoding='utf-8',
         )
 

src/tufup/utils/platform_specific.py

@@ -243,3 +243,20 @@ def _install_update_mac(
     logger.debug(f'Restarting application, running {sys.executable}.')
     subprocess.Popen(sys.executable, shell=True)  # nosec
     sys.exit(0)
+
+
+def _patched_resolve(path: pathlib.Path):
+    """
+    this is a rather crude workaround for cpython issue #82852,
+    where Path.resolve() yields a relative path, on windows, if the target
+    does not exist yet
+
+    https://github.com/python/cpython/issues/82852
+
+    todo: remove this as soon as support for python 3.9 is dropped
+    """
+    if ON_WINDOWS and sys.version_info[:2] < (3, 10):
+        logger.warning('using patched path for cpython #82852')
+        if not path.is_absolute():
+            path = pathlib.Path.cwd() / path
+    return path.resolve()

tests/test_repo.py

@@ -472,6 +472,29 @@ def setUpClass(cls) -> None:
     def test_defaults(self):
         self.assertTrue(Repository(app_name='test'))
 
+    def test_init_paths(self):
+        repo_dir_name = 'repo'
+        keys_dir_name = 'keystore'
+        # absolute paths (could also use resolve on relative path...)
+        repo_dir_abs = self.temp_dir_path / repo_dir_name
+        keys_dir_abs = self.temp_dir_path / keys_dir_name
+        cases = [
+            ('string', repo_dir_name, keys_dir_name),
+            ('relative', pathlib.Path(repo_dir_name), pathlib.Path(keys_dir_name)),
+            ('absolute', repo_dir_abs, keys_dir_abs),
+        ]
+        for message, repo_dir, keys_dir in cases:
+            with self.subTest(msg=message):
+                repo = Repository(app_name='test', repo_dir=repo_dir, keys_dir=keys_dir)
+                # internally we should always have the absolute paths
+                self.assertTrue(repo.repo_dir.is_absolute())
+                self.assertTrue(repo.keys_dir.is_absolute())
+                # compare dirs
+                # resolve is necessary for github actions, see:
+                # https://github.com/actions/runner-images/issues/712#issuecomment-1163036706
+                self.assertEqual(repo_dir_abs.resolve(), repo.repo_dir.resolve())
+                self.assertEqual(keys_dir_abs.resolve(), repo.keys_dir.resolve())
+
     def test_config_dict(self):
         app_name = 'test'
         repo = Repository(app_name=app_name)
@@ -504,7 +527,15 @@ def test_save_config(self):
         # test
         repo.save_config()
         self.assertTrue(repo.get_config_file_path().exists())
-        print(repo.get_config_file_path().read_text())
+        config_file_text = repo.get_config_file_path().read_text()
+        print(config_file_text)  # for convenience
+        # paths saved to config file are relative to current working
+        # directory (cwd) if possible (otherwise absolute paths are saved)
+        config_dict = json.loads(config_file_text)
+        for key in ['repo_dir', 'keys_dir']:
+            with self.subTest(msg=key):
+                # note Path.is_relative_to() is introduced in python 3.9
+                self.assertFalse(pathlib.Path(config_dict[key]).is_absolute())
 
     def test_load_config(self):
         # file does not exist
@@ -516,33 +547,44 @@ def test_load_config(self):
 
     def test_from_config(self):
         temp_dir = self.temp_dir_path.resolve()
-        # prepare
-        config_data = dict(
-            app_name='test',
-            app_version_attr='my_app.__version__',
-            repo_dir=temp_dir / 'repo',
-            keys_dir=temp_dir / 'keystore',
-            key_map=dict(),
-            encrypted_keys=[],
-            expiration_days=dict(),
-            thresholds=dict(),
-        )
-        Repository.get_config_file_path().write_text(
-            json.dumps(config_data, default=str)
-        )
-        mock_load_keys_and_roles = Mock()
-        # test
-        with patch.object(
-            Repository,
-            '_load_keys_and_roles',
-            mock_load_keys_and_roles,
-        ):
-            repo = Repository.from_config()
-        self.assertEqual(
-            config_data,
-            {item: getattr(repo, item) for item in config_data.keys()},
-        )
-        self.assertTrue(mock_load_keys_and_roles.called)
+        repo_dir_abs = temp_dir / 'repo'
+        keys_dir_abs = temp_dir / 'keystore'
+        cases = [
+            ('absolute paths', repo_dir_abs, keys_dir_abs),
+            (
+                'relative paths',
+                repo_dir_abs.relative_to(temp_dir),
+                keys_dir_abs.relative_to(temp_dir),
+            ),
+        ]
+        for message, repo_dir, keys_dir in cases:
+            with self.subTest(msg=message):
+                # prepare
+                config_data = dict(
+                    app_name='test',
+                    app_version_attr='my_app.__version__',
+                    repo_dir=repo_dir,
+                    keys_dir=keys_dir,
+                    key_map=dict(),
+                    encrypted_keys=[],
+                    expiration_days=dict(),
+                    thresholds=dict(),
+                )
+                Repository.get_config_file_path().write_text(
+                    json.dumps(config_data, default=str)
+                )
+                # test
+                with patch.object(Repository, '_load_keys_and_roles') as mmock_load:
+                    repo = Repository.from_config()
+                # internally the repo should always work with absolute paths
+                # (relative paths are resolved in the class initializer)
+                config_data['repo_dir'] = repo_dir_abs
+                config_data['keys_dir'] = keys_dir_abs
+                self.assertEqual(
+                    config_data,
+                    {key: getattr(repo, key) for key in config_data.keys()},
+                )
+                self.assertTrue(mmock_load.called)
 
     def test_initialize(self):
         # prepare