config: read merged config by default (#5179)

* config: read merged config by default

* tests: move root test out of config tests

* tests: refactor config tests

* cli: fix cache dir command

dvc/command/cache.py

@@ -11,10 +11,21 @@
 class CmdCacheDir(CmdConfig):
     def run(self):
         if self.args.value is None and not self.args.unset:
-            logger.info(self.config["cache"]["dir"])
+            if self.args.level:
+                conf = self.config.read(level=self.args.level)
+            else:
+                # Use merged config with default values
+                conf = self.config
+            self._check(conf, False, "cache", "dir")
+            logger.info(conf["cache"]["dir"])
             return 0
-        with self.config.edit(level=self.args.level) as edit:
-            edit["cache"]["dir"] = self.args.value
+        with self.config.edit(level=self.args.level) as conf:
+            if self.args.unset:
+                self._check(conf, False, "cache", "dir")
+                del conf["cache"]["dir"]
+            else:
+                self._check(conf, False, "cache")
+                conf["cache"]["dir"] = self.args.value
         return 0
 
 

dvc/command/config.py

@@ -52,11 +52,10 @@ def run(self):
                 )
                 return 1
 
-            conf = self.config.load_one(self.args.level)
+            conf = self.config.read(self.args.level)
             prefix = self._config_file_prefix(
                 self.args.show_origin, self.config, self.args.level
             )
-
             logger.info("\n".join(self._format_config(conf, prefix)))
             return 0
 
@@ -67,11 +66,10 @@ def run(self):
         remote, section, opt = self.args.name
 
         if self.args.value is None and not self.args.unset:
-            conf = self.config.load_one(self.args.level)
+            conf = self.config.read(self.args.level)
             prefix = self._config_file_prefix(
                 self.args.show_origin, self.config, self.args.level
             )
-
             if remote:
                 conf = conf["remote"]
             self._check(conf, remote, section, opt)
@@ -117,6 +115,7 @@ def _config_file_prefix(show_origin, config, level):
         if not show_origin:
             return ""
 
+        level = level or "repo"
         fname = config.files[level]
 
         if level in ["local", "repo"]:
@@ -141,14 +140,21 @@ def _config_file_prefix(show_origin, config, level):
     const="system",
     help="Use system config.",
 )
+level_group.add_argument(
+    "--repo",
+    dest="level",
+    action="store_const",
+    const="repo",
+    help="Use repo config (.dvc/config).",
+)
 level_group.add_argument(
     "--local",
     dest="level",
     action="store_const",
     const="local",
-    help="Use local config.",
+    help="Use local config (.dvc/config.local).",
 )
-parent_config_parser.set_defaults(level="repo")
+parent_config_parser.set_defaults(level=None)
 
 
 def add_parser(subparsers, parent_parser):

dvc/command/remote.py

@@ -46,13 +46,14 @@ def run(self):
             self._check_exists(conf)
             del conf["remote"][self.args.name]
 
+        up_to_level = self.args.level or "repo"
         # Remove core.remote refs to this remote in any shadowing configs
         for level in reversed(self.config.LEVELS):
             with self.config.edit(level) as conf:
                 if conf["core"].get("remote") == self.args.name:
                     del conf["core"]["remote"]
 
-            if level == self.args.level:
+            if level == up_to_level:
                 break
 
         return 0
@@ -79,7 +80,7 @@ class CmdRemoteDefault(CmdRemote):
     def run(self):
 
         if self.args.name is None and not self.args.unset:
-            conf = self.config.load_one(self.args.level)
+            conf = self.config.read(self.args.level)
             try:
                 print(conf["core"]["remote"])
             except KeyError:
@@ -107,7 +108,7 @@ def run(self):
 
 class CmdRemoteList(CmdRemote):
     def run(self):
-        conf = self.config.load_one(self.args.level)
+        conf = self.config.read(self.args.level)
         for name, conf in conf["remote"].items():
             logger.info("{}\t{}".format(name, conf["url"]))
         return 0
@@ -133,8 +134,9 @@ def run(self):
             del conf["remote"][self.args.name]
             self._rename_default(conf)
 
+        up_to_level = self.args.level or "repo"
         for level in reversed(self.config.LEVELS):
-            if level == self.args.level:
+            if level == up_to_level:
                 break
             with self.config.edit(level) as level_conf:
                 self._rename_default(level_conf)

dvc/config.py

@@ -446,8 +446,16 @@ def load_config_to_level(self, level=None):
                 merge(merged_conf, self.load_one(merge_level))
         return merged_conf
 
+    def read(self, level=None):
+        # NOTE: we read from a merged config by default, same as git config
+        if level is None:
+            return self.load_config_to_level()
+        return self.load_one(level)
+
     @contextmanager
-    def edit(self, level="repo"):
+    def edit(self, level=None):
+        # NOTE: we write to repo config by default, same as git config
+        level = level or "repo"
         if level in {"repo", "local"} and self.dvc_dir is None:
             raise ConfigError("Not inside a DVC repo")
 

tests/func/test_cache.py

@@ -1,5 +1,6 @@
 import os
 import stat
+import textwrap
 
 import configobj
 import pytest
@@ -213,3 +214,27 @@ def test_shared_cache(tmp_dir, dvc, group, dir_mode):
         for fname in fnames:
             path = os.path.join(root, fname)
             assert stat.S_IMODE(os.stat(path).st_mode) == 0o444
+
+
+def test_cache_dir_local(tmp_dir, dvc, caplog):
+    (tmp_dir / ".dvc" / "config.local").write_text(
+        textwrap.dedent(
+            """\
+            [cache]
+                dir = some/path
+            """
+        )
+    )
+    path = os.path.join(dvc.dvc_dir, "some", "path")
+
+    caplog.clear()
+    assert main(["cache", "dir", "--local"]) == 0
+    assert path in caplog.text
+
+    caplog.clear()
+    assert main(["cache", "dir"]) == 0
+    assert path in caplog.text
+
+    caplog.clear()
+    assert main(["cache", "dir", "--repo"]) == 251
+    assert "option 'dir' doesn't exist in section 'cache'" in caplog.text