Repository: iterative/dvc

Issue:
brancher: dvcignore does not work as expected for subrepos when using brancher
As noted by @efiop in #3883, in CleanTree when we do
```py
    @cached_property
    def dvcignore(self):
        return DvcIgnoreFilter(self.tree)
```
`DvcIgnoreFilter` will use `WorkingTree.tree_root` or `GitTree.tree_root` as the root DVC repo directory.

For DVC subrepos, if the tree is a GitTree from brancher, this causes dvcignore to use the host/parent repo root `.dvcignore` rather than the subrepo root `.dvcignore`, since `GitTree.tree_root` will be the (parent/host level) git/scm root directory and not the DVC subrepo directory. For working trees this is not an issue, since the working tree root will be set to the correct `subrepo.root_dir`.

Some investigation should also be done to verify whether or not this issue (using host DVC repo root instead of subrepo root for brancher GitTrees) affects other places in DVC as well.

Discussion:

Patch:
diff --git a/dvc/command/plots.py b/dvc/command/plots.py
index b7900a697..ef50de6e7 100644
--- a/dvc/command/plots.py
+++ b/dvc/command/plots.py
@@ -33,6 +33,16 @@ class CmdPlots(CmdBase):
         raise NotImplementedError
 
     def run(self):
+        if self.args.show_vega:
+            if not self.args.targets:
+                logger.error("please specify a target for `--show-vega`")
+                return 1
+            if len(self.args.targets) > 1:
+                logger.error(
+                    "you can only specify one target for `--show-vega`"
+                )
+                return 1
+
         try:
             plots = self._func(
                 targets=self.args.targets,
@@ -45,10 +55,9 @@ class CmdPlots(CmdBase):
                 y_title=self.args.ylab,
             )
 
-            if self.args.show_json:
-                import json
-
-                logger.info(json.dumps(plots))
+            if self.args.show_vega:
+                target = self.args.targets[0]
+                logger.info(plots[target])
                 return 0
 
             divs = [
@@ -138,10 +147,10 @@ def add_parser(subparsers, parent_parser):
         help="Required when CSV or TSV datafile does not have a header.",
     )
     plots_show_parser.add_argument(
-        "--show-json",
+        "--show-vega",
         action="store_true",
         default=False,
-        help="Show output in JSON format.",
+        help="Show output in VEGA format.",
     )
     plots_show_parser.add_argument("--title", default=None, help="Plot title.")
     plots_show_parser.add_argument(
@@ -201,10 +210,10 @@ def add_parser(subparsers, parent_parser):
         help="Provided CSV ot TSV datafile does not have a header.",
     )
     plots_diff_parser.add_argument(
-        "--show-json",
+        "--show-vega",
         action="store_true",
         default=False,
-        help="Show output in JSON format.",
+        help="Show output in VEGA format.",
     )
     plots_diff_parser.add_argument("--title", default=None, help="Plot title.")
     plots_diff_parser.add_argument(
diff --git a/dvc/ignore.py b/dvc/ignore.py
index 2cc35a816..7ef67763b 100644
--- a/dvc/ignore.py
+++ b/dvc/ignore.py
@@ -92,13 +92,14 @@ class DvcIgnoreRepo(DvcIgnore):
 
 
 class DvcIgnoreFilter:
-    def __init__(self, tree):
+    def __init__(self, tree, root_dir):
         self.tree = tree
+        self.root_dir = root_dir
         self.ignores = {
             DvcIgnoreDirs([".git", ".hg", ".dvc"]),
             DvcIgnoreRepo(),
         }
-        for root, dirs, files in self.tree.walk(self.tree.tree_root):
+        for root, dirs, files in self.tree.walk(self.root_dir):
             self._update(root)
             dirs[:], files[:] = self(root, dirs, files)
 
@@ -124,7 +125,7 @@ class CleanTree(BaseTree):
 
     @cached_property
     def dvcignore(self):
-        return DvcIgnoreFilter(self.tree)
+        return DvcIgnoreFilter(self.tree, self.tree_root)
 
     @property
     def tree_root(self):
diff --git a/dvc/remote/gdrive.py b/dvc/remote/gdrive.py
index 947c37ffb..dfc7b1d5a 100644
--- a/dvc/remote/gdrive.py
+++ b/dvc/remote/gdrive.py
@@ -340,7 +340,7 @@ class GDriveRemote(BaseRemote):
                     "use the `gdrive_user_credentials_file` config\n"
                     "   option if you use multiple GDrive remotes with "
                     "different email accounts.\n\nDetails".format(
-                        item_id, self.path_info, self._credentials_location
+                        item_id, self.path_info, self.credentials_location
                     )
                 ) from exc
             raise


