diff --git a/src/tufup/repo/__init__.py b/src/tufup/repo/__init__.py index e8d0497..30b8890 100644 --- a/src/tufup/repo/__init__.py +++ b/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', ) diff --git a/src/tufup/utils/platform_specific.py b/src/tufup/utils/platform_specific.py index 0c9f537..3e5c588 100644 --- a/src/tufup/utils/platform_specific.py +++ b/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() diff --git a/tests/test_repo.py b/tests/test_repo.py index 7ec36df..4ed6170 100644 --- a/tests/test_repo.py +++ b/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