Fix error during external outputs checkout (iterative#4570)

* Fix error during external outputs checkout User in discord complained that external outputs' checkout was failing. Looks like, during refactor of `HashInfo`, this was forgotten, and as no tests existed before, we had a regression from 1.6.5 onwards. * fix tests * Restyled by black (iterative#4571) Co-authored-by: Restyled.io <[email protected]> Co-authored-by: restyled-io[bot] <32688539+restyled-io[bot]@users.noreply.github.com> Co-authored-by: Restyled.io <[email protected]>
vladkol · Sep 15, 2020 · 01abf7c · 01abf7c
1 parent 4ebacf1
commit 01abf7c
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 6 deletions.
diff --git a/dvc/cache/base.py b/dvc/cache/base.py
@@ -414,7 +414,7 @@ def changed_cache(self, hash_info, path_info=None, filter_info=None):
         return self.changed_cache_file(hash_info)
 
     def already_cached(self, path_info):
-        _, current = self.tree.get_hash(path_info)
+        current = self.tree.get_hash(path_info)
 
         if not current:
             return False

diff --git a/dvc/cache/local.py b/dvc/cache/local.py
@@ -94,11 +94,7 @@ def hashes_exist(
     def already_cached(self, path_info):
         assert path_info.scheme in ["", "local"]
 
-        current = self.tree.get_hash(path_info)
-        if not current:
-            return False
-
-        return not self.changed_cache(current)
+        return super().already_cached(path_info)
 
     def _verify_link(self, path_info, link_type):
         if link_type == "hardlink" and self.tree.getsize(path_info) == 0:

diff --git a/tests/func/test_checkout.py b/tests/func/test_checkout.py
@@ -850,3 +850,22 @@ def test_checkouts_for_pipeline_tracked_outs(tmp_dir, dvc, scm, run_copy):
 
     (tmp_dir / "ipsum").unlink()
     assert set(dvc.checkout()["added"]) == {"bar", "ipsum"}
+
+
+@pytest.mark.parametrize(
+    "workspace", [pytest.lazy_fixture("s3")], indirect=True
+)
+def test_checkout_external_modified_file(tmp_dir, dvc, scm, mocker, workspace):
+    # regression: happened when file in external output changed and checkout
+    # was attempted without force, dvc checks if it's present in its cache
+    # before asking user to remove it.
+    workspace.gen("foo", "foo")
+    dvc.add(str(workspace / "foo"), external=True)
+    scm.add(["foo.dvc"])
+    scm.commit("add foo")
+
+    workspace.gen("foo", "foobar")  # messing up the external outputs
+    mocker.patch("dvc.prompt.confirm", return_value=True)
+    dvc.checkout()
+
+    assert (workspace / "foo").read_text() == "foo"