Test patch:
diff --git a/tests/func/test_ignore.py b/tests/func/test_ignore.py
index 65ad950c4..03806cb1e 100644
--- a/tests/func/test_ignore.py
+++ b/tests/func/test_ignore.py
@@ -11,6 +11,7 @@ from dvc.ignore import (
     DvcIgnoreRepo,
 )
 from dvc.remote import LocalRemote
+from dvc.repo import Repo
 from dvc.scm.tree import WorkingTree
 from dvc.utils import relpath
 from dvc.utils.fs import get_mtime_and_size
@@ -151,3 +152,20 @@ def test_ignore_external(tmp_dir, scm, dvc, tmp_path_factory):
     remote = LocalRemote(dvc, {})
     result = {relpath(f, ext_dir) for f in remote.walk_files(ext_dir)}
     assert result == {"y.backup", "tmp"}
+
+
+def test_ignore_subrepo(tmp_dir, scm, dvc):
+    tmp_dir.gen({".dvcignore": "foo", "subdir": {"foo": "foo"}})
+    scm.add([".dvcignore"])
+    scm.commit("init parent dvcignore")
+
+    subrepo_dir = tmp_dir / "subdir"
+    assert not dvc.tree.exists(subrepo_dir / "foo")
+
+    with subrepo_dir.chdir():
+        subrepo = Repo.init(subdir=True)
+        scm.add(str(subrepo_dir / "foo"))
+        scm.commit("subrepo init")
+
+    for _ in subrepo.brancher(all_commits=True):
+        assert subrepo.tree.exists(subrepo_dir / "foo")
diff --git a/tests/unit/command/test_plots.py b/tests/unit/command/test_plots.py
index 05b115a02..d1728f514 100644
--- a/tests/unit/command/test_plots.py
+++ b/tests/unit/command/test_plots.py
@@ -13,7 +13,7 @@ def test_metrics_diff(dvc, mocker):
             "template",
             "--targets",
             "datafile",
-            "--show-json",
+            "--show-vega",
             "-x",
             "x_field",
             "-y",
@@ -32,7 +32,9 @@ def test_metrics_diff(dvc, mocker):
     assert cli_args.func == CmdPlotsDiff
 
     cmd = cli_args.func(cli_args)
-    m = mocker.patch("dvc.repo.plots.diff.diff", return_value={})
+    m = mocker.patch(
+        "dvc.repo.plots.diff.diff", return_value={"datafile": "filledtemplate"}
+    )
 
     assert cmd.run() == 0
 
@@ -59,7 +61,7 @@ def test_metrics_show(dvc, mocker):
             "result.extension",
             "-t",
             "template",
-            "--show-json",
+            "--show-vega",
             "--no-csv-header",
             "datafile",
         ]
@@ -68,7 +70,9 @@ def test_metrics_show(dvc, mocker):
 
     cmd = cli_args.func(cli_args)
 
-    m = mocker.patch("dvc.repo.plots.show.show", return_value={})
+    m = mocker.patch(
+        "dvc.repo.plots.show.show", return_value={"datafile": "filledtemplate"}
+    )
 
     assert cmd.run() == 0
 
@@ -85,13 +89,21 @@ def test_metrics_show(dvc, mocker):
     )
 
 
-def test_plots_show_json(dvc, mocker, caplog):
+def test_plots_show_vega(dvc, mocker, caplog):
     cli_args = parse_args(
-        ["plots", "diff", "HEAD~10", "HEAD~1", "--show-json"]
+        [
+            "plots",
+            "diff",
+            "HEAD~10",
+            "HEAD~1",
+            "--show-vega",
+            "--targets",
+            "plots.csv",
+        ]
     )
     cmd = cli_args.func(cli_args)
     mocker.patch(
         "dvc.repo.plots.diff.diff", return_value={"plots.csv": "plothtml"}
     )
     assert cmd.run() == 0
-    assert '{"plots.csv": "plothtml"}\n' in caplog.text
+    assert "plothtml" in caplog.text