From 6c1d00c2d955bbf4554bf36034f77edfb4338f38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Saugat=20Pachhai=20=28=E0=A4=B8=E0=A5=8C=E0=A4=97=E0=A4=BE?= =?UTF-8?q?=E0=A4=A4=29?= Date: Tue, 23 Aug 2022 14:14:24 +0545 Subject: [PATCH] fix diff when no object exists by falling back to shallow_diff --- dvc/repo/data.py | 108 ++++++++++++------------- tests/func/test_data_status.py | 144 ++++++++++++++++++++++----------- 2 files changed, 149 insertions(+), 103 deletions(-) diff --git a/dvc/repo/data.py b/dvc/repo/data.py index 25e074fde..e7807bc00 100644 --- a/dvc/repo/data.py +++ b/dvc/repo/data.py @@ -44,20 +44,13 @@ def _shallow_diff( if not old and not new: return d - if not new: return {"deleted": [old_root], **d} if not old: return {"added": [new_root], **d} - if old.isdir != new.isdir: - return {"deleted": [old_root], "added": [new_root], **d} - - assert old_root == new_root - root = old_root # the root are the same if old != new: - assert old.isdir == new.isdir - return {"modified": [root], **d} - return {"unchanged": [root], **d} + return {"modified": [new_root], **d} + return {"unchanged": [new_root], **d} def _granular_diff( @@ -69,53 +62,57 @@ def _granular_diff( ) -> Dict[str, List[str]]: from dvc_data.diff import ROOT from dvc_data.diff import diff as odiff - from dvc_data.objects.tree import Tree - drop_root = False - trees = isinstance(old_obj, Tree) or isinstance(new_obj, Tree) - if trees: - drop_root = not with_dirs - - def path_join(root: str, *paths: str) -> str: - if not trees and paths == ROOT: + def path_join(root: str, *paths: str, isdir: bool = False) -> str: + if not isdir and paths == ROOT: return root return os.path.sep.join([root, *paths]) + def type_changed(old, new): + if old.oid and new.oid: + return old.oid.isdir != new.oid.isdir + return False + diff_data = odiff(old_obj, new_obj, cache) output: Dict[str, List[str]] = defaultdict(list) for state in ("added", "deleted", "modified", "unchanged"): items = getattr(diff_data, state) - output[state].extend( - path_join(root, *item.new.key) - for item in items # pylint: disable=not-an-iterable - if not (drop_root and item.new.key == ROOT) - ) - # TODO: PERF: diff is checking not_in_cache for each even if we only - # need it for the index. - # BUG: not_in_cache file also shows up as modified in staged and - # unstaged. We currently don't know if it is really modified. - output["not_in_cache"].extend( - path_join(root, *item.new.key) - for item in items # pylint: disable=not-an-iterable - if not item.old.in_cache - and not (drop_root and item.new.key == ROOT) - and state != "added" - ) - return output + for item in items: # pylint: disable=not-an-iterable + old = item.old + new = item.new + entry = old if state == "deleted" else new + isdir = entry.oid.isdir if entry.oid else False + obj_type_changed = type_changed(old, new) + if ( + not with_dirs + and isdir + and entry.key == ROOT + and not obj_type_changed + ): + continue + + path = path_join(root, *entry.key, isdir=isdir) + output[state].append(path) + if not item.old.in_cache and state != "added": + output["not_in_cache"].append(path) + return dict(output) -def _granular_diff_oid( +def _diff( root, old_oid: Optional["HashInfo"], old_obj: Optional["HashFile"], new_oid: Optional["HashInfo"], new_obj: Optional["HashFile"], - cache: "HashFileDB", + odb: "HashFileDB", with_dirs: bool = False, - **kwargs: Any, + granular: bool = False, ): - return _granular_diff(root, old_obj, new_obj, cache, with_dirs=with_dirs) + if not granular or not (old_obj or new_obj): + # we don't have enough information to give full details + return _shallow_diff(root, old_oid, new_oid, odb) + return _granular_diff(root, old_obj, new_obj, odb, with_dirs=with_dirs) class GitInfo(TypedDict, total=False): @@ -177,21 +174,19 @@ def _diff_index_to_wtree(repo: "Repo", **kwargs: Any) -> Dict[str, List[str]]: cache = repo.odb.repo root = str(out) old = out.get_obj() + if not old and new and out.hash_info == new.hash_info: + old = new + with ui.status(f"Calculating diff for {root} between index/workspace"): - if kwargs.get("granular", False): - d = _granular_diff_oid( - root, - old.hash_info if old else None, - old, - new.hash_info if new else None, - new, - cache, - **kwargs, - ) - else: - d = _shallow_diff( - root, out.hash_info, new.hash_info if new else None, cache - ) + d = _diff( + root, + out.hash_info, + old, + new.hash_info if new else None, + new, + cache, + **kwargs, + ) for state, items in d.items(): if not items: @@ -221,12 +216,9 @@ def _diff_head_to_index( old_obj, old_oid = obj_d.get(head, (None, None)) new_obj, new_oid = obj_d.get("index", (None, None)) with ui.status(f"Calculating diff for {root} between head/index"): - if kwargs.get("granular", False): - d = _granular_diff_oid( - root, old_oid, old_obj, new_oid, new_obj, cache, **kwargs - ) - else: - d = _shallow_diff(root, old_oid, new_oid, cache) + d = _diff( + root, old_oid, old_obj, new_oid, new_obj, cache, **kwargs + ) for state, items in d.items(): if not items: diff --git a/tests/func/test_data_status.py b/tests/func/test_data_status.py index c99990a7c..3eceeb823 100644 --- a/tests/func/test_data_status.py +++ b/tests/func/test_data_status.py @@ -173,14 +173,23 @@ def test_missing_cache_workspace_exists(M, tmp_dir, dvc, scm): "not_in_cache": M.unordered("foobar", join("dir", "")), "git": M.dict(), } - # assert dvc.data_status(granular=True, untracked_files="all") == { - # **EMPTY_STATUS, - # "untracked": M.unordered("foobar.dvc", "dir.dvc", ".gitignore"), - # "committed": {"added": M.unordered("foobar", join("dir", ""))}, - # "uncommitted": {}, - # "not_in_cache": M.unordered("foobar", join("dir", "")), - # "git": M.dict(), - # } + + assert dvc.data_status(granular=True, untracked_files="all") == { + **EMPTY_STATUS, + "untracked": M.unordered("foobar.dvc", "dir.dvc", ".gitignore"), + # "committed": { + # "added": M.unordered( + # "foobar", join("dir", "foo"), join("dir", "bar") + # )}, + "committed": {"added": M.unordered("foobar", join("dir", ""))}, + "uncommitted": {}, + # even though we don't have cache obj, we can repurpose + # what we have in the workspace + "not_in_cache": M.unordered( + "foobar", join("dir", "bar"), join("dir", "foo") + ), + "git": M.dict(), + } def test_missing_cache_missing_workspace(M, tmp_dir, dvc, scm): @@ -191,21 +200,21 @@ def test_missing_cache_missing_workspace(M, tmp_dir, dvc, scm): assert dvc.data_status(untracked_files="all") == { **EMPTY_STATUS, - "untracked": M.unordered("foobar.dvc", "dir.dvc", ".gitignore"), - "committed": {"added": M.unordered(join("dir", ""), "foobar")}, - "uncommitted": {"deleted": M.unordered(join("dir", ""), "foobar")}, - "not_in_cache": M.unordered(join("dir", ""), "foobar"), + "untracked": ["foobar.dvc", "dir.dvc", ".gitignore"], + "uncommitted": {"deleted": ["foobar", join("dir", "")]}, + "committed": {"added": ["foobar", join("dir", "")]}, + "not_in_cache": ["foobar", join("dir", "")], "git": M.dict(), } - # assert dvc.data_status(granular=True, untracked_files="all") == { - # **EMPTY_STATUS, - # "untracked": M.unordered("foobar.dvc", "dir.dvc", ".gitignore"), - # "uncommitted": {"deleted": [join("dir", ""), "foobar"]}, - # "committed": {"added": M.unordered(join("dir", ""), "foobar")}, - # "not_in_cache": M.unordered(join("dir", ""), "foobar"), - # "git": M.dict(), - # } + assert dvc.data_status(granular=True, untracked_files="all") == { + **EMPTY_STATUS, + "untracked": ["foobar.dvc", "dir.dvc", ".gitignore"], + "uncommitted": {"deleted": ["foobar", join("dir", "")]}, + "committed": {"added": ["foobar", join("dir", "")]}, + "not_in_cache": ["foobar", join("dir", "")], + "git": M.dict(), + } def test_git_committed_missing_cache_workspace_exists(M, tmp_dir, dvc, scm): @@ -215,18 +224,20 @@ def test_git_committed_missing_cache_workspace_exists(M, tmp_dir, dvc, scm): assert dvc.data_status(untracked_files="all") == { **EMPTY_STATUS, - "not_in_cache": M.unordered(join("dir", ""), "foobar"), + "not_in_cache": ["foobar", join("dir", "")], "git": M.dict(), - "unchanged": M.unordered(join("dir", ""), "foobar"), + # TODO: clarify what is unchanged + "unchanged": M.unordered("foobar", join("dir", "")), + } + assert dvc.data_status(granular=True) == { + **EMPTY_STATUS, + "not_in_cache": M.unordered( + "foobar", join("dir", "foo"), join("dir", "bar") + ), + "git": M.dict(), + # TODO: clarify what is unchanged + "unchanged": M.unordered("foobar"), } - # assert dvc.data_status(granular=True) == { - # **EMPTY_STATUS, - # "not_in_cache": M.unordered("bar"), - # "git": M.dict(), - # "unchanged": [], - # "committed": {"modified": ["bar"]}, - # "uncommitted": {"modified": ["bar"], "added": [join("dir", "foo")]}, - # } def test_git_committed_missing_cache_missing_workspace(M, tmp_dir, dvc, scm): @@ -241,13 +252,13 @@ def test_git_committed_missing_cache_missing_workspace(M, tmp_dir, dvc, scm): "not_in_cache": M.unordered(join("dir", ""), "foobar"), "git": M.dict(), } - # assert dvc.data_status(granular=True, untracked_files="all") == { - # **EMPTY_STATUS, - # "untracked": M.unordered("foobar.dvc", "dir.dvc", ".gitignore"), - # "committed": {}, - # "uncommitted": {}, - # "git": M.dict(), - # } + assert dvc.data_status(granular=True, untracked_files="all") == { + **EMPTY_STATUS, + "committed": {}, + "uncommitted": {"deleted": M.unordered(join("dir", ""), "foobar")}, + "not_in_cache": M.unordered(join("dir", ""), "foobar"), + "git": M.dict(), + } def test_partial_missing_cache(M, tmp_dir, dvc, scm): @@ -262,12 +273,35 @@ def test_partial_missing_cache(M, tmp_dir, dvc, scm): "committed": {"added": [join("dir", "")]}, "git": M.dict(), } - # assert dvc.data_status(granular=True) == { - # **EMPTY_STATUS, - # "committed": {"added": [join("dir", "foo"), join("dir", "bar")]}, - # "not_in_cache": [join("dir", "foo")], - # "git": M.dict(), - # } + assert dvc.data_status(granular=True) == { + **EMPTY_STATUS, + "committed": { + "added": M.unordered(join("dir", "foo"), join("dir", "bar")) + }, + "not_in_cache": [join("dir", "foo")], + "git": M.dict(), + } + + +def test_missing_dir_object_from_head(M, tmp_dir, dvc, scm): + (stage,) = tmp_dir.dvc_gen( + {"dir": {"foo": "foo", "bar": "bar"}}, commit="add dir" + ) + remove("dir") + tmp_dir.dvc_gen({"dir": {"foobar": "foobar"}}) + odb = dvc.odb.repo + odb.fs.rm(odb.oid_to_path(stage.outs[0].hash_info.value)) + + assert dvc.data_status() == { + **EMPTY_STATUS, + "committed": {"modified": [join("dir", "")]}, + "git": M.dict(), + } + assert dvc.data_status(granular=True) == { + **EMPTY_STATUS, + "committed": {"added": [join("dir", "foobar")]}, + "git": M.dict(), + } def test_root_from_dir_to_file(M, tmp_dir, dvc, scm): @@ -278,7 +312,18 @@ def test_root_from_dir_to_file(M, tmp_dir, dvc, scm): assert dvc.data_status() == { **EMPTY_STATUS, "committed": {"added": [join("data", "")]}, - "uncommitted": {"deleted": [join("data", "")], "added": ["data"]}, + "uncommitted": {"modified": [join("data")]}, + "git": M.dict(), + } + assert dvc.data_status(granular=True) == { + **EMPTY_STATUS, + "committed": { + "added": M.unordered(join("data", "foo"), join("data", "bar")) + }, + "uncommitted": { + "deleted": M.unordered(join("data", "foo"), join("data", "bar")), + "modified": ["data"], + }, "git": M.dict(), } @@ -291,6 +336,15 @@ def test_root_from_file_to_dir(M, tmp_dir, dvc, scm): assert dvc.data_status() == { **EMPTY_STATUS, "committed": {"added": ["data"]}, - "uncommitted": {"deleted": ["data"], "added": [join("data", "")]}, + "uncommitted": {"modified": [join("data", "")]}, + "git": M.dict(), + } + assert dvc.data_status(granular=True) == { + **EMPTY_STATUS, + "committed": {"added": [join("data")]}, + "uncommitted": { + "modified": [join("data", "")], + "added": M.unordered(join("data", "foo"), join("data", "bar")), + }, "git": M.dict(), }