From 35bb261b8916062f7b3e05ccd8677a3810bd367a Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Wed, 29 Mar 2023 15:06:58 +0300 Subject: [PATCH] config: don't look for .dvc dir implicitly Currently if dvc_dir doesn't exists (e.g. we are creating an uninitialized repo), config will try looking for `.dvc` dir in the current directory, which might be inside of another dvc repository resulting in mixed configs. Use explicit `from_cwd` method to make `Config` try to find repo configs if available to avoid the usage mixup. --- dvc/analytics.py | 2 +- dvc/commands/config.py | 2 +- dvc/config.py | 20 ++++++++++++-------- dvc/parsing/interpolate.py | 2 +- dvc/repo/__init__.py | 9 ++++++--- dvc/updater.py | 2 +- tests/func/test_config.py | 4 ++-- tests/unit/test_config.py | 8 ++++---- 8 files changed, 28 insertions(+), 21 deletions(-) diff --git a/dvc/analytics.py b/dvc/analytics.py index 81d1699d1..6e3dc91b9 100644 --- a/dvc/analytics.py +++ b/dvc/analytics.py @@ -47,7 +47,7 @@ def is_enabled(): enabled = not os.getenv(DVC_NO_ANALYTICS) if enabled: enabled = to_bool( - Config(validate=False).get("core", {}).get("analytics", "true") + Config.from_cwd(validate=False).get("core", {}).get("analytics", "true") ) logger.debug("Analytics is %sabled.", "en" if enabled else "dis") diff --git a/dvc/commands/config.py b/dvc/commands/config.py index e1f710a2b..7117d1191 100644 --- a/dvc/commands/config.py +++ b/dvc/commands/config.py @@ -32,7 +32,7 @@ class CmdConfig(CmdBaseNoRepo): super().__init__(args) - self.config = Config(validate=False) + self.config = Config.from_cwd(validate=False) def run(self): if self.args.show_origin and (self.args.value or self.args.unset): diff --git a/dvc/config.py b/dvc/config.py index f79a6bd30..671179438 100644 --- a/dvc/config.py +++ b/dvc/config.py @@ -103,18 +103,22 @@ class Config(dict): self.wfs = LocalFileSystem() self.fs = fs or self.wfs - if not dvc_dir: - try: - from dvc.repo import Repo - - self.dvc_dir = Repo.find_dvc_dir() - except NotDvcRepoError: - self.dvc_dir = None - else: + if dvc_dir: self.dvc_dir = self.fs.path.abspath(self.fs.path.realpath(dvc_dir)) self.load(validate=validate, config=config) + @classmethod + def from_cwd(cls, fs: Optional["FileSystem"] = None, **kwargs): + from dvc.repo import Repo + + try: + dvc_dir = Repo.find_dvc_dir(fs=fs) + except NotDvcRepoError: + dvc_dir = None + + return cls(dvc_dir=dvc_dir, fs=fs, **kwargs) + @classmethod def get_dir(cls, level): from platformdirs import site_config_dir, user_config_dir diff --git a/dvc/parsing/interpolate.py b/dvc/parsing/interpolate.py index 5fd0d58c6..f175dfdc2 100644 --- a/dvc/parsing/interpolate.py +++ b/dvc/parsing/interpolate.py @@ -95,7 +95,7 @@ def _(obj: bool): def _(obj: dict): # noqa: C901 from dvc.config import Config - config = Config().get("parsing", {}) + config = Config.from_cwd().get("parsing", {}) result = "" for k, v in flatten(obj).items(): diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index f80d19f3a..da1e9cf8e 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -408,9 +408,12 @@ class Repo: raise NotDvcRepoError(msg) @classmethod - def find_dvc_dir(cls, root=None) -> str: - root_dir = cls.find_root(root) - return os.path.join(root_dir, cls.DVC_DIR) + def find_dvc_dir(cls, root=None, fs=None) -> str: + from dvc.fs import localfs + + fs = fs or localfs + root_dir = cls.find_root(root, fs=fs) + return fs.path.join(root_dir, cls.DVC_DIR) @staticmethod def init(root_dir=os.curdir, no_scm=False, force=False, subdir=False) -> "Repo": diff --git a/dvc/updater.py b/dvc/updater.py index 3b5f3fff9..fe59c2a2b 100644 --- a/dvc/updater.py +++ b/dvc/updater.py @@ -166,7 +166,7 @@ class Updater: from dvc.config import Config, to_bool enabled = to_bool( - Config(validate=False).get("core", {}).get("check_update", "true") + Config.from_cwd(validate=False).get("core", {}).get("check_update", "true") ) logger.debug("Check for update is %sabled.", "en" if enabled else "dis") return enabled diff --git a/tests/func/test_config.py b/tests/func/test_config.py index fec065661..5e6e297c9 100644 --- a/tests/func/test_config.py +++ b/tests/func/test_config.py @@ -245,7 +245,7 @@ def test_merging_two_levels(dvc): def test_config_loads_without_error_for_non_dvc_repo(tmp_dir): # regression testing for https://github.com/iterative/dvc/issues/3328 - Config(validate=True) + Config.from_cwd(validate=True) @pytest.mark.parametrize( @@ -284,7 +284,7 @@ def test_config_gdrive_fields(tmp_dir, dvc): "profile": "myprofile", } - Config(validate=True) + Config.from_cwd(validate=True) def test_config_remote(tmp_dir, dvc, capsys): diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index c81efa7fc..6f2ad6a7f 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -25,7 +25,7 @@ def test_get_fs(tmp_dir, scm): tmp_dir.scm_gen("foo", "foo", commit="add foo") fs = scm.get_fs("master") - config = Config(fs=fs) + config = Config.from_cwd(fs=fs) assert config.fs == fs assert config.wfs != fs @@ -38,7 +38,7 @@ def test_get_fs(tmp_dir, scm): def test_s3_ssl_verify(tmp_dir, dvc): - config = Config(validate=False) + config = Config.from_cwd(validate=False) with config.edit() as conf: conf["remote"]["remote-name"] = {"url": "s3://bucket/dvc"} @@ -74,7 +74,7 @@ def test_s3_ssl_verify(tmp_dir, dvc): def test_load_unicode_error(tmp_dir, dvc, mocker): - config = Config(validate=False) + config = Config.from_cwd(validate=False) mocker.patch( "configobj.ConfigObj", side_effect=UnicodeDecodeError("", b"", 0, 0, ""), @@ -87,7 +87,7 @@ def test_load_unicode_error(tmp_dir, dvc, mocker): def test_load_configob_error(tmp_dir, dvc, mocker): from configobj import ConfigObjError - config = Config(validate=False) + config = Config.from_cwd(validate=False) mocker.patch( "configobj.ConfigObj", side_effect=ConfigObjError(),