diff --git a/dvc/command/diff.py b/dvc/command/diff.py index 608db8182..f9f7eebc6 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -22,7 +22,7 @@ def _show_md(diff, show_hash=False, hide_missing=False): header = ["Status", "Hash", "Path"] if show_hash else ["Status", "Path"] rows = [] - statuses = ["added", "deleted", "modified"] + statuses = ["added", "deleted", "renamed", "modified"] if not hide_missing: statuses.append("not in cache") for status in statuses: @@ -31,6 +31,8 @@ def _show_md(diff, show_hash=False, hide_missing=False): continue for entry in entries: path = entry["path"] + if isinstance(path, dict): + path = f"{path['old']} -> {path['new']}" if show_hash: check_sum = _digest(entry.get("hash", "")) rows.append([status, check_sum, path]) @@ -72,13 +74,14 @@ class CmdDiff(CmdBase): "added": colorama.Fore.GREEN, "modified": colorama.Fore.YELLOW, "deleted": colorama.Fore.RED, + "renamed": colorama.Fore.GREEN, "not in cache": colorama.Fore.YELLOW, } summary = {} groups = [] - states = ["added", "deleted", "modified"] + states = ["added", "deleted", "renamed", "modified"] if not hide_missing: states.append("not in cache") for state in states: @@ -92,6 +95,8 @@ class CmdDiff(CmdBase): for entry in entries: path = entry["path"] + if isinstance(path, dict): + path = f"{path['old']} -> {path['new']}" checksum = entry.get("hash") summary[state] += 1 if not path.endswith(os.sep) else 0 content.append( @@ -99,7 +104,7 @@ class CmdDiff(CmdBase): space=" ", checksum=_digest(checksum) if checksum else "", separator=" " if checksum else "", - path=entry["path"], + path=path, ) ) @@ -117,7 +122,7 @@ class CmdDiff(CmdBase): fmt = ( "files summary: {added} added, {deleted} deleted," - " {modified} modified" + " {renamed} renamed, {modified} modified" ) if not hide_missing: fmt += ", {not in cache} not in cache" @@ -138,7 +143,12 @@ class CmdDiff(CmdBase): del diff["not in cache"] for key, entries in diff.items(): - entries = sorted(entries, key=lambda entry: entry["path"]) + entries = sorted( + entries, + key=lambda entry: entry["path"]["old"] + if isinstance(entry["path"], dict) + else entry["path"], + ) if not show_hash: for entry in entries: del entry["hash"] diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index afd872a09..55351c0ce 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -65,6 +65,17 @@ def diff(self, a_rev="HEAD", b_rev=None, targets=None): deleted = sorted(deleted_or_missing - set(missing)) modified = sorted(set(old) & set(new)) + # Cases when file was changed and renamed are resulted + # in having deleted and added record + # To cover such cases we need to change hashing function + # to produce rolling/chunking hash + + renamed = _calculate_renamed(new, old, added, deleted) + + for renamed_item in renamed: + added.remove(renamed_item["path"]["new"]) + deleted.remove(renamed_item["path"]["old"]) + ret = { "added": [{"path": path, "hash": new[path]} for path in added], "deleted": [{"path": path, "hash": old[path]} for path in deleted], @@ -73,6 +84,7 @@ def diff(self, a_rev="HEAD", b_rev=None, targets=None): for path in modified if old[path] != new[path] ], + "renamed": renamed, "not in cache": [ {"path": path, "hash": old[path]} for path in missing ], @@ -171,3 +183,34 @@ def _targets_to_path_infos(repo_tree, targets): missing.append(target) return path_infos, missing + + +def _calculate_renamed(new, old, added, deleted): + old_inverted = {} + # It is needed to be dict of lists to cover cases + # when repo has paths with same hash + for path, path_hash in old.items(): + bucket = old_inverted.get(path_hash, None) + if bucket is None: + old_inverted[path_hash] = [path] + else: + bucket.append(path) + + renamed = [] + for path in added: + path_hash = new[path] + old_paths = old_inverted.get(path_hash, None) + if not old_paths: + continue + old_path = None + for tmp_old_path in old_paths: + if tmp_old_path in deleted: + old_path = tmp_old_path + break + if not old_path: + continue + renamed.append( + {"path": {"old": old_path, "new": path}, "hash": path_hash} + ) + + return renamed diff --git a/tests/func/params/test_diff.py b/tests/func/params/test_diff.py index ba8946778..03e82b34f 100644 --- a/tests/func/params/test_diff.py +++ b/tests/func/params/test_diff.py @@ -1,3 +1,7 @@ +import os + +import pytest + from dvc.utils import relpath from dvc.utils.serialize import dump_yaml @@ -14,6 +18,132 @@ def test_diff_no_changes(tmp_dir, scm, dvc): assert dvc.params.diff() == {} +@pytest.mark.parametrize("commit_last", [True, False]) +def test_diff_add_similar_files(tmp_dir, scm, dvc, commit_last): + if commit_last: + last_commit_msg = "commit #2" + a_rev = "HEAD~1" + else: + last_commit_msg = None + a_rev = "HEAD" + + tmp_dir.dvc_gen( + {"dir": {"file": "text1", "subdir": {"file2": "text2"}}}, + commit="commit #1", + ) + tmp_dir.dvc_gen( + {"dir2": {"file": "text1", "subdir": {"file2": "text2"}}}, + commit=last_commit_msg, + ) + assert dvc.diff(a_rev) == { + "added": [ + { + "path": os.path.join("dir2", ""), + "hash": "cb58ee07cb01044db229e4d6121a0dfc.dir", + }, + { + "path": os.path.join("dir2", "file"), + "hash": "cef7ccd89dacf1ced6f5ec91d759953f", + }, + { + "path": os.path.join("dir2", "subdir", "file2"), + "hash": "fe6123a759017e4a2af4a2d19961ed71", + }, + ], + "deleted": [], + "modified": [], + "renamed": [], + "not in cache": [], + } + + +@pytest.mark.parametrize("commit_last", [True, False]) +def test_diff_rename_folder(tmp_dir, scm, dvc, commit_last): + if commit_last: + last_commit_msg = "commit #2" + a_rev = "HEAD~1" + else: + last_commit_msg = None + a_rev = "HEAD" + + tmp_dir.dvc_gen( + {"dir": {"file": "text1", "subdir": {"file2": "text2"}}}, + commit="commit #1", + ) + (tmp_dir / "dir").replace(tmp_dir / "dir2") + tmp_dir.dvc_add("dir2", commit=last_commit_msg) + assert dvc.diff(a_rev) == { + "added": [], + "deleted": [], + "modified": [], + "renamed": [ + { + "path": { + "old": os.path.join("dir", ""), + "new": os.path.join("dir2", ""), + }, + "hash": "cb58ee07cb01044db229e4d6121a0dfc.dir", + }, + { + "path": { + "old": os.path.join("dir", "file"), + "new": os.path.join("dir2", "file"), + }, + "hash": "cef7ccd89dacf1ced6f5ec91d759953f", + }, + { + "path": { + "old": os.path.join("dir", "subdir", "file2"), + "new": os.path.join("dir2", "subdir", "file2"), + }, + "hash": "fe6123a759017e4a2af4a2d19961ed71", + }, + ], + "not in cache": [], + } + + +@pytest.mark.parametrize("commit_last", [True, False]) +def test_diff_rename_file(tmp_dir, scm, dvc, commit_last): + if commit_last: + last_commit_msg = "commit #2" + a_rev = "HEAD~1" + else: + last_commit_msg = None + a_rev = "HEAD" + + paths = tmp_dir.gen( + {"dir": {"file": "text1", "subdir": {"file2": "text2"}}} + ) + tmp_dir.dvc_add(paths, commit="commit #1") + (tmp_dir / "dir" / "file").replace(tmp_dir / "dir" / "subdir" / "file3") + + tmp_dir.dvc_add(paths, commit=last_commit_msg) + assert dvc.diff(a_rev) == { + "added": [], + "deleted": [], + "modified": [ + { + "path": os.path.join("dir", ""), + "hash": { + "old": "cb58ee07cb01044db229e4d6121a0dfc.dir", + "new": "a4ac9c339aacc60b6a3152e362c319c8.dir", + }, + } + ], + "renamed": [ + { + "path": { + "old": os.path.join("dir", "file"), + "new": os.path.join("dir", "subdir", "file3"), + }, + "hash": "cef7ccd89dacf1ced6f5ec91d759953f", + } + ], + "not in cache": [], + } + + def test_diff(tmp_dir, scm, dvc): tmp_dir.gen("params.yaml", "foo: bar") dvc.run(cmd="echo params.yaml", params=["foo"], single_stage=True) diff --git a/tests/func/test_diff.py b/tests/func/test_diff.py index bb65850cc..a6abb696d 100644 --- a/tests/func/test_diff.py +++ b/tests/func/test_diff.py @@ -29,6 +29,7 @@ def test_added(tmp_dir, scm, dvc): "deleted": [], "modified": [], "not in cache": [], + "renamed": [], } @@ -57,6 +58,7 @@ def test_no_cache_entry(tmp_dir, scm, dvc): } ], "not in cache": [], + "renamed": [], } @@ -72,6 +74,7 @@ def test_deleted(tmp_dir, scm, dvc, delete_data): "deleted": [{"path": "file", "hash": digest("text")}], "modified": [], "not in cache": [], + "renamed": [], } @@ -89,6 +92,7 @@ def test_modified(tmp_dir, scm, dvc): } ], "not in cache": [], + "renamed": [], } @@ -106,6 +110,7 @@ def test_refs(tmp_dir, scm, dvc): "deleted": [], "modified": [{"path": "file", "hash": {"old": HEAD_1, "new": HEAD}}], "not in cache": [], + "renamed": [], } assert dvc.diff("HEAD~2", "HEAD~1") == { @@ -113,6 +118,7 @@ def test_refs(tmp_dir, scm, dvc): "deleted": [], "modified": [{"path": "file", "hash": {"old": HEAD_2, "new": HEAD_1}}], "not in cache": [], + "renamed": [], } with pytest.raises(DvcException, match=r"unknown Git revision 'missing'"): @@ -144,6 +150,7 @@ def test_directories(tmp_dir, scm, dvc): "deleted": [], "modified": [], "not in cache": [], + "renamed": [], } assert dvc.diff(":/directory", ":/modify") == { @@ -163,6 +170,7 @@ def test_directories(tmp_dir, scm, dvc): }, ], "not in cache": [], + "renamed": [], } assert dvc.diff(":/modify", ":/delete") == { @@ -180,6 +188,7 @@ def test_directories(tmp_dir, scm, dvc): } ], "not in cache": [], + "renamed": [], } @@ -208,6 +217,7 @@ def test_diff_no_cache(tmp_dir, scm, dvc): diff = dvc.diff() assert diff["added"] == [] assert diff["deleted"] == [] + assert diff["renamed"] == [] assert diff["modified"] == [] assert diff["not in cache"] == [ { @@ -250,6 +260,7 @@ def test_diff_dirty(tmp_dir, scm, dvc): } ], "not in cache": [], + "renamed": [], } @@ -300,6 +311,7 @@ def test_targets_single_file(tmp_dir, scm, dvc): } ], "not in cache": [], + "renamed": [], } @@ -317,6 +329,7 @@ def test_targets_single_dir(tmp_dir, scm, dvc): "deleted": [], "modified": [], "not in cache": [], + "renamed": [], } assert dvc.diff(targets=["dir"]) == expected_result @@ -331,6 +344,7 @@ def test_targets_single_file_in_dir(tmp_dir, scm, dvc): "deleted": [], "modified": [], "not in cache": [], + "renamed": [], } @@ -347,6 +361,7 @@ def test_targets_two_files_in_dir(tmp_dir, scm, dvc): "deleted": [], "modified": [], "not in cache": [], + "renamed": [], } @@ -369,6 +384,7 @@ def test_targets_file_and_dir(tmp_dir, scm, dvc): } ], "not in cache": [], + "renamed": [], } @@ -385,6 +401,7 @@ def test_targets_single_dir_with_file(tmp_dir, scm, dvc): "deleted": [], "modified": [], "not in cache": [], + "renamed": [], } assert dvc.diff(targets=["dir_with"]) == expected_result @@ -404,4 +421,5 @@ def test_targets_single_file_in_dir_with_file(tmp_dir, scm, dvc): "deleted": [], "modified": [], "not in cache": [], + "renamed": [], } diff --git a/tests/unit/command/test_diff.py b/tests/unit/command/test_diff.py index 51a7fe528..9b67910ab 100644 --- a/tests/unit/command/test_diff.py +++ b/tests/unit/command/test_diff.py @@ -28,6 +28,15 @@ def test_default(mocker, caplog): "added": [{"path": "file", "hash": "00000000"}], "deleted": [], "modified": [], + "renamed": [ + { + "path": { + "old": os.path.join("data", "file_old"), + "new": os.path.join("data", "file_new"), + }, + "hash": "11111111", + } + ], "not in cache": [], } mocker.patch("dvc.repo.Repo.diff", return_value=diff) @@ -37,8 +46,12 @@ def test_default(mocker, caplog): "Added:\n" " file\n" "\n" - "files summary: 1 added, 0 deleted, 0 modified, 0 not in cache" - ) in caplog.text + "Renamed:\n" + " data{sep}file_old -> data{sep}file_new\n" + "\n" + "files summary: 1 added, 0 deleted, " + "1 renamed, 0 modified, 0 not in cache" + ).format(sep=os.path.sep) in caplog.text def test_show_hash(mocker, caplog): @@ -55,6 +68,15 @@ def test_show_hash(mocker, caplog): {"path": "file2", "hash": {"old": "AAAAAAAA", "new": "BBBBBBBB"}}, {"path": "file1", "hash": {"old": "CCCCCCCC", "new": "DDDDDDDD"}}, ], + "renamed": [ + { + "path": { + "old": os.path.join("data", "file_old"), + "new": os.path.join("data", "file_new"), + }, + "hash": "11111111", + } + ], "not in cache": [], } mocker.patch("dvc.repo.Repo.diff", return_value=diff) @@ -65,11 +87,19 @@ def test_show_hash(mocker, caplog): " 00000000 " + os.path.join("data", "bar") + "\n" " 11111111 " + os.path.join("data", "foo") + "\n" "\n" + "Renamed:\n" + " 11111111 " + + os.path.join("data", "file_old") + + " -> " + + os.path.join("data", "file_new") + + "\n" + "\n" "Modified:\n" " CCCCCCCC..DDDDDDDD file1\n" " AAAAAAAA..BBBBBBBB file2\n" "\n" - "files summary: 0 added, 2 deleted, 2 modified, 0 not in cache" + "files summary: 0 added, 2 deleted, " + "1 renamed, 2 modified, 0 not in cache" ) in caplog.text @@ -106,6 +136,12 @@ def test_show_json_and_hash(mocker, caplog): ], "deleted": [], "modified": [], + "renamed": [ + { + "path": {"old": "file_old", "new": "file_new"}, + "hash": "11111111", + } + ], "not in cache": [], } mocker.patch("dvc.repo.Repo.diff", return_value=diff) @@ -117,6 +153,10 @@ def test_show_json_and_hash(mocker, caplog): ) assert '"deleted": []' in caplog.text assert '"modified": []' in caplog.text + assert ( + '"renamed": [{"path": {"old": "file_old", ' + '"new": "file_new"}, "hash": "11111111"}]' in caplog.text + ) assert '"not in cache": []' in caplog.text @@ -130,6 +170,12 @@ def test_show_json_hide_missing(mocker, caplog): ], "deleted": [], "modified": [], + "renamed": [ + { + "path": {"old": "file_old", "new": "file_new"}, + "hash": "11111111", + } + ], "not in cache": [], } mocker.patch("dvc.repo.Repo.diff", return_value=diff) @@ -137,6 +183,10 @@ def test_show_json_hide_missing(mocker, caplog): assert 0 == cmd.run() assert '"added": [{"path": "file1"}, {"path": "file2"}]' in caplog.text assert '"deleted": []' in caplog.text + assert ( + '"renamed": [{"path": {"old": "file_old", "new": "file_new"}' + in caplog.text + ) assert '"modified": []' in caplog.text assert '"not in cache": []' not in caplog.text @@ -193,18 +243,20 @@ def test_show_md(): ], "modified": [{"path": "file"}], "added": [{"path": "file"}], + "renamed": [{"path": {"old": "file_old", "new": "file_new"}}], "not in cache": [{"path": "file2"}], } assert _show_md(diff) == ( - "| Status | Path |\n" - "|--------------|----------|\n" - "| added | file |\n" - "| deleted | zoo |\n" - "| deleted | data{sep} |\n" - "| deleted | data{sep}foo |\n" - "| deleted | data{sep}bar |\n" - "| modified | file |\n" - "| not in cache | file2 |\n" + "| Status | Path |\n" + "|--------------|----------------------|\n" + "| added | file |\n" + "| deleted | zoo |\n" + "| deleted | data{sep} |\n" + "| deleted | data{sep}foo |\n" + "| deleted | data{sep}bar |\n" + "| renamed | file_old -> file_new |\n" + "| modified | file |\n" + "| not in cache | file2 |\n" ).format(sep=os.path.sep) @@ -220,18 +272,25 @@ def test_show_md_with_hash(): {"path": "file", "hash": {"old": "AAAAAAAA", "new": "BBBBBBBB"}} ], "added": [{"path": "file", "hash": "00000000"}], + "renamed": [ + { + "path": {"old": "file_old", "new": "file_new"}, + "hash": "11111111", + } + ], "not in cache": [{"path": "file2", "hash": "12345678"}], } assert _show_md(diff, show_hash=True) == ( - "| Status | Hash | Path |\n" - "|--------------|--------------------|----------|\n" - "| added | 00000000 | file |\n" - "| deleted | 22222 | zoo |\n" - "| deleted | XXXXXXXX | data{sep} |\n" - "| deleted | 11111111 | data{sep}foo |\n" - "| deleted | 00000000 | data{sep}bar |\n" - "| modified | AAAAAAAA..BBBBBBBB | file |\n" - "| not in cache | 12345678 | file2 |\n" + "| Status | Hash | Path |\n" + "|--------------|--------------------|----------------------|\n" + "| added | 00000000 | file |\n" + "| deleted | 22222 | zoo |\n" + "| deleted | XXXXXXXX | data{sep} |\n" + "| deleted | 11111111 | data{sep}foo |\n" + "| deleted | 00000000 | data{sep}bar |\n" + "| renamed | 11111111 | file_old -> file_new |\n" + "| modified | AAAAAAAA..BBBBBBBB | file |\n" + "| not in cache | 12345678 | file2 |\n" ).format(sep=os.path.sep) @@ -245,17 +304,19 @@ def test_show_md_hide_missing(): ], "modified": [{"path": "file"}], "added": [{"path": "file"}], + "renamed": [{"path": {"old": "file_old", "new": "file_new"}}], "not in cache": [{"path": "file2"}], } assert _show_md(diff, hide_missing=True) == ( - "| Status | Path |\n" - "|----------|----------|\n" - "| added | file |\n" - "| deleted | zoo |\n" - "| deleted | data{sep} |\n" - "| deleted | data{sep}foo |\n" - "| deleted | data{sep}bar |\n" - "| modified | file |\n" + "| Status | Path |\n" + "|----------|----------------------|\n" + "| added | file |\n" + "| deleted | zoo |\n" + "| deleted | data{sep} |\n" + "| deleted | data{sep}foo |\n" + "| deleted | data{sep}bar |\n" + "| renamed | file_old -> file_new |\n" + "| modified | file |\n" ).format(sep=os.path.sep) @@ -266,6 +327,12 @@ def test_hide_missing(mocker, caplog): "added": [{"path": "file", "hash": "00000000"}], "deleted": [], "modified": [], + "renamed": [ + { + "path": {"old": "file_old", "new": "file_new"}, + "hash": "11111111", + } + ], "not in cache": [], } mocker.patch("dvc.repo.Repo.diff", return_value=diff) @@ -275,6 +342,9 @@ def test_hide_missing(mocker, caplog): "Added:\n" " file\n" "\n" - "files summary: 1 added, 0 deleted, 0 modified" + "Renamed:\n" + " file_old -> file_new\n" + "\n" + "files summary: 1 added, 0 deleted, 1 renamed, 0 modified" ) in caplog.text assert "not in cache" not in caplog.text