From 4f61a63afdae5783dfed88f3f43e15f0b9246e58 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 10 Aug 2023 00:16:46 +0300 Subject: [PATCH] config: resolve: handle ~ home dir Fixes #9822 --- dvc/config.py | 39 +++++++++++++++++++++++++++------------ dvc/config_schema.py | 11 +++++++++++ tests/func/test_config.py | 8 +++++--- tests/unit/test_config.py | 23 +++++++++++++++++++++++ 4 files changed, 66 insertions(+), 15 deletions(-) diff --git a/dvc/config.py b/dvc/config.py index 77f086c14..cee34420e 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -250,21 +250,30 @@ class Config(dict): return conf @staticmethod - def _load_paths(conf, filename): - abs_conf_dir = os.path.abspath(os.path.dirname(filename)) + def _resolve(conf_dir, path): + from .config_schema import ExpPath, RelPath - def resolve(path): - from .config_schema import RelPath + if re.match(r"\w+://", path): + return path - if os.path.isabs(path) or re.match(r"\w+://", path): - return path + if os.path.isabs(path): + return path - # on windows convert slashes to backslashes - # to have path compatible with abs_conf_dir - if os.path.sep == "\\" and "/" in path: - path = path.replace("/", "\\") + # on windows convert slashes to backslashes + # to have path compatible with abs_conf_dir + if os.path.sep == "\\" and "/" in path: + path = path.replace("/", "\\") - return RelPath(os.path.join(abs_conf_dir, path)) + expanded = os.path.expanduser(path) + if os.path.isabs(expanded): + return ExpPath(expanded, path) + + return RelPath(os.path.abspath(os.path.join(conf_dir, path))) + + @classmethod + def _load_paths(cls, conf, filename): + conf_dir = os.path.abspath(os.path.dirname(filename)) + resolve = partial(cls._resolve, conf_dir) return Config._map_dirs(conf, resolve) @@ -273,11 +282,17 @@ class Config(dict): from dvc.fs import localfs from dvc.utils import relpath - from .config_schema import RelPath + from .config_schema import ExpPath, RelPath if re.match(r"\w+://", path): return path + if isinstance(path, ExpPath): + return path.def_path + + if os.path.expanduser(path) != path: + return localfs.path.as_posix(path) + if isinstance(path, RelPath) or not os.path.isabs(path): path = relpath(path, conf_dir) return localfs.path.as_posix(path) diff --git a/dvc/config_schema.py b/dvc/config_schema.py index 38610a502..ede8afa2c 100644 --- a/dvc/config_schema.py +++ b/dvc/config_schema.py @@ -75,6 +75,17 @@ def ByUrl(mapping): # noqa: N802 return validate +class ExpPath(str): + __slots__ = ("def_path",) + + def_path: str + + def __new__(cls, string, def_path): + ret = super().__new__(cls, string) + ret.def_path = def_path + return ret + + class RelPath(str): __slots__ = () diff --git a/tests/func/test_config.py b/tests/func/test_config.py index 9829b3ad1..ac812836d 100644 --- a/tests/func/test_config.py +++ b/tests/func/test_config.py @@ -265,14 +265,16 @@ def test_load_relative_paths(dvc, field, remote_url): # check if written paths are correct dvc_dir = dvc.config.dvc_dir - assert dvc.config["remote"]["test"][field] == os.path.join( - dvc_dir, "..", "file.txt" + assert dvc.config["remote"]["test"][field] == os.path.abspath( + os.path.join(dvc_dir, "..", "file.txt") ) # load config and check that it contains what we expect # (relative paths are evaluated correctly) cfg = Config(dvc_dir) - assert cfg["remote"]["test"][field] == os.path.join(dvc_dir, "..", "file.txt") + assert cfg["remote"]["test"][field] == os.path.abspath( + os.path.join(dvc_dir, "..", "file.txt") + ) def test_config_gdrive_fields(tmp_dir, dvc): diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 6f2ad6a7f..9c7f2dc9d 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -21,6 +21,29 @@ def test_to_relpath(path, expected): assert Config._to_relpath(os.path.join(".", "config"), path) == expected +@pytest.mark.parametrize( + "path, expected", + [ + ("cache", os.path.abspath(os.path.join("conf_dir", "cache"))), + ("dir/cache", os.path.abspath(os.path.join("conf_dir", "dir", "cache"))), + ("../cache", os.path.abspath("cache")), + (os.getcwd(), os.getcwd()), + ("ssh://some/path", "ssh://some/path"), + ], +) +def test_resolve(path, expected): + conf_dir = os.path.abspath("conf_dir") + assert Config._resolve(conf_dir, path) == expected + + +def test_resolve_homedir(): + # NOTE: our test suit patches $HOME, but that only works within the + # test itself, so we can't use expanduser in @parametrize here. + conf_dir = os.path.abspath("conf_dir") + expected = os.path.expanduser(os.path.join("~", "cache")) + assert Config._resolve(conf_dir, "~/cache") == expected + + def test_get_fs(tmp_dir, scm): tmp_dir.scm_gen("foo", "foo", commit="add foo")