Stone_Red/dvc
1
0
Fork 0
mirror of https://github.com/Stone-Red-Code/dvc.git synced 2026-11-04 15:28:27 +01:00
Code Issues Packages Projects Releases Wiki Activity

dvc.command.diff: add renamed detection feature (#5160)

Browse Source 
fixes #5150
This commit is contained in:
Dmitry Kropachev
2021-01-09 13:49:03 +09:00
committed by GitHub
parent 388eccad13
commit 6b05cc5e24
5 changed files with 306 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files
dvc/command/diff.py
+15 -5
View File
@@ -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"]
dvc/repo/diff.py
+43
View File
@@ -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
tests/func/params/test_diff.py
+130
View File
@@ -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)
tests/func/test_diff.py
+18
View File
@@ -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": [],
}
tests/unit/command/test_diff.py
+100 -30
View File
@@ -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