From c2639cfb3a8d5087fac1d581270009b63cccd041 Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Sat, 23 Mar 2019 15:21:16 +0600 Subject: [PATCH 01/25] dvc: add support for dvc diff command Some summary statistics regarding the folder/file. How many files were added/deleted/modified. How size changed. TO DO things: 1)To support for detecting diff after `dvc move` command. 2)To add unit tests. --- dvc/cli.py | 2 + dvc/command/diff.py | 36 +++++++++++ dvc/repo/__init__.py | 1 + dvc/repo/diff.py | 135 ++++++++++++++++++++++++++++++++++++++++ dvc/scm/git/__init__.py | 80 ++++++++++++++++++++++++ tests/test_diff.py | 96 ++++++++++++++++++++++++++++ 6 files changed, 350 insertions(+) create mode 100644 dvc/command/diff.py create mode 100644 dvc/repo/diff.py create mode 100644 tests/test_diff.py diff --git a/dvc/cli.py b/dvc/cli.py index d8af5717a..7fe70dbef 100644 --- a/dvc/cli.py +++ b/dvc/cli.py @@ -32,6 +32,7 @@ import dvc.command.pipeline as pipeline import dvc.command.daemon as daemon import dvc.command.commit as commit import dvc.command.tag as tag +import dvc.command.diff as diff from dvc.exceptions import DvcParserError from dvc import VERSION @@ -62,6 +63,7 @@ COMMANDS = [ daemon, commit, tag, + diff, ] diff --git a/dvc/command/diff.py b/dvc/command/diff.py new file mode 100644 index 000000000..01a12c4cc --- /dev/null +++ b/dvc/command/diff.py @@ -0,0 +1,36 @@ +from __future__ import unicode_literals + +from dvc.command.base import CmdBase + + +class CmdMove(CmdBase): + def run(self): + self.repo.diff( + self.args.target, a_ref=self.args.a_ref, b_ref=self.args.b_ref + ) + return 0 + + +def add_parser(subparsers, parent_parser): + description = ( + "show diff of a data file or a directory that " + "is under DVC control. Some basic statistics " + "summary, how many files were deleted/changed." + ) + help = "Show a diff of a DVC controlled data file or a directory." + diff_parser = subparsers.add_parser( + "diff", parents=[parent_parser], description=description, help=help + ) + diff_parser.add_argument( + "target", help="Source path to a data file or directory." + ) + diff_parser.add_argument( + "a_ref", help="Git tag/reference from which diff calculates" + ) + diff_parser.add_argument( + "b_ref", + help="Git tag/reference till which diff calculates", + nargs="?", + default=None, + ) + diff_parser.set_defaults(func=CmdMove) diff --git a/dvc/repo/__init__.py b/dvc/repo/__init__.py index 43ecbe600..a082e7c0b 100644 --- a/dvc/repo/__init__.py +++ b/dvc/repo/__init__.py @@ -33,6 +33,7 @@ class Repo(object): from dvc.repo.gc import gc from dvc.repo.commit import commit from dvc.repo.pkg import install_pkg + from dvc.repo.diff import diff from dvc.repo.brancher import brancher def __init__(self, root_dir=None): diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py new file mode 100644 index 000000000..e8681266e --- /dev/null +++ b/dvc/repo/diff.py @@ -0,0 +1,135 @@ +import os +import math + +import dvc.logger as logger + + +def _extract_tree(self, cache_dir): + lst = self.cache.local.load_dir_cache(cache_dir) + return {i["relpath"]: i["md5"] for i in lst} + + +def _convert_size(size_bytes): + sign = "" if size_bytes > 0 else "-" + if size_bytes > 0: + sign = "" + else: + size_bytes = -size_bytes + sign = "-" + size_bytes = abs(size_bytes) + if size_bytes == 0: + return "0B" + size_name = ("B", "KB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB") + i = int(math.floor(math.log(size_bytes, 1024))) + p = math.pow(1024, i) + s = round(size_bytes / p, 2) + return "%s %s" % (s, size_name[i]), sign + + +def _get_tree_changes(self, a_entries, b_entries): + result = {"del": 0, "ident": 0, "changes": 0, "new": 0, "moves": 0} + keys = set(a_entries.keys()) + keys.update(b_entries.keys()) + diff_size = 0 + for key in keys: + if key in a_entries and key in b_entries: + if a_entries[key] == b_entries[key]: + result["ident"] += 1 + else: + result["changes"] += 1 + diff_size += os.path.getsize( + self.cache.local.get(b_entries[key]) + ) - os.path.getsize(self.cache.local.get(a_entries[key])) + elif key in a_entries: + result["del"] += 1 + diff_size -= os.path.getsize(self.cache.local.get(a_entries[key])) + else: + result["new"] += 1 + diff_size += os.path.getsize(self.cache.local.get(b_entries[key])) + result["diff_size"], result["diff_sign"] = _convert_size(diff_size) + return result + + +def plural(s): + return str(s) + " file" + "s" * (s > 1) + + +def _diff_dir(self, target, diff_dct): + msg = "" + a_entries, b_entries = {}, {} + if not diff_dct["new_file"]: + msg += "-{} with md5 {}\n".format( + target, diff_dct["a_fname"].split(".dir")[0] + ) + a_entries = _extract_tree(self, diff_dct["a_fname"]) + if not diff_dct["deleted_file"]: + msg += "+{} with md5 {}\n".format( + target, diff_dct["b_fname"].split(".dir")[0] + ) + b_entries = _extract_tree(self, diff_dct["b_fname"]) + msg += "\n" + result = _get_tree_changes(self, a_entries, b_entries) + msg += plural(result["ident"]) + " didn't change, " + msg += plural(result["changes"]) + " modified, " + msg += plural(result["new"]) + " added, " + msg += plural(result["del"]) + " deleted, " + msg += "size was " + "increased" * (result["diff_sign"] != "-") + msg += ( + "decreased" * (result["diff_sign"] == "-") + + " by " + + result["diff_size"] + ) + return msg + + +def _diff_file(self, target, diff_dct): + msg = "" + size = 0 + # os.path.getsize(self.cache.local.get(diff_obj['a_fname'])) + if not diff_dct["new_file"]: + msg += "-{} with md5 {}\n".format( + target, diff_dct["a_fname"].split(".dir")[0] + ) + size -= os.path.getsize(self.cache.local.get(diff_dct["a_fname"])) + if not diff_dct["deleted_file"]: + msg += "+{} with md5 {}\n".format( + target, diff_dct["b_fname"].split(".dir")[0] + ) + size += os.path.getsize(self.cache.local.get(diff_dct["b_fname"])) + if not diff_dct["new_file"] and not diff_dct["deleted_file"] and size == 0: + msg += "file did not change\n" + elif diff_dct["new_file"]: + msg += "added file with size %s" % _convert_size(size)[0] + elif diff_dct["deleted_file"]: + msg += "deleted file with size %s" % _convert_size(size)[0] + else: + msg += "file was modified, file size " + "increased" * (size >= 0) + msg += "decreased" * (size < 0) + " by " + _convert_size(size)[0] + return msg + + +def diff(self, target, a_ref=None, b_ref=None): + """ + Gerenates diff message string output + input: + target - file/folder to check diff of + a_ref - first git ref/tag + b_ref(optional) - second git ref/tag + output: + string of output message with diff info + """ + diff_dct = self.scm.diff(target, a_ref=a_ref, b_ref=b_ref) + msg = "dvc diff for '{}' from {} to {} \n\n".format( + target, diff_dct["a_tag"], diff_dct["b_tag"] + ) + if diff_dct["equal"]: + logger.info(msg) + return msg + if diff_dct["is_dir"]: + dir_info = _diff_dir(self, target, diff_dct) + msg += dir_info + else: + file_info = _diff_file(self, target, diff_dct) + msg += file_info + logger.info(msg) + return msg diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index cd585f841..1b5a0fc0e 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals import os +import re from dvc.utils.compat import str, open from dvc.utils import fix_env @@ -235,3 +236,82 @@ class Git(Base): def get_tree(self, rev): return GitTree(self.git, rev) + + def _get_diff_obj(self, file_name, a_ref, **kwargs): + + from gitdb.exc import BadObject, BadName + + file_name = file_name[:-1] if file_name.endswith("/") else file_name + file_name += ".dvc" + commits = [] + try: + if "b_ref" in kwargs and kwargs["b_ref"] is not None: + b_ref = kwargs["b_ref"] + a_commit = self.git.commit(a_ref) + b_commit = self.git.commit(b_ref) + commits.append(str(a_commit)) + commits.append(str(b_commit)) + diff_obj = a_commit.diff(b_commit, paths=file_name) + # other way of getting the diff between commits : + # self.git.diff(a_commit, b_commit) returns the diff string + # by reading the files in a, b commits and computing the diff + else: + a_commit = self.git.head.commit + b_commit = self.git.commit(a_ref) + commits.append(str(a_commit)) + commits.append(str(b_commit)) + if a_commit == b_commit: + return None, commits + diff_obj = a_commit.diff(b_commit, paths=file_name) + except (BadName, BadObject) as e: + raise SCMError(e) + if len(diff_obj) == 0: + msg = "Have not found file/folder '{}' in the commits" + raise FileNotInRepoError(msg.format(file_name)) + return diff_obj[0], commits + + def _extract_fname(self, is_deleted, blob_obj): + if not is_deleted: + s1 = blob_obj.data_stream.read().decode("utf-8") + a_hash, a_fname = re.findall(r"md5:\s([a-fA-F\d]{32}[.dir]*)", s1) + return a_hash, a_fname + else: + return None, None + + def diff(self, file_name, a_ref, **kwargs): + """ + function for diff between two git tag commits + returns the dvc hash names of changed file/folder + file_name - file on which to calculate diff + returns dictionary with keys: (new_file, a_file_name, b_file_name) + """ + diff_dct = {"equal": False} + diff_obj, commit_refs = self._get_diff_obj(file_name, a_ref, **kwargs) + diff_dct["a_tag"] = commit_refs[0] + diff_dct["b_tag"] = commit_refs[1] + if diff_obj is None: + diff_dct["equal"] = True + return diff_dct + if not diff_obj.a_path: + msg = ( + "file path {} doesn't exist maybe you forgot to add dataset" + " to dvc?" + ) + raise SCMError(msg.format(file_name)) + # this function doesn't support an indicating of moved of a file/folder + diff_dct["new_file"] = diff_obj.new_file + diff_dct["deleted_file"] = diff_obj.deleted_file + diff_dct["a_hash"], diff_dct["a_fname"] = self._extract_fname( + diff_dct["new_file"], diff_obj.a_blob + ) + diff_dct["b_hash"], diff_dct["b_fname"] = self._extract_fname( + diff_dct["deleted_file"], diff_obj.b_blob + ) + actual_path = ( + diff_dct["b_fname"] if diff_dct["b_fname"] else diff_dct["a_fname"] + ) + diff_dct["is_dir"] = actual_path.endswith(".dir") + diff_dct["file_path"] = ( + diff_obj.b_path if diff_obj.b_path else diff_obj.a_path + ) + return diff_dct diff --git a/tests/test_diff.py b/tests/test_diff.py new file mode 100644 index 000000000..5c9223a60 --- /dev/null +++ b/tests/test_diff.py @@ -0,0 +1,96 @@ +from __future__ import unicode_literals +import os + +from dvc.main import main + + +from dvc.scm.base import FileNotInRepoError + + +from tests.basic_env import TestDvc + + +class TestDiff(TestDvc): + def setUp(self): + super(TestDiff, self).setUp() + + self.new_file = "new_file" + self.create(self.new_file, self.new_file) + self.dvc.add(self.new_file) + self.a_ref = self.git.head.commit + self.git.index.add([self.new_file + ".dvc"]) + self.git.index.commit("adds new_file") + + def test(self): + out = self.dvc.scm.diff(self.new_file, self.a_ref) + self.assertTrue(out["deleted_file"]) + self.assertEqual("new_file.dvc", out["file_path"]) + + +class TestDiffRepo(TestDiff): + def test(self): + msg = self.dvc.diff(self.new_file, a_ref=self.a_ref) + self.assertTrue("deleted file" in msg) + self.assertTrue(self.new_file in msg) + + +class TestDiffCmdLine(TestDiff): + def test(self): + ret = main(["diff", self.new_file, str(self.a_ref)]) + self.assertEqual(ret, 0) + + +class TestDiffDir(TestDvc): + def setUp(self): + super(TestDiffDir, self).setUp() + + self.dvc.add(self.DATA_DIR) + self.git.index.add([self.DATA_DIR + ".dvc"]) + self.git.index.commit("adds data_dir") + self.a_ref = str(self.dvc.scm.git.head.commit) + self.new_file = os.path.join(self.DATA_SUB_DIR, "new_file") + self.create(self.new_file, self.new_file) + self.dvc.add(self.DATA_DIR) + self.git.index.add([self.DATA_DIR + ".dvc"]) + self.git.index.commit(message="adds data_dir with new_file") + + def test(self): + out = self.dvc.scm.diff(self.DATA_DIR, self.a_ref) + self.assertFalse(out["deleted_file"], "new file added checker failed") + self.assertEqual( + self.DATA_DIR + ".dvc", + out["file_path"], + "path to file/folder is incorrect", + ) + + +class TestDiffDirRepo(TestDiffDir): + def test(self): + msg = self.dvc.diff(self.DATA_DIR, a_ref=self.a_ref) + self.assertTrue("1 file deleted" in msg, msg) + self.assertTrue("size was decreased by " in msg, msg) + + +class TestDiffFileNotFound(TestDiffDir): + def setUp(self): + super(TestDiffFileNotFound, self).setUp() + self.unknown_file = "unknown_file_" + str(id(self)) + + def test(self): + with self.assertRaises( + FileNotInRepoError, msg="diff with incorrect arg" + ) as context: + self.dvc.scm.diff(self.unknown_file, self.a_ref) + self.assertTrue( + "Have not found file/folder" in str(context.exception), + "scm raises with incorrect error message", + ) + + +class TestDiffFileNotFoundRepo(TestDiffFileNotFound): + def test(self): + with self.assertRaises( + FileNotInRepoError, msg="diff with incorrect arg" + ) as context: + self.dvc.diff(self.unknown_file, a_ref=self.a_ref) + self.assertTrue("Have not found file/folder" in str(context.exception)) From 716a5eef7f209cec652ce35333b210cc74c24439 Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Mon, 25 Mar 2019 16:12:17 +0600 Subject: [PATCH 02/25] move print output from repo/diff.py to command/diff.py and minor fixes in command/diff.py --- dvc/command/diff.py | 26 +++++++++++++++++++++----- dvc/repo/diff.py | 4 ---- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/dvc/command/diff.py b/dvc/command/diff.py index 01a12c4cc..3159b23bb 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -1,13 +1,29 @@ from __future__ import unicode_literals +import dvc.logger as logger +from dvc.exceptions import DvcException from dvc.command.base import CmdBase -class CmdMove(CmdBase): +class CmdDiff(CmdBase): + def _show(self, msg): + logger.info(msg) + def run(self): - self.repo.diff( - self.args.target, a_ref=self.args.a_ref, b_ref=self.args.b_ref - ) + try: + msg = self.repo.diff( + self.args.target, a_ref=self.args.a_ref, b_ref=self.args.b_ref + ) + self._show(msg) + except DvcException: + msg = "failed to get diff " + if self.args.target: + msg += self.args.target + " " + if self.args.a_ref: + msg += self.args.a_ref + " " + if self.args.b_ref: + msg += self.args.b_ref + " " + logger.error(msg) return 0 @@ -33,4 +49,4 @@ def add_parser(subparsers, parent_parser): nargs="?", default=None, ) - diff_parser.set_defaults(func=CmdMove) + diff_parser.set_defaults(func=CmdDiff) diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index e8681266e..6261068dd 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -1,8 +1,6 @@ import os import math -import dvc.logger as logger - def _extract_tree(self, cache_dir): lst = self.cache.local.load_dir_cache(cache_dir) @@ -123,7 +121,6 @@ def diff(self, target, a_ref=None, b_ref=None): target, diff_dct["a_tag"], diff_dct["b_tag"] ) if diff_dct["equal"]: - logger.info(msg) return msg if diff_dct["is_dir"]: dir_info = _diff_dir(self, target, diff_dct) @@ -131,5 +128,4 @@ def diff(self, target, a_ref=None, b_ref=None): else: file_info = _diff_file(self, target, diff_dct) msg += file_info - logger.info(msg) return msg From 8787995504f4af34663ef2aad6ac6e1845e2d0ce Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Mon, 25 Mar 2019 16:21:18 +0600 Subject: [PATCH 03/25] dvc: fixes style issues with string ops --- dvc/command/diff.py | 4 ++-- dvc/repo/diff.py | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/dvc/command/diff.py b/dvc/command/diff.py index 3159b23bb..e7ff8b395 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -41,11 +41,11 @@ def add_parser(subparsers, parent_parser): "target", help="Source path to a data file or directory." ) diff_parser.add_argument( - "a_ref", help="Git tag/reference from which diff calculates" + "a_ref", help="Git reference from which diff calculates" ) diff_parser.add_argument( "b_ref", - help="Git tag/reference till which diff calculates", + help="Git reference till which diff calculates", nargs="?", default=None, ) diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index 6261068dd..c62a24ee7 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -1,3 +1,5 @@ +from __future__ import unicode_literals + import os import math @@ -21,7 +23,7 @@ def _convert_size(size_bytes): i = int(math.floor(math.log(size_bytes, 1024))) p = math.pow(1024, i) s = round(size_bytes / p, 2) - return "%s %s" % (s, size_name[i]), sign + return "{} {}".format(s, size_name[i]), sign def _get_tree_changes(self, a_entries, b_entries): From b5251960bf681dc6b8f7d797f237ba288083e78c Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Mon, 25 Mar 2019 16:48:44 +0600 Subject: [PATCH 04/25] repo: replace custom _convert_size func to humanize lib --- dvc/repo/diff.py | 32 ++++++++------------------------ 1 file changed, 8 insertions(+), 24 deletions(-) diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index c62a24ee7..41b8382db 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -1,7 +1,7 @@ from __future__ import unicode_literals import os -import math +import humanize def _extract_tree(self, cache_dir): @@ -9,23 +9,6 @@ def _extract_tree(self, cache_dir): return {i["relpath"]: i["md5"] for i in lst} -def _convert_size(size_bytes): - sign = "" if size_bytes > 0 else "-" - if size_bytes > 0: - sign = "" - else: - size_bytes = -size_bytes - sign = "-" - size_bytes = abs(size_bytes) - if size_bytes == 0: - return "0B" - size_name = ("B", "KB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB") - i = int(math.floor(math.log(size_bytes, 1024))) - p = math.pow(1024, i) - s = round(size_bytes / p, 2) - return "{} {}".format(s, size_name[i]), sign - - def _get_tree_changes(self, a_entries, b_entries): result = {"del": 0, "ident": 0, "changes": 0, "new": 0, "moves": 0} keys = set(a_entries.keys()) @@ -46,12 +29,13 @@ def _get_tree_changes(self, a_entries, b_entries): else: result["new"] += 1 diff_size += os.path.getsize(self.cache.local.get(b_entries[key])) - result["diff_size"], result["diff_sign"] = _convert_size(diff_size) + result["diff_size"] = humanize.naturalsize(abs(diff_size)) + result["diff_sign"] = "-" if diff_size < 0 else "" return result -def plural(s): - return str(s) + " file" + "s" * (s > 1) +def plural(string): + return str(string) + " file" + "s" * (string > 1) def _diff_dir(self, target, diff_dct): @@ -99,12 +83,12 @@ def _diff_file(self, target, diff_dct): if not diff_dct["new_file"] and not diff_dct["deleted_file"] and size == 0: msg += "file did not change\n" elif diff_dct["new_file"]: - msg += "added file with size %s" % _convert_size(size)[0] + msg += "added file with size {}".format(humanize.naturalsize(size)) elif diff_dct["deleted_file"]: - msg += "deleted file with size %s" % _convert_size(size)[0] + msg += "deleted file with size {}".format(humanize.naturalsize(size)) else: msg += "file was modified, file size " + "increased" * (size >= 0) - msg += "decreased" * (size < 0) + " by " + _convert_size(size)[0] + msg += "decreased" * (size < 0) + " by " + humanize.naturalsize(size) return msg From 8b50ef8bb66d1f90a0569ddf2e8e1d4bf576beea Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Mon, 25 Mar 2019 17:11:12 +0600 Subject: [PATCH 05/25] repo: replace custom plural func to inflect lib usage --- dvc/repo/diff.py | 19 ++++++++++--------- requirements.txt | 2 ++ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index 41b8382db..741fb3f0d 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -2,6 +2,7 @@ from __future__ import unicode_literals import os import humanize +import inflect def _extract_tree(self, cache_dir): @@ -34,11 +35,8 @@ def _get_tree_changes(self, a_entries, b_entries): return result -def plural(string): - return str(string) + " file" + "s" * (string > 1) - - def _diff_dir(self, target, diff_dct): + engine = inflect.engine() msg = "" a_entries, b_entries = {}, {} if not diff_dct["new_file"]: @@ -53,10 +51,14 @@ def _diff_dir(self, target, diff_dct): b_entries = _extract_tree(self, diff_dct["b_fname"]) msg += "\n" result = _get_tree_changes(self, a_entries, b_entries) - msg += plural(result["ident"]) + " didn't change, " - msg += plural(result["changes"]) + " modified, " - msg += plural(result["new"]) + " added, " - msg += plural(result["del"]) + " deleted, " + msg += f"{result['ident']} " + engine.plural("file", result["ident"]) + msg += " didn't change, " + msg += f"{result['changes']} " + engine.plural("file", result["changes"]) + msg += " modified, " + msg += f"{result['new']} " + engine.plural("file", result["new"]) + msg += " added, " + msg += f"{result['del']} " + engine.plural("file", result["del"]) + msg += " deleted, " msg += "size was " + "increased" * (result["diff_sign"] != "-") msg += ( "decreased" * (result["diff_sign"] == "-") @@ -69,7 +71,6 @@ def _diff_dir(self, target, diff_dct): def _diff_file(self, target, diff_dct): msg = "" size = 0 - # os.path.getsize(self.cache.local.get(diff_obj['a_fname'])) if not diff_dct["new_file"]: msg += "-{} with md5 {}\n".format( target, diff_dct["a_fname"].split(".dir")[0] diff --git a/requirements.txt b/requirements.txt index 80c639d86..374f7baed 100644 --- a/requirements.txt +++ b/requirements.txt @@ -26,3 +26,5 @@ asciimatics>=1.10.0 distro>=1.3.0 appdirs>=1.4.3 treelib>=1.5.5 +inflect>=2.1.0 +humanize>=0.5.1 From 1c9d72942b0b097e0bc19433dd4f53c95a8cfc87 Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Mon, 25 Mar 2019 17:17:50 +0600 Subject: [PATCH 06/25] scm: add usage of os.path.normpath --- dvc/scm/git/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 1b5a0fc0e..24434a937 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -241,7 +241,7 @@ class Git(Base): from gitdb.exc import BadObject, BadName - file_name = file_name[:-1] if file_name.endswith("/") else file_name + file_name = os.path.normpath(file_name) file_name += ".dvc" commits = [] try: From 3499c277ff1f515aae2c3c2566ec75e5f7aaad9f Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Mon, 25 Mar 2019 17:44:54 +0600 Subject: [PATCH 07/25] dvc: fix comment style --- dvc/repo/diff.py | 17 +++++++++-------- dvc/scm/git/__init__.py | 19 ++++++++++++------- tests/test_diff.py | 8 +++++--- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index 741fb3f0d..354f0693b 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -94,14 +94,15 @@ def _diff_file(self, target, diff_dct): def diff(self, target, a_ref=None, b_ref=None): - """ - Gerenates diff message string output - input: - target - file/folder to check diff of - a_ref - first git ref/tag - b_ref(optional) - second git ref/tag - output: - string of output message with diff info + """Gerenates diff message string output + + Args: + target(str) - file/directory to check diff of + a_ref(str) - first git tag + (optional) b_ref(str) - second git tag + + Returns: + string: string of output message with diff info """ diff_dct = self.scm.diff(target, a_ref=a_ref, b_ref=b_ref) msg = "dvc diff for '{}' from {} to {} \n\n".format( diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 24434a937..c1c93e00c 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -266,7 +266,7 @@ class Git(Base): except (BadName, BadObject) as e: raise SCMError(e) if len(diff_obj) == 0: - msg = "Have not found file/folder '{}' in the commits" + msg = "Have not found file/directory '{}' in the commits" raise FileNotInRepoError(msg.format(file_name)) return diff_obj[0], commits @@ -279,11 +279,16 @@ class Git(Base): return None, None def diff(self, file_name, a_ref, **kwargs): - """ - function for diff between two git tag commits - returns the dvc hash names of changed file/folder - file_name - file on which to calculate diff - returns dictionary with keys: (new_file, a_file_name, b_file_name) + """Method for diff between two git tag commits + returns the dvc hash names of changed file/directory + + Args: + file_name(str) - file/directory to perform diff of + a_ref(str) - git reference + b_ref(str) - optional second git reference + + Returns: + dict - dictionary with keys: (new_file, a_file_name, b_file_name) """ diff_dct = {"equal": False} diff_obj, commit_refs = self._get_diff_obj(file_name, a_ref, **kwargs) @@ -298,7 +303,7 @@ class Git(Base): " to dvc?" ) raise SCMError(msg.format(file_name)) - # this function doesn't support an indicating of moved of a file/folder + # function doesn't support an indicating of moved of a file/directory diff_dct["new_file"] = diff_obj.new_file diff_dct["deleted_file"] = diff_obj.deleted_file diff_dct["a_hash"], diff_dct["a_fname"] = self._extract_fname( diff --git a/tests/test_diff.py b/tests/test_diff.py index 5c9223a60..7037ce422 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -60,7 +60,7 @@ class TestDiffDir(TestDvc): self.assertEqual( self.DATA_DIR + ".dvc", out["file_path"], - "path to file/folder is incorrect", + "path to file/directory is incorrect", ) @@ -82,7 +82,7 @@ class TestDiffFileNotFound(TestDiffDir): ) as context: self.dvc.scm.diff(self.unknown_file, self.a_ref) self.assertTrue( - "Have not found file/folder" in str(context.exception), + "Have not found file/directory" in str(context.exception), "scm raises with incorrect error message", ) @@ -93,4 +93,6 @@ class TestDiffFileNotFoundRepo(TestDiffFileNotFound): FileNotInRepoError, msg="diff with incorrect arg" ) as context: self.dvc.diff(self.unknown_file, a_ref=self.a_ref) - self.assertTrue("Have not found file/folder" in str(context.exception)) + self.assertTrue( + "Have not found file/directory" in str(context.exception) + ) From 73b3b5720033f9c0b382534506b9bb1ea2efceb0 Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Mon, 25 Mar 2019 23:40:16 +0600 Subject: [PATCH 08/25] test: change some test logic for test_diff.py --- dvc/repo/diff.py | 2 +- dvc/scm/base.py | 6 ++++++ dvc/scm/git/__init__.py | 5 +++-- tests/test_diff.py | 35 +++++++++++++++++------------------ 4 files changed, 27 insertions(+), 21 deletions(-) diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index 354f0693b..9e0dfb7c0 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -105,7 +105,7 @@ def diff(self, target, a_ref=None, b_ref=None): string: string of output message with diff info """ diff_dct = self.scm.diff(target, a_ref=a_ref, b_ref=b_ref) - msg = "dvc diff for '{}' from {} to {} \n\n".format( + msg = "dvc diff for '{}' from {} to {}\n\n".format( target, diff_dct["a_tag"], diff_dct["b_tag"] ) if diff_dct["equal"]: diff --git a/dvc/scm/base.py b/dvc/scm/base.py index dc2dde69e..e2ae0ae36 100644 --- a/dvc/scm/base.py +++ b/dvc/scm/base.py @@ -17,6 +17,12 @@ class FileNotInRepoError(DvcException): """ +class FileNotInCommitError(DvcException): + """Thrown when trying to find a file/directory that is not + into the specified commit in the repository + """ + + class FileNotInTargetSubdirError(DvcException): """Thrown when trying to place .gitignore for a file that not in the file subdirectory.""" diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index c1c93e00c..872bb247c 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -12,6 +12,7 @@ from dvc.scm.base import ( SCMError, FileNotInRepoError, FileNotInTargetSubdirError, + FileNotInCommitError, ) from dvc.scm.git.tree import GitTree import dvc.logger as logger @@ -264,10 +265,10 @@ class Git(Base): return None, commits diff_obj = a_commit.diff(b_commit, paths=file_name) except (BadName, BadObject) as e: - raise SCMError(e) + raise SCMError("git problem", cause=e) if len(diff_obj) == 0: msg = "Have not found file/directory '{}' in the commits" - raise FileNotInRepoError(msg.format(file_name)) + raise FileNotInCommitError(msg.format(file_name)) return diff_obj[0], commits def _extract_fname(self, is_deleted, blob_obj): diff --git a/tests/test_diff.py b/tests/test_diff.py index 7037ce422..f9dfc960b 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -4,7 +4,7 @@ import os from dvc.main import main -from dvc.scm.base import FileNotInRepoError +from dvc.scm.base import FileNotInCommitError from tests.basic_env import TestDvc @@ -29,9 +29,13 @@ class TestDiff(TestDvc): class TestDiffRepo(TestDiff): def test(self): + new_ref = self.git.head.commit msg = self.dvc.diff(self.new_file, a_ref=self.a_ref) - self.assertTrue("deleted file" in msg) - self.assertTrue(self.new_file in msg) + test_msg = "dvc diff for '{}' from ".format(self.new_file) + test_msg += "{} to {}\n\n".format(new_ref, self.a_ref) + test_msg += "-new_file with md5 25fa8325e4e0eb8180445e42558e60bd\n" + test_msg += "deleted file with size -8 Bytes" + self.assertEqual(test_msg, msg) class TestDiffCmdLine(TestDiff): @@ -66,9 +70,15 @@ class TestDiffDir(TestDvc): class TestDiffDirRepo(TestDiffDir): def test(self): + new_ref = self.git.head.commit msg = self.dvc.diff(self.DATA_DIR, a_ref=self.a_ref) - self.assertTrue("1 file deleted" in msg, msg) - self.assertTrue("size was decreased by " in msg, msg) + test_msg = "dvc diff for '{}' from ".format(self.DATA_DIR) + test_msg += "{} to {}\n\n".format(new_ref, self.a_ref) + test_msg += "-data_dir with md5 d5782f3072167ad3a53ee80b92b30718\n" + test_msg += "+data_dir with md5 bff5a787d16460f32e9f2e62b183b1cc\n\n" + test_msg += "2 files didn't change, 0 files modified, 0 files added, " + test_msg += "1 file deleted, size was decreased by 30 Bytes" + self.assertEqual(msg, test_msg) class TestDiffFileNotFound(TestDiffDir): @@ -77,22 +87,11 @@ class TestDiffFileNotFound(TestDiffDir): self.unknown_file = "unknown_file_" + str(id(self)) def test(self): - with self.assertRaises( - FileNotInRepoError, msg="diff with incorrect arg" - ) as context: + with self.assertRaises(FileNotInCommitError): self.dvc.scm.diff(self.unknown_file, self.a_ref) - self.assertTrue( - "Have not found file/directory" in str(context.exception), - "scm raises with incorrect error message", - ) class TestDiffFileNotFoundRepo(TestDiffFileNotFound): def test(self): - with self.assertRaises( - FileNotInRepoError, msg="diff with incorrect arg" - ) as context: + with self.assertRaises(FileNotInCommitError): self.dvc.diff(self.unknown_file, a_ref=self.a_ref) - self.assertTrue( - "Have not found file/directory" in str(context.exception) - ) From f0225d85cd9963bb2c9a5b055e15da11a184965c Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Tue, 26 Mar 2019 19:14:32 +0600 Subject: [PATCH 09/25] dvc: change logic of getting dvc Getting git snapshots of file/directory using GitTree object --- dvc/command/diff.py | 13 ++++- dvc/repo/diff.py | 113 ++++++++++++++++++++++++++++++---------- dvc/scm/git/__init__.py | 67 ++++++------------------ 3 files changed, 112 insertions(+), 81 deletions(-) diff --git a/dvc/command/diff.py b/dvc/command/diff.py index e7ff8b395..4a784b705 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -38,14 +38,23 @@ def add_parser(subparsers, parent_parser): "diff", parents=[parent_parser], description=description, help=help ) diff_parser.add_argument( - "target", help="Source path to a data file or directory." + "-t", + "--target", + default=None, + help=( + "Source path to a data file or directory. Default None," + "if None shows the diff between all DVC tracked files/directories" + ), ) diff_parser.add_argument( "a_ref", help="Git reference from which diff calculates" ) diff_parser.add_argument( "b_ref", - help="Git reference till which diff calculates", + help=( + "Git reference till which diff calculates, if omitted " + "diff shows the difference between current HEAD and a_ref" + ), nargs="?", default=None, ) diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index 9e0dfb7c0..7b1af6bd6 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -5,8 +5,11 @@ import humanize import inflect -def _extract_tree(self, cache_dir): - lst = self.cache.local.load_dir_cache(cache_dir) +from dvc.scm.base import FileNotInCommitError + + +def _extract_output(self, output): + lst = output.dir_cache return {i["relpath"]: i["md5"] for i in lst} @@ -35,36 +38,74 @@ def _get_tree_changes(self, a_entries, b_entries): return result +def _get_diff_outs(self, diff_dct): + self.tree = diff_dct["a_tree"] + a_outs = {str(out): out for st in self.stages() for out in st.outs} + self.tree = diff_dct["b_tree"] + b_outs = {str(out): out for st in self.stages() for out in st.outs} + outs_paths = set(a_outs.keys()) + outs_paths.update(b_outs.keys()) + results = {} + for path in outs_paths: + if path in a_outs and path in b_outs: + results[path] = { + "a_output": a_outs[path], + "b_output": b_outs[path], + "new_file": False, + "deleted_file": False, + # possible drawback: regular file ->directory movement + "is_dir": a_outs[path].is_dir_cache, + } + elif path in a_outs: + results[path] = { + "a_output": a_outs[path], + "new_file": False, + "deleted_file": True, + "is_dir": a_outs[path].is_dir_cache, + } + else: + results[path] = { + "b_output": b_outs[path], + "new_file": True, + "deleted_file": False, + "is_dir": b_outs[path].is_dir_cache, + } + return results + + def _diff_dir(self, target, diff_dct): engine = inflect.engine() msg = "" a_entries, b_entries = {}, {} if not diff_dct["new_file"]: msg += "-{} with md5 {}\n".format( - target, diff_dct["a_fname"].split(".dir")[0] + target, diff_dct["a_output"].checksum ) - a_entries = _extract_tree(self, diff_dct["a_fname"]) + a_entries = _extract_output(self, diff_dct["a_output"]) if not diff_dct["deleted_file"]: msg += "+{} with md5 {}\n".format( - target, diff_dct["b_fname"].split(".dir")[0] + target, diff_dct["b_output"].checksum ) - b_entries = _extract_tree(self, diff_dct["b_fname"]) + b_entries = _extract_output(self, diff_dct["b_output"]) msg += "\n" result = _get_tree_changes(self, a_entries, b_entries) msg += f"{result['ident']} " + engine.plural("file", result["ident"]) - msg += " didn't change, " + msg += " not changed, " msg += f"{result['changes']} " + engine.plural("file", result["changes"]) msg += " modified, " msg += f"{result['new']} " + engine.plural("file", result["new"]) msg += " added, " msg += f"{result['del']} " + engine.plural("file", result["del"]) msg += " deleted, " - msg += "size was " + "increased" * (result["diff_sign"] != "-") - msg += ( - "decreased" * (result["diff_sign"] == "-") - + " by " - + result["diff_size"] - ) + if result["diff_size"] == "0 Bytes": + msg += "size wasn't changed" + else: + msg += "size was " + "increased" * (result["diff_sign"] != "-") + msg += ( + "decreased" * (result["diff_sign"] == "-") + + " by " + + result["diff_size"] + ) return msg @@ -73,16 +114,20 @@ def _diff_file(self, target, diff_dct): size = 0 if not diff_dct["new_file"]: msg += "-{} with md5 {}\n".format( - target, diff_dct["a_fname"].split(".dir")[0] + target, diff_dct["a_output"].checksum + ) + size -= os.path.getsize( + self.cache.local.get(diff_dct["a_output"].checksum) ) - size -= os.path.getsize(self.cache.local.get(diff_dct["a_fname"])) if not diff_dct["deleted_file"]: - msg += "+{} with md5 {}\n".format( - target, diff_dct["b_fname"].split(".dir")[0] + msg += "+{} with md5 {}\n\n".format( + target, diff_dct["b_output"].checksum + ) + size += os.path.getsize( + self.cache.local.get(diff_dct["b_output"].checksum) ) - size += os.path.getsize(self.cache.local.get(diff_dct["b_fname"])) if not diff_dct["new_file"] and not diff_dct["deleted_file"] and size == 0: - msg += "file did not change\n" + msg += "file was not changed" elif diff_dct["new_file"]: msg += "added file with size {}".format(humanize.naturalsize(size)) elif diff_dct["deleted_file"]: @@ -93,6 +138,13 @@ def _diff_file(self, target, diff_dct): return msg +def _diff_royal(self, target, diff_dct): + msg = "diff for " + target + "\n" + if diff_dct["is_dir"]: + return _diff_dir(self, target, diff_dct) + return msg + _diff_file(self, target, diff_dct) + + def diff(self, target, a_ref=None, b_ref=None): """Gerenates diff message string output @@ -104,16 +156,23 @@ def diff(self, target, a_ref=None, b_ref=None): Returns: string: string of output message with diff info """ - diff_dct = self.scm.diff(target, a_ref=a_ref, b_ref=b_ref) - msg = "dvc diff for '{}' from {} to {}\n\n".format( - target, diff_dct["a_tag"], diff_dct["b_tag"] + diff_dct = self.scm.get_diff_trees(a_ref, b_ref=b_ref) + msg = "dvc diff from {} to {}\n\n".format( + diff_dct["a_ref"], diff_dct["b_ref"] ) if diff_dct["equal"]: return msg - if diff_dct["is_dir"]: - dir_info = _diff_dir(self, target, diff_dct) - msg += dir_info + diff_outs = _get_diff_outs(self, diff_dct) + if target is None: + for path in diff_outs: + info_msg = _diff_royal(self, path, diff_outs[path]) + msg += info_msg + msg += "\n\n" + msg = msg[:-2] + elif target in diff_outs: + info_msg = _diff_royal(self, target, diff_outs[target]) + msg += info_msg else: - file_info = _diff_file(self, target, diff_dct) - msg += file_info + msg = "Have not found file/directory '{}' in the commits" + raise FileNotInCommitError(msg.format(target)) return msg diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 872bb247c..39b153eb2 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -3,7 +3,6 @@ from __future__ import unicode_literals import os -import re from dvc.utils.compat import str, open from dvc.utils import fix_env @@ -12,7 +11,6 @@ from dvc.scm.base import ( SCMError, FileNotInRepoError, FileNotInTargetSubdirError, - FileNotInCommitError, ) from dvc.scm.git.tree import GitTree import dvc.logger as logger @@ -238,12 +236,11 @@ class Git(Base): def get_tree(self, rev): return GitTree(self.git, rev) - def _get_diff_obj(self, file_name, a_ref, **kwargs): + def _get_diff_trees(self, a_ref, **kwargs): from gitdb.exc import BadObject, BadName - file_name = os.path.normpath(file_name) - file_name += ".dvc" + trees = {"a_tree": None, "b_tree": None} commits = [] try: if "b_ref" in kwargs and kwargs["b_ref"] is not None: @@ -252,7 +249,6 @@ class Git(Base): b_commit = self.git.commit(b_ref) commits.append(str(a_commit)) commits.append(str(b_commit)) - diff_obj = a_commit.diff(b_commit, paths=file_name) # other way of getting the diff between commits : # self.git.diff(a_commit, b_commit) returns the diff string # by reading the files in a, b commits and computing the diff @@ -261,63 +257,30 @@ class Git(Base): b_commit = self.git.commit(a_ref) commits.append(str(a_commit)) commits.append(str(b_commit)) - if a_commit == b_commit: - return None, commits - diff_obj = a_commit.diff(b_commit, paths=file_name) + trees["a_tree"] = self.get_tree(commits[0]) + trees["b_tree"] = self.get_tree(commits[1]) except (BadName, BadObject) as e: raise SCMError("git problem", cause=e) - if len(diff_obj) == 0: - msg = "Have not found file/directory '{}' in the commits" - raise FileNotInCommitError(msg.format(file_name)) - return diff_obj[0], commits + return trees, commits - def _extract_fname(self, is_deleted, blob_obj): - if not is_deleted: - s1 = blob_obj.data_stream.read().decode("utf-8") - a_hash, a_fname = re.findall(r"md5:\s([a-fA-F\d]{32}[.dir]*)", s1) - return a_hash, a_fname - else: - return None, None - - def diff(self, file_name, a_ref, **kwargs): - """Method for diff between two git tag commits + def get_diff_trees(self, a_ref, **kwargs): + """Method for getting two repo trees between two git tag commits returns the dvc hash names of changed file/directory Args: - file_name(str) - file/directory to perform diff of a_ref(str) - git reference - b_ref(str) - optional second git reference + b_ref(str) - optional second git reference, default None Returns: - dict - dictionary with keys: (new_file, a_file_name, b_file_name) + dict - dictionary with keys: (a_tree, b_tree, a_ref, b_ref, equal) """ diff_dct = {"equal": False} - diff_obj, commit_refs = self._get_diff_obj(file_name, a_ref, **kwargs) - diff_dct["a_tag"] = commit_refs[0] - diff_dct["b_tag"] = commit_refs[1] - if diff_obj is None: + trees, commit_refs = self._get_diff_trees(a_ref, **kwargs) + diff_dct["a_ref"] = commit_refs[0] + diff_dct["b_ref"] = commit_refs[1] + if commit_refs[0] == commit_refs[1]: diff_dct["equal"] = True return diff_dct - if not diff_obj.a_path: - msg = ( - "file path {} doesn't exist maybe you forgot to add dataset" - " to dvc?" - ) - raise SCMError(msg.format(file_name)) - # function doesn't support an indicating of moved of a file/directory - diff_dct["new_file"] = diff_obj.new_file - diff_dct["deleted_file"] = diff_obj.deleted_file - diff_dct["a_hash"], diff_dct["a_fname"] = self._extract_fname( - diff_dct["new_file"], diff_obj.a_blob - ) - diff_dct["b_hash"], diff_dct["b_fname"] = self._extract_fname( - diff_dct["deleted_file"], diff_obj.b_blob - ) - actual_path = ( - diff_dct["b_fname"] if diff_dct["b_fname"] else diff_dct["a_fname"] - ) - diff_dct["is_dir"] = actual_path.endswith(".dir") - diff_dct["file_path"] = ( - diff_obj.b_path if diff_obj.b_path else diff_obj.a_path - ) + diff_dct["a_tree"] = trees["a_tree"] + diff_dct["b_tree"] = trees["b_tree"] return diff_dct From f0a9255ddbc0857c7ea4ce5eb9b95daf6bf8a92f Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Wed, 27 Mar 2019 12:59:42 +0600 Subject: [PATCH 10/25] tests: fix test_diff with new logic --- tests/test_diff.py | 42 +++++++++++++++++++----------------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/tests/test_diff.py b/tests/test_diff.py index f9dfc960b..935697c0b 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -22,17 +22,18 @@ class TestDiff(TestDvc): self.git.index.commit("adds new_file") def test(self): - out = self.dvc.scm.diff(self.new_file, self.a_ref) - self.assertTrue(out["deleted_file"]) - self.assertEqual("new_file.dvc", out["file_path"]) + out = self.dvc.scm.get_diff_trees(self.a_ref) + self.assertFalse(out["equal"]) + self.assertEqual(str(self.a_ref), out["b_ref"]) + self.assertEqual(str(self.git.head.commit), out["a_ref"]) class TestDiffRepo(TestDiff): def test(self): new_ref = self.git.head.commit msg = self.dvc.diff(self.new_file, a_ref=self.a_ref) - test_msg = "dvc diff for '{}' from ".format(self.new_file) - test_msg += "{} to {}\n\n".format(new_ref, self.a_ref) + test_msg = "dvc diff from {} to {}\n\n".format(new_ref, self.a_ref) + test_msg += "diff for {}\n".format(self.new_file) test_msg += "-new_file with md5 25fa8325e4e0eb8180445e42558e60bd\n" test_msg += "deleted file with size -8 Bytes" self.assertEqual(test_msg, msg) @@ -59,26 +60,27 @@ class TestDiffDir(TestDvc): self.git.index.commit(message="adds data_dir with new_file") def test(self): - out = self.dvc.scm.diff(self.DATA_DIR, self.a_ref) - self.assertFalse(out["deleted_file"], "new file added checker failed") - self.assertEqual( - self.DATA_DIR + ".dvc", - out["file_path"], - "path to file/directory is incorrect", - ) + out = self.dvc.scm.get_diff_trees(self.a_ref) + self.assertFalse(out["equal"]) + self.assertEqual(str(self.a_ref), out["b_ref"]) + self.assertEqual(str(self.git.head.commit), out["a_ref"]) class TestDiffDirRepo(TestDiffDir): def test(self): new_ref = self.git.head.commit msg = self.dvc.diff(self.DATA_DIR, a_ref=self.a_ref) - test_msg = "dvc diff for '{}' from ".format(self.DATA_DIR) + test_msg = "dvc diff from ".format(self.DATA_DIR) test_msg += "{} to {}\n\n".format(new_ref, self.a_ref) - test_msg += "-data_dir with md5 d5782f3072167ad3a53ee80b92b30718\n" - test_msg += "+data_dir with md5 bff5a787d16460f32e9f2e62b183b1cc\n\n" - test_msg += "2 files didn't change, 0 files modified, 0 files added, " + test_msg += "-data_dir with md5 d5782f3072167ad3a53ee80b92b30718.dir\n" + test_msg += ( + "+data_dir with md5 bff5a787d16460f32e9f2e62b183b1cc.dir\n\n" + ) + test_msg += "2 files not changed, 0 files modified, 0 files added, " test_msg += "1 file deleted, size was decreased by 30 Bytes" - self.assertEqual(msg, test_msg) + self.assertEqual( + msg, test_msg, msg="msg is %s test_msg is %s" % (msg, test_msg) + ) class TestDiffFileNotFound(TestDiffDir): @@ -86,12 +88,6 @@ class TestDiffFileNotFound(TestDiffDir): super(TestDiffFileNotFound, self).setUp() self.unknown_file = "unknown_file_" + str(id(self)) - def test(self): - with self.assertRaises(FileNotInCommitError): - self.dvc.scm.diff(self.unknown_file, self.a_ref) - - -class TestDiffFileNotFoundRepo(TestDiffFileNotFound): def test(self): with self.assertRaises(FileNotInCommitError): self.dvc.diff(self.unknown_file, a_ref=self.a_ref) From 49dead0474c84dddc114caabf5bc99c8fe01c8a6 Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Wed, 27 Mar 2019 14:50:53 +0600 Subject: [PATCH 11/25] tests: add CmdDiff command line parsing test --- tests/test_diff.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/test_diff.py b/tests/test_diff.py index 935697c0b..8e823b815 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -1,5 +1,7 @@ from __future__ import unicode_literals + import os +from mock import patch from dvc.main import main @@ -41,8 +43,11 @@ class TestDiffRepo(TestDiff): class TestDiffCmdLine(TestDiff): def test(self): - ret = main(["diff", self.new_file, str(self.a_ref)]) - self.assertEqual(ret, 0) + with patch("dvc.repo.Repo", config="testing") as MockRepo: + MockRepo.return_value.diff.return_value = "testing" + MockRepo.return_value.diff.return_value = "testing" + ret = main(["diff", "-t", self.new_file, str(self.a_ref)]) + self.assertEqual(ret, 0) class TestDiffDir(TestDvc): From 23b420554f1bdf6e55b55c5147b718d72d1acf09 Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Wed, 27 Mar 2019 16:04:48 +0600 Subject: [PATCH 12/25] dvc: change help output of diff command and minor fixes --- dvc/command/diff.py | 18 +++++++++--------- dvc/repo/diff.py | 18 +++++++++++++----- dvc/scm/git/__init__.py | 9 ++++----- dvc/utils/collections.py | 6 ++++++ setup.py | 2 ++ tests/test_diff.py | 3 ++- 6 files changed, 36 insertions(+), 20 deletions(-) create mode 100644 dvc/utils/collections.py diff --git a/dvc/command/diff.py b/dvc/command/diff.py index 4a784b705..c48905fe5 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals import dvc.logger as logger from dvc.exceptions import DvcException from dvc.command.base import CmdBase +from dvc.utils.collections import compact class CmdDiff(CmdBase): @@ -16,20 +17,18 @@ class CmdDiff(CmdBase): ) self._show(msg) except DvcException: - msg = "failed to get diff " - if self.args.target: - msg += self.args.target + " " - if self.args.a_ref: - msg += self.args.a_ref + " " - if self.args.b_ref: - msg += self.args.b_ref + " " + msg = "failed to get 'diff " + msg += " ".join( + compact([self.args.target, self.args.a_ref, self.args.b_ref]) + ) + msg += "'" logger.error(msg) return 0 def add_parser(subparsers, parent_parser): description = ( - "show diff of a data file or a directory that " + "Show diff of a data file or a directory that " "is under DVC control. Some basic statistics " "summary, how many files were deleted/changed." ) @@ -43,7 +42,8 @@ def add_parser(subparsers, parent_parser): default=None, help=( "Source path to a data file or directory. Default None," - "if None shows the diff between all DVC tracked files/directories" + "If not specified, compares all files and directories " + "that are under DVC control in the current working space." ), ) diff_parser.add_argument( diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index 7b1af6bd6..cd34a8cba 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -8,7 +8,15 @@ import inflect from dvc.scm.base import FileNotInCommitError -def _extract_output(self, output): +def _extract_dir(self, output): + """Extract the content of dvc tree file + Args: + self(object) - Repo class instance + output(object) - OutputLOCAL class instance + Returns: + dict - dictionary with keys - paths to file in .dvc/cache + values -checksums for that files + """ lst = output.dir_cache return {i["relpath"]: i["md5"] for i in lst} @@ -81,12 +89,12 @@ def _diff_dir(self, target, diff_dct): msg += "-{} with md5 {}\n".format( target, diff_dct["a_output"].checksum ) - a_entries = _extract_output(self, diff_dct["a_output"]) + a_entries = _extract_dir(self, diff_dct["a_output"]) if not diff_dct["deleted_file"]: msg += "+{} with md5 {}\n".format( target, diff_dct["b_output"].checksum ) - b_entries = _extract_output(self, diff_dct["b_output"]) + b_entries = _extract_dir(self, diff_dct["b_output"]) msg += "\n" result = _get_tree_changes(self, a_entries, b_entries) msg += f"{result['ident']} " + engine.plural("file", result["ident"]) @@ -139,9 +147,9 @@ def _diff_file(self, target, diff_dct): def _diff_royal(self, target, diff_dct): - msg = "diff for " + target + "\n" + msg = "diff for '" + target + "'\n" if diff_dct["is_dir"]: - return _diff_dir(self, target, diff_dct) + return msg + _diff_dir(self, target, diff_dct) return msg + _diff_file(self, target, diff_dct) diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 39b153eb2..f850aae68 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -236,15 +236,14 @@ class Git(Base): def get_tree(self, rev): return GitTree(self.git, rev) - def _get_diff_trees(self, a_ref, **kwargs): + def _get_diff_trees(self, a_ref, b_ref): from gitdb.exc import BadObject, BadName trees = {"a_tree": None, "b_tree": None} commits = [] try: - if "b_ref" in kwargs and kwargs["b_ref"] is not None: - b_ref = kwargs["b_ref"] + if b_ref is not None: a_commit = self.git.commit(a_ref) b_commit = self.git.commit(b_ref) commits.append(str(a_commit)) @@ -263,7 +262,7 @@ class Git(Base): raise SCMError("git problem", cause=e) return trees, commits - def get_diff_trees(self, a_ref, **kwargs): + def get_diff_trees(self, a_ref, b_ref=None): """Method for getting two repo trees between two git tag commits returns the dvc hash names of changed file/directory @@ -275,7 +274,7 @@ class Git(Base): dict - dictionary with keys: (a_tree, b_tree, a_ref, b_ref, equal) """ diff_dct = {"equal": False} - trees, commit_refs = self._get_diff_trees(a_ref, **kwargs) + trees, commit_refs = self._get_diff_trees(a_ref, b_ref) diff_dct["a_ref"] = commit_refs[0] diff_dct["b_ref"] = commit_refs[1] if commit_refs[0] == commit_refs[1]: diff --git a/dvc/utils/collections.py b/dvc/utils/collections.py new file mode 100644 index 000000000..46d8c9d44 --- /dev/null +++ b/dvc/utils/collections.py @@ -0,0 +1,6 @@ +from __future__ import unicode_literals + + +# just simple check for Nones and emtpy strings +def compact(args): + return list(filter(bool, args)) diff --git a/setup.py b/setup.py index 07fe236ba..e01c045d5 100644 --- a/setup.py +++ b/setup.py @@ -23,6 +23,8 @@ install_requires = [ "distro>=1.3.0", "appdirs>=1.4.3", "treelib>=1.5.5", + "inflect>=2.1.0", + "humanize>=0.5.1", ] # Extra dependencies for remote integrations diff --git a/tests/test_diff.py b/tests/test_diff.py index 8e823b815..f1ce66993 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -35,7 +35,7 @@ class TestDiffRepo(TestDiff): new_ref = self.git.head.commit msg = self.dvc.diff(self.new_file, a_ref=self.a_ref) test_msg = "dvc diff from {} to {}\n\n".format(new_ref, self.a_ref) - test_msg += "diff for {}\n".format(self.new_file) + test_msg += "diff for '{}'\n".format(self.new_file) test_msg += "-new_file with md5 25fa8325e4e0eb8180445e42558e60bd\n" test_msg += "deleted file with size -8 Bytes" self.assertEqual(test_msg, msg) @@ -77,6 +77,7 @@ class TestDiffDirRepo(TestDiffDir): msg = self.dvc.diff(self.DATA_DIR, a_ref=self.a_ref) test_msg = "dvc diff from ".format(self.DATA_DIR) test_msg += "{} to {}\n\n".format(new_ref, self.a_ref) + test_msg += "diff for '{}'\n".format(self.DATA_DIR) test_msg += "-data_dir with md5 d5782f3072167ad3a53ee80b92b30718.dir\n" test_msg += ( "+data_dir with md5 bff5a787d16460f32e9f2e62b183b1cc.dir\n\n" From 44e2ff7f839dd51d00701be00e173fcf0deaf2e0 Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Wed, 27 Mar 2019 19:35:13 +0600 Subject: [PATCH 13/25] repo: change the returning value of repo.diff repo.diff now returns list of dictionaries for each file/directory change --- dvc/command/diff.py | 67 +++++++++++++++++++++++++++++++- dvc/repo/diff.py | 95 +++++++++++++++------------------------------ tests/test_diff.py | 67 +++++++++++++++++++------------- 3 files changed, 138 insertions(+), 91 deletions(-) diff --git a/dvc/command/diff.py b/dvc/command/diff.py index c48905fe5..d93147a34 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -1,5 +1,9 @@ from __future__ import unicode_literals +import humanize +import inflect + + import dvc.logger as logger from dvc.exceptions import DvcException from dvc.command.base import CmdBase @@ -7,7 +11,68 @@ from dvc.utils.collections import compact class CmdDiff(CmdBase): - def _show(self, msg): + def _print_size(self, size): + if size < 0: + change = "decreased by {}" + elif size > 0: + change = "increased by {}" + else: + change = "not changed" + natur_size = humanize.naturalsize(abs(size)) + return change.format(natur_size) + + def _show(self, diff_dct): + engine = inflect.engine() + msg = "dvc diff from {} to {}".format( + diff_dct["a_ref"], diff_dct["b_ref"] + ) + if diff_dct.get("equal"): + logger.info(msg) + return + for dct in diff_dct["diffs"]: + msg += "\n\ndiff for '{}' \n".format(dct["target"]) + if dct.get("old_file"): + msg += "-{} with md5 {}\n".format( + dct["old_file"], dct["old_checksum"] + ) + if dct.get("new_file"): + msg += "+{} with md5 {}\n".format( + dct["new_file"], dct["new_checksum"] + ) + msg += "\n" + if dct.get("is_dir"): + msg += "{} {} not changed, ".format( + dct["ident"], engine.plural("file", dct["ident"]) + ) + msg += "{} {} modified, ".format( + dct["changes"], engine.plural("file", dct["changes"]) + ) + msg += "{} {} added, ".format( + dct["new"], engine.plural("file", dct["new"]) + ) + msg += "{} {} deleted, ".format( + dct["del"], engine.plural("file", dct["del"]) + ) + msg += "size was {}".format(self._print_size(dct["size_diff"])) + else: + if ( + dct.get("old_file") + and dct.get("new_file") + and dct["size_diff"] == 0 + ): + msg += "file size was not changed" + elif dct.get("new_file"): + msg += "added file with size {}".format( + humanize.naturalsize(dct["size_diff"]) + ) + elif dct.get("old_file"): + msg += "deleted file with size {}".format( + humanize.naturalsize(abs(dct["size_diff"])) + ) + else: + msg += "file was modified, file size {}".format( + self._print_size(dct["size_diff"]) + ) logger.info(msg) def run(self): diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index cd34a8cba..bc7228ebb 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -1,8 +1,6 @@ from __future__ import unicode_literals import os -import humanize -import inflect from dvc.scm.base import FileNotInCommitError @@ -41,8 +39,7 @@ def _get_tree_changes(self, a_entries, b_entries): else: result["new"] += 1 diff_size += os.path.getsize(self.cache.local.get(b_entries[key])) - result["diff_size"] = humanize.naturalsize(abs(diff_size)) - result["diff_sign"] = "-" if diff_size < 0 else "" + result["size_diff"] = diff_size return result @@ -82,75 +79,46 @@ def _get_diff_outs(self, diff_dct): def _diff_dir(self, target, diff_dct): - engine = inflect.engine() - msg = "" + result = {"target": target} + result["is_dir"] = True a_entries, b_entries = {}, {} if not diff_dct["new_file"]: - msg += "-{} with md5 {}\n".format( - target, diff_dct["a_output"].checksum - ) + result["old_file"] = target + result["old_checksum"] = diff_dct["a_output"].checksum a_entries = _extract_dir(self, diff_dct["a_output"]) if not diff_dct["deleted_file"]: - msg += "+{} with md5 {}\n".format( - target, diff_dct["b_output"].checksum - ) + result["new_file"] = target + result["new_checksum"] = diff_dct["b_output"].checksum b_entries = _extract_dir(self, diff_dct["b_output"]) - msg += "\n" - result = _get_tree_changes(self, a_entries, b_entries) - msg += f"{result['ident']} " + engine.plural("file", result["ident"]) - msg += " not changed, " - msg += f"{result['changes']} " + engine.plural("file", result["changes"]) - msg += " modified, " - msg += f"{result['new']} " + engine.plural("file", result["new"]) - msg += " added, " - msg += f"{result['del']} " + engine.plural("file", result["del"]) - msg += " deleted, " - if result["diff_size"] == "0 Bytes": - msg += "size wasn't changed" - else: - msg += "size was " + "increased" * (result["diff_sign"] != "-") - msg += ( - "decreased" * (result["diff_sign"] == "-") - + " by " - + result["diff_size"] - ) - return msg + result.update(_get_tree_changes(self, a_entries, b_entries)) + return result def _diff_file(self, target, diff_dct): - msg = "" + result = {"target": target} size = 0 if not diff_dct["new_file"]: - msg += "-{} with md5 {}\n".format( - target, diff_dct["a_output"].checksum - ) + result["old_file"] = target + result["old_checksum"] = diff_dct["a_output"].checksum size -= os.path.getsize( self.cache.local.get(diff_dct["a_output"].checksum) ) + print("first", size) if not diff_dct["deleted_file"]: - msg += "+{} with md5 {}\n\n".format( - target, diff_dct["b_output"].checksum - ) + result["new_file"] = target + result["new_checksum"] = diff_dct["b_output"].checksum size += os.path.getsize( self.cache.local.get(diff_dct["b_output"].checksum) ) - if not diff_dct["new_file"] and not diff_dct["deleted_file"] and size == 0: - msg += "file was not changed" - elif diff_dct["new_file"]: - msg += "added file with size {}".format(humanize.naturalsize(size)) - elif diff_dct["deleted_file"]: - msg += "deleted file with size {}".format(humanize.naturalsize(size)) - else: - msg += "file was modified, file size " + "increased" * (size >= 0) - msg += "decreased" * (size < 0) + " by " + humanize.naturalsize(size) - return msg + print("second", size) + result["size_diff"] = size + return result def _diff_royal(self, target, diff_dct): - msg = "diff for '" + target + "'\n" if diff_dct["is_dir"]: - return msg + _diff_dir(self, target, diff_dct) - return msg + _diff_file(self, target, diff_dct) + return _diff_dir(self, target, diff_dct) + return _diff_file(self, target, diff_dct) def diff(self, target, a_ref=None, b_ref=None): @@ -164,23 +132,22 @@ def diff(self, target, a_ref=None, b_ref=None): Returns: string: string of output message with diff info """ + result = {} diff_dct = self.scm.get_diff_trees(a_ref, b_ref=b_ref) - msg = "dvc diff from {} to {}\n\n".format( - diff_dct["a_ref"], diff_dct["b_ref"] - ) + result["a_ref"] = diff_dct["a_ref"] + result["b_ref"] = diff_dct["b_ref"] if diff_dct["equal"]: - return msg + result["equal"] = True + return result + result["diffs"] = [] diff_outs = _get_diff_outs(self, diff_dct) if target is None: - for path in diff_outs: - info_msg = _diff_royal(self, path, diff_outs[path]) - msg += info_msg - msg += "\n\n" - msg = msg[:-2] + result["diffs"] = [ + _diff_royal(self, path, diff_outs[path]) for path in diff_outs + ] elif target in diff_outs: - info_msg = _diff_royal(self, target, diff_outs[target]) - msg += info_msg + result["diffs"] = [_diff_royal(self, target, diff_outs[target])] else: msg = "Have not found file/directory '{}' in the commits" raise FileNotInCommitError(msg.format(target)) - return msg + return result diff --git a/tests/test_diff.py b/tests/test_diff.py index f1ce66993..434e5d94a 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -32,22 +32,30 @@ class TestDiff(TestDvc): class TestDiffRepo(TestDiff): def test(self): - new_ref = self.git.head.commit - msg = self.dvc.diff(self.new_file, a_ref=self.a_ref) - test_msg = "dvc diff from {} to {}\n\n".format(new_ref, self.a_ref) - test_msg += "diff for '{}'\n".format(self.new_file) - test_msg += "-new_file with md5 25fa8325e4e0eb8180445e42558e60bd\n" - test_msg += "deleted file with size -8 Bytes" - self.assertEqual(test_msg, msg) + result = self.dvc.diff(self.new_file, a_ref=self.a_ref) + test_dct = { + "b_ref": str(self.a_ref), + "a_ref": str(self.git.head.commit), + "diffs": [ + { + "target": self.new_file, + "old_file": self.new_file, + "old_checksum": "25fa8325e4e0eb8180445e42558e60bd", + "size_diff": -8, + } + ], + } + self.assertEqual(test_dct, result) class TestDiffCmdLine(TestDiff): def test(self): - with patch("dvc.repo.Repo", config="testing") as MockRepo: - MockRepo.return_value.diff.return_value = "testing" - MockRepo.return_value.diff.return_value = "testing" - ret = main(["diff", "-t", self.new_file, str(self.a_ref)]) - self.assertEqual(ret, 0) + with patch("dvc.cli.diff.CmdDiff._show", autospec=True): + with patch("dvc.repo.Repo", config="testing") as MockRepo: + MockRepo.return_value.diff.return_value = "testing" + MockRepo.return_value.diff.return_value = "testing" + ret = main(["diff", "-t", self.new_file, str(self.a_ref)]) + self.assertEqual(ret, 0) class TestDiffDir(TestDvc): @@ -73,20 +81,27 @@ class TestDiffDir(TestDvc): class TestDiffDirRepo(TestDiffDir): def test(self): - new_ref = self.git.head.commit - msg = self.dvc.diff(self.DATA_DIR, a_ref=self.a_ref) - test_msg = "dvc diff from ".format(self.DATA_DIR) - test_msg += "{} to {}\n\n".format(new_ref, self.a_ref) - test_msg += "diff for '{}'\n".format(self.DATA_DIR) - test_msg += "-data_dir with md5 d5782f3072167ad3a53ee80b92b30718.dir\n" - test_msg += ( - "+data_dir with md5 bff5a787d16460f32e9f2e62b183b1cc.dir\n\n" - ) - test_msg += "2 files not changed, 0 files modified, 0 files added, " - test_msg += "1 file deleted, size was decreased by 30 Bytes" - self.assertEqual( - msg, test_msg, msg="msg is %s test_msg is %s" % (msg, test_msg) - ) + result = self.dvc.diff(self.DATA_DIR, a_ref=self.a_ref) + test_dct = { + "b_ref": str(self.a_ref), + "a_ref": str(self.git.head.commit), + "diffs": [ + { + "changes": 0, + "del": 1, + "ident": 2, + "moves": 0, + "new": 0, + "target": self.DATA_DIR, + "old_file": self.DATA_DIR, + "old_checksum": "d5782f3072167ad3a53ee80b92b30718.dir", + "new_file": self.DATA_DIR, + "new_checksum": "bff5a787d16460f32e9f2e62b183b1cc.dir", + "size_diff": -30, + } + ], + } + self.assertEqual(test_dct, result) class TestDiffFileNotFound(TestDiffDir): From 5f68b8e5a07e70c86577da8d29936b87c838b358 Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Mon, 1 Apr 2019 17:19:32 +0600 Subject: [PATCH 14/25] dvc: replace keys in dictionary with macros in dvc/repo/diff.py --- dvc/command/diff.py | 93 +++++++++++++++++++--------------- dvc/repo/diff.py | 109 ++++++++++++++++++++++------------------ dvc/scm/base.py | 23 ++++++++- dvc/scm/git/__init__.py | 26 +++++----- tests/test_diff.py | 64 ++++++++++++----------- 5 files changed, 183 insertions(+), 132 deletions(-) diff --git a/dvc/command/diff.py b/dvc/command/diff.py index d93147a34..5ff7be489 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -8,6 +8,7 @@ import dvc.logger as logger from dvc.exceptions import DvcException from dvc.command.base import CmdBase from dvc.utils.collections import compact +import dvc.scm.base as git class CmdDiff(CmdBase): @@ -24,55 +25,64 @@ class CmdDiff(CmdBase): def _show(self, diff_dct): engine = inflect.engine() msg = "dvc diff from {} to {}".format( - diff_dct["a_ref"], diff_dct["b_ref"] + diff_dct[git.DIFF_A_REF], diff_dct[git.DIFF_B_REF] ) - if diff_dct.get("equal"): + if diff_dct.get(git.DIFF_EQUAL): logger.info(msg) return - for dct in diff_dct["diffs"]: - msg += "\n\ndiff for '{}' \n".format(dct["target"]) - if dct.get("old_file"): + for dct in diff_dct[git.DIFF_LIST]: + msg += "\n\ndiff for '{}' \n".format(dct[git.DIFF_TARGET]) + if dct.get(git.DIFF_OLD_FILE): msg += "-{} with md5 {}\n".format( - dct["old_file"], dct["old_checksum"] + dct[git.DIFF_OLD_FILE], dct[git.DIFF_OLD_CHECKSUM] ) - if dct.get("new_file"): + if dct.get(git.DIFF_NEW_FILE): msg += "+{} with md5 {}\n".format( - dct["new_file"], dct["new_checksum"] + dct[git.DIFF_NEW_FILE], dct[git.DIFF_NEW_CHECKSUM] ) msg += "\n" - if dct.get("is_dir"): - msg += "{} {} not changed, ".format( - dct["ident"], engine.plural("file", dct["ident"]) - ) - msg += "{} {} modified, ".format( - dct["changes"], engine.plural("file", dct["changes"]) - ) - msg += "{} {} added, ".format( - dct["new"], engine.plural("file", dct["new"]) - ) - msg += "{} {} deleted, ".format( - dct["del"], engine.plural("file", dct["del"]) - ) - msg += "size was {}".format(self._print_size(dct["size_diff"])) - else: - if ( - dct.get("old_file") - and dct.get("new_file") - and dct["size_diff"] == 0 - ): - msg += "file size was not changed" - elif dct.get("new_file"): - msg += "added file with size {}".format( - humanize.naturalsize(dct["size_diff"]) + if dct[git.DIFF_SIZE] != git.DIFF_SIZE_UNKNOWN: + if dct.get("is_dir"): + msg += "{} {} not changed, ".format( + dct[git.DIFF_IDENT], + engine.plural("file", dct[git.DIFF_IDENT]), ) - elif dct.get("old_file"): - msg += "deleted file with size {}".format( - humanize.naturalsize(abs(dct["size_diff"])) + msg += "{} {} modified, ".format( + dct[git.DIFF_CHANGE], + engine.plural("file", dct[git.DIFF_CHANGE]), + ) + msg += "{} {} added, ".format( + dct[git.DIFF_NEW], + engine.plural("file", dct[git.DIFF_NEW]), + ) + msg += "{} {} deleted, ".format( + dct[git.DIFF_DEL], + engine.plural("file", dct[git.DIFF_DEL]), + ) + msg += "size was {}".format( + self._print_size(dct[git.DIFF_SIZE]) ) else: - msg += "file was modified, file size {}".format( - self._print_size(dct["size_diff"]) - ) + if ( + dct.get(git.DIFF_OLD_FILE) + and dct.get(git.DIFF_NEW_FILE) + and dct[git.DIFF_SIZE] == 0 + ): + msg += "file size was not changed" + elif dct.get(git.DIFF_NEW_FILE): + msg += "added file with size {}".format( + humanize.naturalsize(dct[git.DIFF_SIZE]) + ) + elif dct.get(git.DIFF_OLD_FILE): + msg += "deleted file with size {}".format( + humanize.naturalsize(abs(dct[git.DIFF_SIZE])) + ) + else: + msg += "file was modified, file size {}".format( + self._print_size(dct[git.DIFF_SIZE]) + ) + else: + msg += "size is ?" logger.info(msg) def run(self): @@ -82,12 +92,13 @@ class CmdDiff(CmdBase): ) self._show(msg) except DvcException: - msg = "failed to get 'diff " - msg += " ".join( + msg = "failed to get 'diff {}'" + args = " ".join( compact([self.args.target, self.args.a_ref, self.args.b_ref]) ) - msg += "'" + msg = msg.format(args) logger.error(msg) + return 1 return 0 diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index bc7228ebb..85f4601bd 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -4,6 +4,7 @@ import os from dvc.scm.base import FileNotInCommitError +import dvc.scm.base as git def _extract_dir(self, output): @@ -20,33 +21,39 @@ def _extract_dir(self, output): def _get_tree_changes(self, a_entries, b_entries): - result = {"del": 0, "ident": 0, "changes": 0, "new": 0, "moves": 0} + result = { + git.DIFF_DEL: 0, + git.DIFF_IDENT: 0, + git.DIFF_CHANGE: 0, + git.DIFF_NEW: 0, + git.DIFF_MOVE: 0, + } keys = set(a_entries.keys()) keys.update(b_entries.keys()) diff_size = 0 for key in keys: if key in a_entries and key in b_entries: if a_entries[key] == b_entries[key]: - result["ident"] += 1 + result[git.DIFF_IDENT] += 1 else: - result["changes"] += 1 + result[git.DIFF_CHANGE] += 1 diff_size += os.path.getsize( self.cache.local.get(b_entries[key]) ) - os.path.getsize(self.cache.local.get(a_entries[key])) elif key in a_entries: - result["del"] += 1 + result[git.DIFF_DEL] += 1 diff_size -= os.path.getsize(self.cache.local.get(a_entries[key])) else: - result["new"] += 1 + result[git.DIFF_NEW] += 1 diff_size += os.path.getsize(self.cache.local.get(b_entries[key])) - result["size_diff"] = diff_size + result[git.DIFF_SIZE] = diff_size return result def _get_diff_outs(self, diff_dct): - self.tree = diff_dct["a_tree"] + self.tree = diff_dct[git.DIFF_A_TREE] a_outs = {str(out): out for st in self.stages() for out in st.outs} - self.tree = diff_dct["b_tree"] + self.tree = diff_dct[git.DIFF_B_TREE] b_outs = {str(out): out for st in self.stages() for out in st.outs} outs_paths = set(a_outs.keys()) outs_paths.update(b_outs.keys()) @@ -56,67 +63,71 @@ def _get_diff_outs(self, diff_dct): results[path] = { "a_output": a_outs[path], "b_output": b_outs[path], - "new_file": False, + git.DIFF_NEW_FILE: False, "deleted_file": False, # possible drawback: regular file ->directory movement - "is_dir": a_outs[path].is_dir_cache, + git.DIFF_IS_DIR: a_outs[path].is_dir_cache, } elif path in a_outs: results[path] = { "a_output": a_outs[path], - "new_file": False, + git.DIFF_NEW_FILE: False, "deleted_file": True, - "is_dir": a_outs[path].is_dir_cache, + git.DIFF_IS_DIR: a_outs[path].is_dir_cache, } else: results[path] = { "b_output": b_outs[path], - "new_file": True, + git.DIFF_NEW_FILE: True, "deleted_file": False, - "is_dir": b_outs[path].is_dir_cache, + git.DIFF_IS_DIR: b_outs[path].is_dir_cache, } return results def _diff_dir(self, target, diff_dct): - result = {"target": target} - result["is_dir"] = True + result = {git.DIFF_TARGET: target} + result[git.DIFF_IS_DIR] = True a_entries, b_entries = {}, {} - if not diff_dct["new_file"]: - result["old_file"] = target - result["old_checksum"] = diff_dct["a_output"].checksum - a_entries = _extract_dir(self, diff_dct["a_output"]) - if not diff_dct["deleted_file"]: - result["new_file"] = target - result["new_checksum"] = diff_dct["b_output"].checksum - b_entries = _extract_dir(self, diff_dct["b_output"]) - result.update(_get_tree_changes(self, a_entries, b_entries)) + try: + if not diff_dct[git.DIFF_NEW_FILE]: + result[git.DIFF_OLD_FILE] = target + result[git.DIFF_OLD_CHECKSUM] = diff_dct["a_output"].checksum + a_entries = _extract_dir(self, diff_dct["a_output"]) + if not diff_dct["deleted_file"]: + result[git.DIFF_NEW_FILE] = target + result[git.DIFF_NEW_CHECKSUM] = diff_dct["b_output"].checksum + b_entries = _extract_dir(self, diff_dct["b_output"]) + result.update(_get_tree_changes(self, a_entries, b_entries)) + except FileNotFoundError: + result[git.DIFF_SIZE] = git.DIFF_SIZE_UNKNOWN return result def _diff_file(self, target, diff_dct): - result = {"target": target} + result = {git.DIFF_TARGET: target} size = 0 - if not diff_dct["new_file"]: - result["old_file"] = target - result["old_checksum"] = diff_dct["a_output"].checksum - size -= os.path.getsize( - self.cache.local.get(diff_dct["a_output"].checksum) - ) - print("first", size) - if not diff_dct["deleted_file"]: - result["new_file"] = target - result["new_checksum"] = diff_dct["b_output"].checksum - size += os.path.getsize( - self.cache.local.get(diff_dct["b_output"].checksum) - ) - print("second", size) - result["size_diff"] = size + try: + if not diff_dct[git.DIFF_NEW_FILE]: + result[git.DIFF_OLD_FILE] = target + result[git.DIFF_OLD_CHECKSUM] = diff_dct["a_output"].checksum + size -= os.path.getsize( + self.cache.local.get(diff_dct["a_output"].checksum) + ) + if not diff_dct["deleted_file"]: + result[git.DIFF_NEW_FILE] = target + result[git.DIFF_NEW_CHECKSUM] = diff_dct["b_output"].checksum + size += os.path.getsize( + self.cache.local.get(diff_dct["b_output"].checksum) + ) + except FileNotFoundError: + size = git.DIFF_SIZE_UNKNOWN + result[git.DIFF_SIZE] = size return result def _diff_royal(self, target, diff_dct): - if diff_dct["is_dir"]: + if diff_dct[git.DIFF_IS_DIR]: return _diff_dir(self, target, diff_dct) return _diff_file(self, target, diff_dct) @@ -134,19 +145,19 @@ def diff(self, target, a_ref=None, b_ref=None): """ result = {} diff_dct = self.scm.get_diff_trees(a_ref, b_ref=b_ref) - result["a_ref"] = diff_dct["a_ref"] - result["b_ref"] = diff_dct["b_ref"] - if diff_dct["equal"]: - result["equal"] = True + result[git.DIFF_A_REF] = diff_dct[git.DIFF_A_REF] + result[git.DIFF_B_REF] = diff_dct[git.DIFF_B_REF] + if diff_dct[git.DIFF_EQUAL]: + result[git.DIFF_EQUAL] = True return result - result["diffs"] = [] + result[git.DIFF_LIST] = [] diff_outs = _get_diff_outs(self, diff_dct) if target is None: - result["diffs"] = [ + result[git.DIFF_LIST] = [ _diff_royal(self, path, diff_outs[path]) for path in diff_outs ] elif target in diff_outs: - result["diffs"] = [_diff_royal(self, target, diff_outs[target])] + result[git.DIFF_LIST] = [_diff_royal(self, target, diff_outs[target])] else: msg = "Have not found file/directory '{}' in the commits" raise FileNotInCommitError(msg.format(target)) diff --git a/dvc/scm/base.py b/dvc/scm/base.py index e2ae0ae36..be054345a 100644 --- a/dvc/scm/base.py +++ b/dvc/scm/base.py @@ -7,6 +7,27 @@ import os from dvc.exceptions import DvcException +DIFF_TARGET = "target" +DIFF_IS_DIR = "is_dir" +DIFF_OLD_FILE = "old_file" +DIFF_OLD_CHECKSUM = "old_checksum" +DIFF_NEW_FILE = "new_file" +DIFF_NEW_CHECKSUM = "new_checksum" +DIFF_SIZE = "size_diff" +DIFF_DEL = "del" +DIFF_IDENT = "ident" +DIFF_CHANGE = "changes" +DIFF_NEW = "new" +DIFF_MOVE = "moves" +DIFF_A_TREE = "a_tree" +DIFF_B_TREE = "b_tree" +DIFF_A_REF = "a_ref" +DIFF_B_REF = "b_ref" +DIFF_EQUAL = "equal" +DIFF_LIST = "diffs" +DIFF_SIZE_UNKNOWN = "?" + + class SCMError(DvcException): """Base class for source control management errors.""" @@ -19,7 +40,7 @@ class FileNotInRepoError(DvcException): class FileNotInCommitError(DvcException): """Thrown when trying to find a file/directory that is not - into the specified commit in the repository + in the specified commit in the repository. """ diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index f850aae68..17c1067e9 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -11,6 +11,11 @@ from dvc.scm.base import ( SCMError, FileNotInRepoError, FileNotInTargetSubdirError, + DIFF_A_TREE, + DIFF_B_TREE, + DIFF_A_REF, + DIFF_B_REF, + DIFF_EQUAL, ) from dvc.scm.git.tree import GitTree import dvc.logger as logger @@ -240,7 +245,7 @@ class Git(Base): from gitdb.exc import BadObject, BadName - trees = {"a_tree": None, "b_tree": None} + trees = {DIFF_A_TREE: None, DIFF_B_TREE: None} commits = [] try: if b_ref is not None: @@ -248,16 +253,13 @@ class Git(Base): b_commit = self.git.commit(b_ref) commits.append(str(a_commit)) commits.append(str(b_commit)) - # other way of getting the diff between commits : - # self.git.diff(a_commit, b_commit) returns the diff string - # by reading the files in a, b commits and computing the diff else: a_commit = self.git.head.commit b_commit = self.git.commit(a_ref) commits.append(str(a_commit)) commits.append(str(b_commit)) - trees["a_tree"] = self.get_tree(commits[0]) - trees["b_tree"] = self.get_tree(commits[1]) + trees[DIFF_A_TREE] = self.get_tree(commits[0]) + trees[DIFF_B_TREE] = self.get_tree(commits[1]) except (BadName, BadObject) as e: raise SCMError("git problem", cause=e) return trees, commits @@ -273,13 +275,13 @@ class Git(Base): Returns: dict - dictionary with keys: (a_tree, b_tree, a_ref, b_ref, equal) """ - diff_dct = {"equal": False} + diff_dct = {DIFF_EQUAL: False} trees, commit_refs = self._get_diff_trees(a_ref, b_ref) - diff_dct["a_ref"] = commit_refs[0] - diff_dct["b_ref"] = commit_refs[1] + diff_dct[DIFF_A_REF] = commit_refs[0] + diff_dct[DIFF_B_REF] = commit_refs[1] if commit_refs[0] == commit_refs[1]: - diff_dct["equal"] = True + diff_dct[DIFF_EQUAL] = True return diff_dct - diff_dct["a_tree"] = trees["a_tree"] - diff_dct["b_tree"] = trees["b_tree"] + diff_dct[DIFF_A_TREE] = trees[DIFF_A_TREE] + diff_dct[DIFF_B_TREE] = trees[DIFF_B_TREE] return diff_dct diff --git a/tests/test_diff.py b/tests/test_diff.py index 434e5d94a..b9c6d5975 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -7,6 +7,7 @@ from dvc.main import main from dvc.scm.base import FileNotInCommitError +import dvc.scm.base as git from tests.basic_env import TestDvc @@ -16,7 +17,7 @@ class TestDiff(TestDvc): def setUp(self): super(TestDiff, self).setUp() - self.new_file = "new_file" + self.new_file = "new_test_file" self.create(self.new_file, self.new_file) self.dvc.add(self.new_file) self.a_ref = self.git.head.commit @@ -25,23 +26,23 @@ class TestDiff(TestDvc): def test(self): out = self.dvc.scm.get_diff_trees(self.a_ref) - self.assertFalse(out["equal"]) - self.assertEqual(str(self.a_ref), out["b_ref"]) - self.assertEqual(str(self.git.head.commit), out["a_ref"]) + self.assertFalse(out[git.DIFF_EQUAL]) + self.assertEqual(str(self.a_ref), out[git.DIFF_B_REF]) + self.assertEqual(str(self.git.head.commit), out[git.DIFF_A_REF]) class TestDiffRepo(TestDiff): def test(self): result = self.dvc.diff(self.new_file, a_ref=self.a_ref) test_dct = { - "b_ref": str(self.a_ref), - "a_ref": str(self.git.head.commit), - "diffs": [ + git.DIFF_B_REF: str(self.a_ref), + git.DIFF_A_REF: str(self.git.head.commit), + git.DIFF_LIST: [ { - "target": self.new_file, - "old_file": self.new_file, - "old_checksum": "25fa8325e4e0eb8180445e42558e60bd", - "size_diff": -8, + git.DIFF_TARGET: self.new_file, + git.DIFF_OLD_FILE: self.new_file, + git.DIFF_OLD_CHECKSUM: "c81fec3d710806070bcd67f182d1279c", + git.DIFF_SIZE: -13, } ], } @@ -66,7 +67,7 @@ class TestDiffDir(TestDvc): self.git.index.add([self.DATA_DIR + ".dvc"]) self.git.index.commit("adds data_dir") self.a_ref = str(self.dvc.scm.git.head.commit) - self.new_file = os.path.join(self.DATA_SUB_DIR, "new_file") + self.new_file = os.path.join(self.DATA_SUB_DIR, git.DIFF_NEW_FILE) self.create(self.new_file, self.new_file) self.dvc.add(self.DATA_DIR) self.git.index.add([self.DATA_DIR + ".dvc"]) @@ -74,30 +75,35 @@ class TestDiffDir(TestDvc): def test(self): out = self.dvc.scm.get_diff_trees(self.a_ref) - self.assertFalse(out["equal"]) - self.assertEqual(str(self.a_ref), out["b_ref"]) - self.assertEqual(str(self.git.head.commit), out["a_ref"]) + self.assertFalse(out[git.DIFF_EQUAL]) + self.assertEqual(str(self.a_ref), out[git.DIFF_B_REF]) + self.assertEqual(str(self.git.head.commit), out[git.DIFF_A_REF]) class TestDiffDirRepo(TestDiffDir): def test(self): result = self.dvc.diff(self.DATA_DIR, a_ref=self.a_ref) test_dct = { - "b_ref": str(self.a_ref), - "a_ref": str(self.git.head.commit), - "diffs": [ + git.DIFF_B_REF: str(self.a_ref), + git.DIFF_A_REF: str(self.git.head.commit), + git.DIFF_LIST: [ { - "changes": 0, - "del": 1, - "ident": 2, - "moves": 0, - "new": 0, - "target": self.DATA_DIR, - "old_file": self.DATA_DIR, - "old_checksum": "d5782f3072167ad3a53ee80b92b30718.dir", - "new_file": self.DATA_DIR, - "new_checksum": "bff5a787d16460f32e9f2e62b183b1cc.dir", - "size_diff": -30, + git.DIFF_CHANGE: 0, + git.DIFF_DEL: 1, + git.DIFF_IDENT: 2, + git.DIFF_MOVE: 0, + git.DIFF_NEW: 0, + git.DIFF_IS_DIR: True, + git.DIFF_TARGET: self.DATA_DIR, + git.DIFF_OLD_FILE: self.DATA_DIR, + git.DIFF_OLD_CHECKSUM: ( + "d5782f3072167ad3a53ee80b92b30718.dir" + ), + git.DIFF_NEW_FILE: self.DATA_DIR, + git.DIFF_NEW_CHECKSUM: ( + "bff5a787d16460f32e9f2e62b183b1cc.dir" + ), + git.DIFF_SIZE: -30, } ], } From c235850540208e1cca3ca6dd7e3308d199e463ee Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Mon, 1 Apr 2019 17:27:31 +0600 Subject: [PATCH 15/25] scm: change the order of dvc diff with one argument Now the dvc diff reference command will return the difference betweent from specified commit to current HEAD. --- dvc/scm/git/__init__.py | 4 ++-- tests/test_diff.py | 36 ++++++++++++++++++------------------ 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 17c1067e9..186fad525 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -254,8 +254,8 @@ class Git(Base): commits.append(str(a_commit)) commits.append(str(b_commit)) else: - a_commit = self.git.head.commit - b_commit = self.git.commit(a_ref) + a_commit = self.git.commit(a_ref) + b_commit = self.git.head.commit commits.append(str(a_commit)) commits.append(str(b_commit)) trees[DIFF_A_TREE] = self.get_tree(commits[0]) diff --git a/tests/test_diff.py b/tests/test_diff.py index b9c6d5975..37e2af358 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -27,22 +27,22 @@ class TestDiff(TestDvc): def test(self): out = self.dvc.scm.get_diff_trees(self.a_ref) self.assertFalse(out[git.DIFF_EQUAL]) - self.assertEqual(str(self.a_ref), out[git.DIFF_B_REF]) - self.assertEqual(str(self.git.head.commit), out[git.DIFF_A_REF]) + self.assertEqual(str(self.a_ref), out[git.DIFF_A_REF]) + self.assertEqual(str(self.git.head.commit), out[git.DIFF_B_REF]) class TestDiffRepo(TestDiff): def test(self): result = self.dvc.diff(self.new_file, a_ref=self.a_ref) test_dct = { - git.DIFF_B_REF: str(self.a_ref), - git.DIFF_A_REF: str(self.git.head.commit), + git.DIFF_A_REF: str(self.a_ref), + git.DIFF_B_REF: str(self.git.head.commit), git.DIFF_LIST: [ { git.DIFF_TARGET: self.new_file, - git.DIFF_OLD_FILE: self.new_file, - git.DIFF_OLD_CHECKSUM: "c81fec3d710806070bcd67f182d1279c", - git.DIFF_SIZE: -13, + git.DIFF_NEW_FILE: self.new_file, + git.DIFF_NEW_CHECKSUM: "c81fec3d710806070bcd67f182d1279c", + git.DIFF_SIZE: 13, } ], } @@ -76,34 +76,34 @@ class TestDiffDir(TestDvc): def test(self): out = self.dvc.scm.get_diff_trees(self.a_ref) self.assertFalse(out[git.DIFF_EQUAL]) - self.assertEqual(str(self.a_ref), out[git.DIFF_B_REF]) - self.assertEqual(str(self.git.head.commit), out[git.DIFF_A_REF]) + self.assertEqual(str(self.a_ref), out[git.DIFF_A_REF]) + self.assertEqual(str(self.git.head.commit), out[git.DIFF_B_REF]) class TestDiffDirRepo(TestDiffDir): def test(self): result = self.dvc.diff(self.DATA_DIR, a_ref=self.a_ref) test_dct = { - git.DIFF_B_REF: str(self.a_ref), - git.DIFF_A_REF: str(self.git.head.commit), + git.DIFF_A_REF: str(self.a_ref), + git.DIFF_B_REF: str(self.git.head.commit), git.DIFF_LIST: [ { git.DIFF_CHANGE: 0, - git.DIFF_DEL: 1, + git.DIFF_DEL: 0, git.DIFF_IDENT: 2, git.DIFF_MOVE: 0, - git.DIFF_NEW: 0, + git.DIFF_NEW: 1, git.DIFF_IS_DIR: True, git.DIFF_TARGET: self.DATA_DIR, - git.DIFF_OLD_FILE: self.DATA_DIR, - git.DIFF_OLD_CHECKSUM: ( - "d5782f3072167ad3a53ee80b92b30718.dir" - ), git.DIFF_NEW_FILE: self.DATA_DIR, git.DIFF_NEW_CHECKSUM: ( + "d5782f3072167ad3a53ee80b92b30718.dir" + ), + git.DIFF_OLD_FILE: self.DATA_DIR, + git.DIFF_OLD_CHECKSUM: ( "bff5a787d16460f32e9f2e62b183b1cc.dir" ), - git.DIFF_SIZE: -30, + git.DIFF_SIZE: 30, } ], } From e5d62187b37b06b4a8033ba431593419729d165f Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Mon, 1 Apr 2019 18:20:40 +0600 Subject: [PATCH 16/25] tests: fix the test for dvc diff command --- tests/test_diff.py | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/tests/test_diff.py b/tests/test_diff.py index 37e2af358..a8147a8ec 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -13,6 +13,13 @@ import dvc.scm.base as git from tests.basic_env import TestDvc +def _get_checksum(repo, file_name): + outs = [out for s in repo.stages() for out in s.outs] + for out in outs: + if out.url == file_name: + return out.checksum + + class TestDiff(TestDvc): def setUp(self): super(TestDiff, self).setUp() @@ -21,6 +28,7 @@ class TestDiff(TestDvc): self.create(self.new_file, self.new_file) self.dvc.add(self.new_file) self.a_ref = self.git.head.commit + self.new_checksum = _get_checksum(self.dvc, self.new_file) self.git.index.add([self.new_file + ".dvc"]) self.git.index.commit("adds new_file") @@ -41,7 +49,7 @@ class TestDiffRepo(TestDiff): { git.DIFF_TARGET: self.new_file, git.DIFF_NEW_FILE: self.new_file, - git.DIFF_NEW_CHECKSUM: "c81fec3d710806070bcd67f182d1279c", + git.DIFF_NEW_CHECKSUM: self.new_checksum, git.DIFF_SIZE: 13, } ], @@ -67,11 +75,13 @@ class TestDiffDir(TestDvc): self.git.index.add([self.DATA_DIR + ".dvc"]) self.git.index.commit("adds data_dir") self.a_ref = str(self.dvc.scm.git.head.commit) + self.old_checksum = _get_checksum(self.dvc, self.DATA_DIR) self.new_file = os.path.join(self.DATA_SUB_DIR, git.DIFF_NEW_FILE) self.create(self.new_file, self.new_file) self.dvc.add(self.DATA_DIR) self.git.index.add([self.DATA_DIR + ".dvc"]) self.git.index.commit(message="adds data_dir with new_file") + self.new_checksum = _get_checksum(self.dvc, self.DATA_DIR) def test(self): out = self.dvc.scm.get_diff_trees(self.a_ref) @@ -81,6 +91,8 @@ class TestDiffDir(TestDvc): class TestDiffDirRepo(TestDiffDir): + maxDiff = None + def test(self): result = self.dvc.diff(self.DATA_DIR, a_ref=self.a_ref) test_dct = { @@ -96,13 +108,9 @@ class TestDiffDirRepo(TestDiffDir): git.DIFF_IS_DIR: True, git.DIFF_TARGET: self.DATA_DIR, git.DIFF_NEW_FILE: self.DATA_DIR, - git.DIFF_NEW_CHECKSUM: ( - "d5782f3072167ad3a53ee80b92b30718.dir" - ), + git.DIFF_NEW_CHECKSUM: self.new_checksum, git.DIFF_OLD_FILE: self.DATA_DIR, - git.DIFF_OLD_CHECKSUM: ( - "bff5a787d16460f32e9f2e62b183b1cc.dir" - ), + git.DIFF_OLD_CHECKSUM: self.old_checksum, git.DIFF_SIZE: 30, } ], From c3d70f2958baccdec1661ead5792712a56d305dc Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Mon, 1 Apr 2019 19:08:12 +0600 Subject: [PATCH 17/25] repo: adjust repo.diff API to match `dvc diff` CLI --- dvc/command/diff.py | 2 +- dvc/repo/diff.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dvc/command/diff.py b/dvc/command/diff.py index 5ff7be489..2aae03bda 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -88,7 +88,7 @@ class CmdDiff(CmdBase): def run(self): try: msg = self.repo.diff( - self.args.target, a_ref=self.args.a_ref, b_ref=self.args.b_ref + self.args.a_ref, target=self.args.target, b_ref=self.args.b_ref ) self._show(msg) except DvcException: diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index 85f4601bd..90e474455 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -132,7 +132,7 @@ def _diff_royal(self, target, diff_dct): return _diff_file(self, target, diff_dct) -def diff(self, target, a_ref=None, b_ref=None): +def diff(self, a_ref, target=None, b_ref=None): """Gerenates diff message string output Args: From 7b3790e70136b6ef98e84b05bea0c75e9db0caa9 Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Mon, 1 Apr 2019 20:19:06 +0600 Subject: [PATCH 18/25] repo: fix macros placement from `dvc/scm` to `dvc/repo/diff` --- dvc/command/diff.py | 54 ++++++++-------- dvc/repo/diff.py | 139 ++++++++++++++++++++++++---------------- dvc/scm/base.py | 21 ------ dvc/scm/git/__init__.py | 12 ++-- tests/test_diff.py | 66 +++++++++---------- 5 files changed, 149 insertions(+), 143 deletions(-) diff --git a/dvc/command/diff.py b/dvc/command/diff.py index 2aae03bda..5feadce28 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -8,7 +8,7 @@ import dvc.logger as logger from dvc.exceptions import DvcException from dvc.command.base import CmdBase from dvc.utils.collections import compact -import dvc.scm.base as git +import dvc.repo.diff as diff class CmdDiff(CmdBase): @@ -25,61 +25,61 @@ class CmdDiff(CmdBase): def _show(self, diff_dct): engine = inflect.engine() msg = "dvc diff from {} to {}".format( - diff_dct[git.DIFF_A_REF], diff_dct[git.DIFF_B_REF] + diff_dct[diff.DIFF_A_REF], diff_dct[diff.DIFF_B_REF] ) - if diff_dct.get(git.DIFF_EQUAL): + if diff_dct.get(diff.DIFF_EQUAL): logger.info(msg) return - for dct in diff_dct[git.DIFF_LIST]: - msg += "\n\ndiff for '{}' \n".format(dct[git.DIFF_TARGET]) - if dct.get(git.DIFF_OLD_FILE): + for dct in diff_dct[diff.DIFF_LIST]: + msg += "\n\ndiff for '{}' \n".format(dct[diff.DIFF_TARGET]) + if dct.get(diff.DIFF_OLD_FILE): msg += "-{} with md5 {}\n".format( - dct[git.DIFF_OLD_FILE], dct[git.DIFF_OLD_CHECKSUM] + dct[diff.DIFF_OLD_FILE], dct[diff.DIFF_OLD_CHECKSUM] ) - if dct.get(git.DIFF_NEW_FILE): + if dct.get(diff.DIFF_NEW_FILE): msg += "+{} with md5 {}\n".format( - dct[git.DIFF_NEW_FILE], dct[git.DIFF_NEW_CHECKSUM] + dct[diff.DIFF_NEW_FILE], dct[diff.DIFF_NEW_CHECKSUM] ) msg += "\n" - if dct[git.DIFF_SIZE] != git.DIFF_SIZE_UNKNOWN: + if dct[diff.DIFF_SIZE] != diff.DIFF_SIZE_UNKNOWN: if dct.get("is_dir"): msg += "{} {} not changed, ".format( - dct[git.DIFF_IDENT], - engine.plural("file", dct[git.DIFF_IDENT]), + dct[diff.DIFF_IDENT], + engine.plural("file", dct[diff.DIFF_IDENT]), ) msg += "{} {} modified, ".format( - dct[git.DIFF_CHANGE], - engine.plural("file", dct[git.DIFF_CHANGE]), + dct[diff.DIFF_CHANGE], + engine.plural("file", dct[diff.DIFF_CHANGE]), ) msg += "{} {} added, ".format( - dct[git.DIFF_NEW], - engine.plural("file", dct[git.DIFF_NEW]), + dct[diff.DIFF_NEW], + engine.plural("file", dct[diff.DIFF_NEW]), ) msg += "{} {} deleted, ".format( - dct[git.DIFF_DEL], - engine.plural("file", dct[git.DIFF_DEL]), + dct[diff.DIFF_DEL], + engine.plural("file", dct[diff.DIFF_DEL]), ) msg += "size was {}".format( - self._print_size(dct[git.DIFF_SIZE]) + self._print_size(dct[diff.DIFF_SIZE]) ) else: if ( - dct.get(git.DIFF_OLD_FILE) - and dct.get(git.DIFF_NEW_FILE) - and dct[git.DIFF_SIZE] == 0 + dct.get(diff.DIFF_OLD_FILE) + and dct.get(diff.DIFF_NEW_FILE) + and dct[diff.DIFF_SIZE] == 0 ): msg += "file size was not changed" - elif dct.get(git.DIFF_NEW_FILE): + elif dct.get(diff.DIFF_NEW_FILE): msg += "added file with size {}".format( - humanize.naturalsize(dct[git.DIFF_SIZE]) + humanize.naturalsize(dct[diff.DIFF_SIZE]) ) - elif dct.get(git.DIFF_OLD_FILE): + elif dct.get(diff.DIFF_OLD_FILE): msg += "deleted file with size {}".format( - humanize.naturalsize(abs(dct[git.DIFF_SIZE])) + humanize.naturalsize(abs(dct[diff.DIFF_SIZE])) ) else: msg += "file was modified, file size {}".format( - self._print_size(dct[git.DIFF_SIZE]) + self._print_size(dct[diff.DIFF_SIZE]) ) else: msg += "size is ?" diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index 90e474455..011be69a7 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -4,7 +4,32 @@ import os from dvc.scm.base import FileNotInCommitError -import dvc.scm.base as git +from dvc.scm.git import ( + DIFF_A_TREE, + DIFF_B_TREE, + DIFF_A_REF, + DIFF_B_REF, + DIFF_EQUAL, +) + + +DIFF_TARGET = "target" +DIFF_IS_DIR = "is_dir" +DIFF_OLD_FILE = "old_file" +DIFF_OLD_CHECKSUM = "old_checksum" +DIFF_NEW_FILE = "new_file" +DIFF_NEW_CHECKSUM = "new_checksum" +DIFF_SIZE = "size_diff" +DIFF_DEL = "del" +DIFF_IDENT = "ident" +DIFF_CHANGE = "changes" +DIFF_NEW = "new" +DIFF_MOVE = "moves" +DIFF_LIST = "diffs" +DIFF_SIZE_UNKNOWN = "?" +DIFF_A_OUTPUT = "a_output" +DIFF_B_OUTPUT = "b_output" +DIFF_DELETED = "deleted_file" def _extract_dir(self, output): @@ -22,11 +47,11 @@ def _extract_dir(self, output): def _get_tree_changes(self, a_entries, b_entries): result = { - git.DIFF_DEL: 0, - git.DIFF_IDENT: 0, - git.DIFF_CHANGE: 0, - git.DIFF_NEW: 0, - git.DIFF_MOVE: 0, + DIFF_DEL: 0, + DIFF_IDENT: 0, + DIFF_CHANGE: 0, + DIFF_NEW: 0, + DIFF_MOVE: 0, } keys = set(a_entries.keys()) keys.update(b_entries.keys()) @@ -34,26 +59,26 @@ def _get_tree_changes(self, a_entries, b_entries): for key in keys: if key in a_entries and key in b_entries: if a_entries[key] == b_entries[key]: - result[git.DIFF_IDENT] += 1 + result[DIFF_IDENT] += 1 else: - result[git.DIFF_CHANGE] += 1 + result[DIFF_CHANGE] += 1 diff_size += os.path.getsize( self.cache.local.get(b_entries[key]) ) - os.path.getsize(self.cache.local.get(a_entries[key])) elif key in a_entries: - result[git.DIFF_DEL] += 1 + result[DIFF_DEL] += 1 diff_size -= os.path.getsize(self.cache.local.get(a_entries[key])) else: - result[git.DIFF_NEW] += 1 + result[DIFF_NEW] += 1 diff_size += os.path.getsize(self.cache.local.get(b_entries[key])) - result[git.DIFF_SIZE] = diff_size + result[DIFF_SIZE] = diff_size return result def _get_diff_outs(self, diff_dct): - self.tree = diff_dct[git.DIFF_A_TREE] + self.tree = diff_dct[DIFF_A_TREE] a_outs = {str(out): out for st in self.stages() for out in st.outs} - self.tree = diff_dct[git.DIFF_B_TREE] + self.tree = diff_dct[DIFF_B_TREE] b_outs = {str(out): out for st in self.stages() for out in st.outs} outs_paths = set(a_outs.keys()) outs_paths.update(b_outs.keys()) @@ -61,73 +86,73 @@ def _get_diff_outs(self, diff_dct): for path in outs_paths: if path in a_outs and path in b_outs: results[path] = { - "a_output": a_outs[path], - "b_output": b_outs[path], - git.DIFF_NEW_FILE: False, - "deleted_file": False, + DIFF_A_OUTPUT: a_outs[path], + DIFF_B_OUTPUT: b_outs[path], + DIFF_NEW_FILE: False, + DIFF_DELETED: False, # possible drawback: regular file ->directory movement - git.DIFF_IS_DIR: a_outs[path].is_dir_cache, + DIFF_IS_DIR: a_outs[path].is_dir_cache, } elif path in a_outs: results[path] = { - "a_output": a_outs[path], - git.DIFF_NEW_FILE: False, - "deleted_file": True, - git.DIFF_IS_DIR: a_outs[path].is_dir_cache, + DIFF_A_OUTPUT: a_outs[path], + DIFF_NEW_FILE: False, + DIFF_DELETED: True, + DIFF_IS_DIR: a_outs[path].is_dir_cache, } else: results[path] = { - "b_output": b_outs[path], - git.DIFF_NEW_FILE: True, - "deleted_file": False, - git.DIFF_IS_DIR: b_outs[path].is_dir_cache, + DIFF_B_OUTPUT: b_outs[path], + DIFF_NEW_FILE: True, + DIFF_DELETED: False, + DIFF_IS_DIR: b_outs[path].is_dir_cache, } return results def _diff_dir(self, target, diff_dct): - result = {git.DIFF_TARGET: target} - result[git.DIFF_IS_DIR] = True + result = {DIFF_TARGET: target} + result[DIFF_IS_DIR] = True a_entries, b_entries = {}, {} try: - if not diff_dct[git.DIFF_NEW_FILE]: - result[git.DIFF_OLD_FILE] = target - result[git.DIFF_OLD_CHECKSUM] = diff_dct["a_output"].checksum - a_entries = _extract_dir(self, diff_dct["a_output"]) - if not diff_dct["deleted_file"]: - result[git.DIFF_NEW_FILE] = target - result[git.DIFF_NEW_CHECKSUM] = diff_dct["b_output"].checksum - b_entries = _extract_dir(self, diff_dct["b_output"]) + if not diff_dct[DIFF_NEW_FILE]: + result[DIFF_OLD_FILE] = target + result[DIFF_OLD_CHECKSUM] = diff_dct[DIFF_A_OUTPUT].checksum + a_entries = _extract_dir(self, diff_dct[DIFF_A_OUTPUT]) + if not diff_dct[DIFF_DELETED]: + result[DIFF_NEW_FILE] = target + result[DIFF_NEW_CHECKSUM] = diff_dct[DIFF_B_OUTPUT].checksum + b_entries = _extract_dir(self, diff_dct[DIFF_B_OUTPUT]) result.update(_get_tree_changes(self, a_entries, b_entries)) except FileNotFoundError: - result[git.DIFF_SIZE] = git.DIFF_SIZE_UNKNOWN + result[DIFF_SIZE] = DIFF_SIZE_UNKNOWN return result def _diff_file(self, target, diff_dct): - result = {git.DIFF_TARGET: target} + result = {DIFF_TARGET: target} size = 0 try: - if not diff_dct[git.DIFF_NEW_FILE]: - result[git.DIFF_OLD_FILE] = target - result[git.DIFF_OLD_CHECKSUM] = diff_dct["a_output"].checksum + if not diff_dct[DIFF_NEW_FILE]: + result[DIFF_OLD_FILE] = target + result[DIFF_OLD_CHECKSUM] = diff_dct[DIFF_A_OUTPUT].checksum size -= os.path.getsize( - self.cache.local.get(diff_dct["a_output"].checksum) + self.cache.local.get(diff_dct[DIFF_A_OUTPUT].checksum) ) - if not diff_dct["deleted_file"]: - result[git.DIFF_NEW_FILE] = target - result[git.DIFF_NEW_CHECKSUM] = diff_dct["b_output"].checksum + if not diff_dct[DIFF_DELETED]: + result[DIFF_NEW_FILE] = target + result[DIFF_NEW_CHECKSUM] = diff_dct[DIFF_B_OUTPUT].checksum size += os.path.getsize( - self.cache.local.get(diff_dct["b_output"].checksum) + self.cache.local.get(diff_dct[DIFF_B_OUTPUT].checksum) ) except FileNotFoundError: - size = git.DIFF_SIZE_UNKNOWN - result[git.DIFF_SIZE] = size + size = DIFF_SIZE_UNKNOWN + result[DIFF_SIZE] = size return result def _diff_royal(self, target, diff_dct): - if diff_dct[git.DIFF_IS_DIR]: + if diff_dct[DIFF_IS_DIR]: return _diff_dir(self, target, diff_dct) return _diff_file(self, target, diff_dct) @@ -137,7 +162,7 @@ def diff(self, a_ref, target=None, b_ref=None): Args: target(str) - file/directory to check diff of - a_ref(str) - first git tag + a_ref(str) - first tag (optional) b_ref(str) - second git tag Returns: @@ -145,19 +170,19 @@ def diff(self, a_ref, target=None, b_ref=None): """ result = {} diff_dct = self.scm.get_diff_trees(a_ref, b_ref=b_ref) - result[git.DIFF_A_REF] = diff_dct[git.DIFF_A_REF] - result[git.DIFF_B_REF] = diff_dct[git.DIFF_B_REF] - if diff_dct[git.DIFF_EQUAL]: - result[git.DIFF_EQUAL] = True + result[DIFF_A_REF] = diff_dct[DIFF_A_REF] + result[DIFF_B_REF] = diff_dct[DIFF_B_REF] + if diff_dct[DIFF_EQUAL]: + result[DIFF_EQUAL] = True return result - result[git.DIFF_LIST] = [] + result[DIFF_LIST] = [] diff_outs = _get_diff_outs(self, diff_dct) if target is None: - result[git.DIFF_LIST] = [ + result[DIFF_LIST] = [ _diff_royal(self, path, diff_outs[path]) for path in diff_outs ] elif target in diff_outs: - result[git.DIFF_LIST] = [_diff_royal(self, target, diff_outs[target])] + result[DIFF_LIST] = [_diff_royal(self, target, diff_outs[target])] else: msg = "Have not found file/directory '{}' in the commits" raise FileNotInCommitError(msg.format(target)) diff --git a/dvc/scm/base.py b/dvc/scm/base.py index be054345a..d7f5333e3 100644 --- a/dvc/scm/base.py +++ b/dvc/scm/base.py @@ -7,27 +7,6 @@ import os from dvc.exceptions import DvcException -DIFF_TARGET = "target" -DIFF_IS_DIR = "is_dir" -DIFF_OLD_FILE = "old_file" -DIFF_OLD_CHECKSUM = "old_checksum" -DIFF_NEW_FILE = "new_file" -DIFF_NEW_CHECKSUM = "new_checksum" -DIFF_SIZE = "size_diff" -DIFF_DEL = "del" -DIFF_IDENT = "ident" -DIFF_CHANGE = "changes" -DIFF_NEW = "new" -DIFF_MOVE = "moves" -DIFF_A_TREE = "a_tree" -DIFF_B_TREE = "b_tree" -DIFF_A_REF = "a_ref" -DIFF_B_REF = "b_ref" -DIFF_EQUAL = "equal" -DIFF_LIST = "diffs" -DIFF_SIZE_UNKNOWN = "?" - - class SCMError(DvcException): """Base class for source control management errors.""" diff --git a/dvc/scm/git/__init__.py b/dvc/scm/git/__init__.py index 186fad525..9eb49f096 100644 --- a/dvc/scm/git/__init__.py +++ b/dvc/scm/git/__init__.py @@ -11,16 +11,18 @@ from dvc.scm.base import ( SCMError, FileNotInRepoError, FileNotInTargetSubdirError, - DIFF_A_TREE, - DIFF_B_TREE, - DIFF_A_REF, - DIFF_B_REF, - DIFF_EQUAL, ) from dvc.scm.git.tree import GitTree import dvc.logger as logger +DIFF_A_TREE = "a_tree" +DIFF_B_TREE = "b_tree" +DIFF_A_REF = "a_ref" +DIFF_B_REF = "b_ref" +DIFF_EQUAL = "equal" + + class Git(Base): """Class for managing Git.""" diff --git a/tests/test_diff.py b/tests/test_diff.py index a8147a8ec..e22d78620 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -7,7 +7,7 @@ from dvc.main import main from dvc.scm.base import FileNotInCommitError -import dvc.scm.base as git +import dvc.repo.diff as diff from tests.basic_env import TestDvc @@ -34,23 +34,23 @@ class TestDiff(TestDvc): def test(self): out = self.dvc.scm.get_diff_trees(self.a_ref) - self.assertFalse(out[git.DIFF_EQUAL]) - self.assertEqual(str(self.a_ref), out[git.DIFF_A_REF]) - self.assertEqual(str(self.git.head.commit), out[git.DIFF_B_REF]) + self.assertFalse(out[diff.DIFF_EQUAL]) + self.assertEqual(str(self.a_ref), out[diff.DIFF_A_REF]) + self.assertEqual(str(self.git.head.commit), out[diff.DIFF_B_REF]) class TestDiffRepo(TestDiff): def test(self): - result = self.dvc.diff(self.new_file, a_ref=self.a_ref) + result = self.dvc.diff(self.a_ref, target=self.new_file) test_dct = { - git.DIFF_A_REF: str(self.a_ref), - git.DIFF_B_REF: str(self.git.head.commit), - git.DIFF_LIST: [ + diff.DIFF_A_REF: str(self.a_ref), + diff.DIFF_B_REF: str(self.git.head.commit), + diff.DIFF_LIST: [ { - git.DIFF_TARGET: self.new_file, - git.DIFF_NEW_FILE: self.new_file, - git.DIFF_NEW_CHECKSUM: self.new_checksum, - git.DIFF_SIZE: 13, + diff.DIFF_TARGET: self.new_file, + diff.DIFF_NEW_FILE: self.new_file, + diff.DIFF_NEW_CHECKSUM: self.new_checksum, + diff.DIFF_SIZE: 13, } ], } @@ -76,7 +76,7 @@ class TestDiffDir(TestDvc): self.git.index.commit("adds data_dir") self.a_ref = str(self.dvc.scm.git.head.commit) self.old_checksum = _get_checksum(self.dvc, self.DATA_DIR) - self.new_file = os.path.join(self.DATA_SUB_DIR, git.DIFF_NEW_FILE) + self.new_file = os.path.join(self.DATA_SUB_DIR, diff.DIFF_NEW_FILE) self.create(self.new_file, self.new_file) self.dvc.add(self.DATA_DIR) self.git.index.add([self.DATA_DIR + ".dvc"]) @@ -85,33 +85,33 @@ class TestDiffDir(TestDvc): def test(self): out = self.dvc.scm.get_diff_trees(self.a_ref) - self.assertFalse(out[git.DIFF_EQUAL]) - self.assertEqual(str(self.a_ref), out[git.DIFF_A_REF]) - self.assertEqual(str(self.git.head.commit), out[git.DIFF_B_REF]) + self.assertFalse(out[diff.DIFF_EQUAL]) + self.assertEqual(str(self.a_ref), out[diff.DIFF_A_REF]) + self.assertEqual(str(self.git.head.commit), out[diff.DIFF_B_REF]) class TestDiffDirRepo(TestDiffDir): maxDiff = None def test(self): - result = self.dvc.diff(self.DATA_DIR, a_ref=self.a_ref) + result = self.dvc.diff(self.a_ref, target=self.DATA_DIR) test_dct = { - git.DIFF_A_REF: str(self.a_ref), - git.DIFF_B_REF: str(self.git.head.commit), - git.DIFF_LIST: [ + diff.DIFF_A_REF: str(self.a_ref), + diff.DIFF_B_REF: str(self.git.head.commit), + diff.DIFF_LIST: [ { - git.DIFF_CHANGE: 0, - git.DIFF_DEL: 0, - git.DIFF_IDENT: 2, - git.DIFF_MOVE: 0, - git.DIFF_NEW: 1, - git.DIFF_IS_DIR: True, - git.DIFF_TARGET: self.DATA_DIR, - git.DIFF_NEW_FILE: self.DATA_DIR, - git.DIFF_NEW_CHECKSUM: self.new_checksum, - git.DIFF_OLD_FILE: self.DATA_DIR, - git.DIFF_OLD_CHECKSUM: self.old_checksum, - git.DIFF_SIZE: 30, + diff.DIFF_CHANGE: 0, + diff.DIFF_DEL: 0, + diff.DIFF_IDENT: 2, + diff.DIFF_MOVE: 0, + diff.DIFF_NEW: 1, + diff.DIFF_IS_DIR: True, + diff.DIFF_TARGET: self.DATA_DIR, + diff.DIFF_NEW_FILE: self.DATA_DIR, + diff.DIFF_NEW_CHECKSUM: self.new_checksum, + diff.DIFF_OLD_FILE: self.DATA_DIR, + diff.DIFF_OLD_CHECKSUM: self.old_checksum, + diff.DIFF_SIZE: 30, } ], } @@ -125,4 +125,4 @@ class TestDiffFileNotFound(TestDiffDir): def test(self): with self.assertRaises(FileNotInCommitError): - self.dvc.diff(self.unknown_file, a_ref=self.a_ref) + self.dvc.diff(self.a_ref, target=self.unknown_file) From f7550c19c03b6dc264cc07866cdd783672147479 Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Tue, 2 Apr 2019 15:41:20 +0600 Subject: [PATCH 19/25] repo: change FileNotFoundError exception to IOError --- dvc/repo/diff.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index 011be69a7..305ab0626 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals import os +from errno import ENOENT from dvc.scm.base import FileNotInCommitError @@ -32,6 +33,13 @@ DIFF_B_OUTPUT = "b_output" DIFF_DELETED = "deleted_file" +def _file_not_exists(error, result): + if error.errno == ENOENT: + result.update({DIFF_SIZE: DIFF_SIZE_UNKNOWN}) + else: + raise error + + def _extract_dir(self, output): """Extract the content of dvc tree file Args: @@ -124,8 +132,8 @@ def _diff_dir(self, target, diff_dct): result[DIFF_NEW_CHECKSUM] = diff_dct[DIFF_B_OUTPUT].checksum b_entries = _extract_dir(self, diff_dct[DIFF_B_OUTPUT]) result.update(_get_tree_changes(self, a_entries, b_entries)) - except FileNotFoundError: - result[DIFF_SIZE] = DIFF_SIZE_UNKNOWN + except IOError as e: + _file_not_exists(e, result) return result @@ -145,9 +153,9 @@ def _diff_file(self, target, diff_dct): size += os.path.getsize( self.cache.local.get(diff_dct[DIFF_B_OUTPUT].checksum) ) - except FileNotFoundError: - size = DIFF_SIZE_UNKNOWN - result[DIFF_SIZE] = size + result[DIFF_SIZE] = size + except IOError as e: + _file_not_exists(e, result) return result From fcca6aebe8ed6627e41870362cef118c4878d4fb Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Wed, 3 Apr 2019 15:22:11 +0600 Subject: [PATCH 20/25] tests: add tests for CLI message output --- dvc/command/diff.py | 3 +- tests/test_diff.py | 91 ++++++++++++++++++++++++++++++++++++--------- 2 files changed, 76 insertions(+), 18 deletions(-) diff --git a/dvc/command/diff.py b/dvc/command/diff.py index 5feadce28..3326b69b1 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -31,7 +31,7 @@ class CmdDiff(CmdBase): logger.info(msg) return for dct in diff_dct[diff.DIFF_LIST]: - msg += "\n\ndiff for '{}' \n".format(dct[diff.DIFF_TARGET]) + msg += "\n\ndiff for '{}'\n".format(dct[diff.DIFF_TARGET]) if dct.get(diff.DIFF_OLD_FILE): msg += "-{} with md5 {}\n".format( dct[diff.DIFF_OLD_FILE], dct[diff.DIFF_OLD_CHECKSUM] @@ -84,6 +84,7 @@ class CmdDiff(CmdBase): else: msg += "size is ?" logger.info(msg) + return msg def run(self): try: diff --git a/tests/test_diff.py b/tests/test_diff.py index e22d78620..4b92c791a 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -1,13 +1,14 @@ from __future__ import unicode_literals import os -from mock import patch +from mock import patch, Mock from dvc.main import main from dvc.scm.base import FileNotInCommitError import dvc.repo.diff as diff +from dvc.command.diff import CmdDiff from tests.basic_env import TestDvc @@ -31,18 +32,7 @@ class TestDiff(TestDvc): self.new_checksum = _get_checksum(self.dvc, self.new_file) self.git.index.add([self.new_file + ".dvc"]) self.git.index.commit("adds new_file") - - def test(self): - out = self.dvc.scm.get_diff_trees(self.a_ref) - self.assertFalse(out[diff.DIFF_EQUAL]) - self.assertEqual(str(self.a_ref), out[diff.DIFF_A_REF]) - self.assertEqual(str(self.git.head.commit), out[diff.DIFF_B_REF]) - - -class TestDiffRepo(TestDiff): - def test(self): - result = self.dvc.diff(self.a_ref, target=self.new_file) - test_dct = { + self.test_dct = { diff.DIFF_A_REF: str(self.a_ref), diff.DIFF_B_REF: str(self.git.head.commit), diff.DIFF_LIST: [ @@ -54,19 +44,52 @@ class TestDiffRepo(TestDiff): } ], } - self.assertEqual(test_dct, result) + + def test(self): + out = self.dvc.scm.get_diff_trees(self.a_ref) + self.assertFalse(out[diff.DIFF_EQUAL]) + self.assertEqual(str(self.a_ref), out[diff.DIFF_A_REF]) + self.assertEqual(str(self.git.head.commit), out[diff.DIFF_B_REF]) + + +class TestDiffRepo(TestDiff): + def test(self): + result = self.dvc.diff(self.a_ref, target=self.new_file) + self.assertEqual(self.test_dct, result) class TestDiffCmdLine(TestDiff): def test(self): with patch("dvc.cli.diff.CmdDiff._show", autospec=True): with patch("dvc.repo.Repo", config="testing") as MockRepo: - MockRepo.return_value.diff.return_value = "testing" MockRepo.return_value.diff.return_value = "testing" ret = main(["diff", "-t", self.new_file, str(self.a_ref)]) self.assertEqual(ret, 0) +class TestDiffCmdMessage(TestDiff): + maxDiff = None + + def test(self): + with patch("dvc.repo.Repo", config="testing"): + m = Mock() + cmd_diff = CmdDiff(m) + msg = cmd_diff._show(self.test_dct) + test_msg = ( + "dvc diff from {0} to {1}\n\n" + "diff for '{2}'\n" + "+{2} with md5 {3}\n\n" + "added file with size 13 Bytes" + ) + test_msg = test_msg.format( + self.test_dct[diff.DIFF_A_REF], + self.test_dct[diff.DIFF_B_REF], + self.test_dct[diff.DIFF_LIST][0][diff.DIFF_TARGET], + self.test_dct[diff.DIFF_LIST][0][diff.DIFF_NEW_CHECKSUM], + ) + self.assertEqual(test_msg, msg) + + class TestDiffDir(TestDvc): def setUp(self): super(TestDiffDir, self).setUp() @@ -91,8 +114,6 @@ class TestDiffDir(TestDvc): class TestDiffDirRepo(TestDiffDir): - maxDiff = None - def test(self): result = self.dvc.diff(self.a_ref, target=self.DATA_DIR) test_dct = { @@ -118,6 +139,42 @@ class TestDiffDirRepo(TestDiffDir): self.assertEqual(test_dct, result) +class TestDiffDirRepoDeletedFile(TestDiffDir): + def setUp(self): + super(TestDiffDirRepoDeletedFile, self).setUp() + + self.b_ref = self.a_ref + self.new_checksum = self.old_checksum + self.a_ref = str(self.dvc.scm.git.head.commit) + self.old_checksum = _get_checksum(self.dvc, self.DATA_DIR) + + def test(self): + result = self.dvc.diff( + self.a_ref, b_ref=self.b_ref, target=self.DATA_DIR + ) + test_dct = { + diff.DIFF_A_REF: str(self.a_ref), + diff.DIFF_B_REF: str(self.b_ref), + diff.DIFF_LIST: [ + { + diff.DIFF_CHANGE: 0, + diff.DIFF_DEL: 1, + diff.DIFF_IDENT: 2, + diff.DIFF_MOVE: 0, + diff.DIFF_NEW: 0, + diff.DIFF_IS_DIR: True, + diff.DIFF_TARGET: self.DATA_DIR, + diff.DIFF_NEW_FILE: self.DATA_DIR, + diff.DIFF_NEW_CHECKSUM: self.new_checksum, + diff.DIFF_OLD_FILE: self.DATA_DIR, + diff.DIFF_OLD_CHECKSUM: self.old_checksum, + diff.DIFF_SIZE: -30, + } + ], + } + self.assertEqual(test_dct, result) + + class TestDiffFileNotFound(TestDiffDir): def setUp(self): super(TestDiffFileNotFound, self).setUp() From ca2f4c50caa8ee9ad39af4d9252752d1b283ae06 Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Wed, 3 Apr 2019 15:49:54 +0600 Subject: [PATCH 21/25] repo: add checker for out.cheme != "local" case for `dvc diff` --- dvc/repo/diff.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index 305ab0626..7ffe3f729 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -4,6 +4,7 @@ import os from errno import ENOENT +import dvc.logger as logger from dvc.scm.base import FileNotInCommitError from dvc.scm.git import ( DIFF_A_TREE, @@ -83,6 +84,20 @@ def _get_tree_changes(self, a_entries, b_entries): return result +def _check_local_cache(a_out, b_out, is_checked): + if not is_checked: + outputs = [] + if a_out is not None and a_out.scheme != "local": + is_checked = True + outputs.append(str(a_out)) + if b_out is not None and b_out.scheme != "local": + is_checked = True + outputs.append(str(b_out)) + if is_checked: + logger.warning("non-local output: {}".format(outputs)) + return is_checked + + def _get_diff_outs(self, diff_dct): self.tree = diff_dct[DIFF_A_TREE] a_outs = {str(out): out for st in self.stages() for out in st.outs} @@ -91,7 +106,14 @@ def _get_diff_outs(self, diff_dct): outs_paths = set(a_outs.keys()) outs_paths.update(b_outs.keys()) results = {} + not_local_cache = False for path in outs_paths: + not_local_cache = _check_local_cache( + a_outs.get(path), b_outs.get(path), not_local_cache + ) + if b_outs.get(path) and b_outs[path].scheme != "local": + not_local_cache = True + continue if path in a_outs and path in b_outs: results[path] = { DIFF_A_OUTPUT: a_outs[path], From 585babb6365c23ac6a14fbd0e3aea6c4ab3d81dd Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Thu, 4 Apr 2019 15:06:16 +0600 Subject: [PATCH 22/25] dvc: some refactoring of code --- dvc/command/diff.py | 25 +++++------- dvc/repo/diff.py | 99 ++++++++++++++++++++++++++++----------------- 2 files changed, 74 insertions(+), 50 deletions(-) diff --git a/dvc/command/diff.py b/dvc/command/diff.py index 3326b69b1..a092f6cf3 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -32,35 +32,32 @@ class CmdDiff(CmdBase): return for dct in diff_dct[diff.DIFF_LIST]: msg += "\n\ndiff for '{}'\n".format(dct[diff.DIFF_TARGET]) + sample_msg = "{}{} with md5 {}\n" if dct.get(diff.DIFF_OLD_FILE): - msg += "-{} with md5 {}\n".format( - dct[diff.DIFF_OLD_FILE], dct[diff.DIFF_OLD_CHECKSUM] + msg += sample_msg.format( + "-", dct[diff.DIFF_OLD_FILE], dct[diff.DIFF_OLD_CHECKSUM] ) if dct.get(diff.DIFF_NEW_FILE): - msg += "+{} with md5 {}\n".format( - dct[diff.DIFF_NEW_FILE], dct[diff.DIFF_NEW_CHECKSUM] + msg += sample_msg.format( + "+", dct[diff.DIFF_NEW_FILE], dct[diff.DIFF_NEW_CHECKSUM] ) msg += "\n" if dct[diff.DIFF_SIZE] != diff.DIFF_SIZE_UNKNOWN: if dct.get("is_dir"): - msg += "{} {} not changed, ".format( + changes_msg = ( + "{} {} not changed, {} {} modified, {} {} added, " + "{} {} deleted, size was {}" + ) + msg += changes_msg.format( dct[diff.DIFF_IDENT], engine.plural("file", dct[diff.DIFF_IDENT]), - ) - msg += "{} {} modified, ".format( dct[diff.DIFF_CHANGE], engine.plural("file", dct[diff.DIFF_CHANGE]), - ) - msg += "{} {} added, ".format( dct[diff.DIFF_NEW], engine.plural("file", dct[diff.DIFF_NEW]), - ) - msg += "{} {} deleted, ".format( dct[diff.DIFF_DEL], engine.plural("file", dct[diff.DIFF_DEL]), - ) - msg += "size was {}".format( - self._print_size(dct[diff.DIFF_SIZE]) + self._print_size(dct[diff.DIFF_SIZE]), ) else: if ( diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index 7ffe3f729..7a184c47e 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -54,6 +54,44 @@ def _extract_dir(self, output): return {i["relpath"]: i["md5"] for i in lst} +def _ident_files(a_entries, b_entries): + keys = [key for key in a_entries.keys() if key in b_entries] + return len(keys) + + +def _modified_files(self, a_entries, b_entries): + keys = [key for key in a_entries.keys() if key in b_entries] + diff_size = 0 + modified_count = 0 + for key in keys: + if a_entries[key] != b_entries[key]: + modified_count + diff_size += os.path.getsize( + self.cache.local.get(b_entries[key]) + ) - os.path.getsize(self.cache.local.get(a_entries[key])) + return modified_count, diff_size + + +def _deleted_files(self, a_entries, b_entries): + diff_size = 0 + deleted_count = 0 + for key, value in a_entries.items(): + if key not in b_entries: + deleted_count += 1 + diff_size -= os.path.getsize(self.cache.local.get(a_entries[key])) + return deleted_count, diff_size + + +def _new_files(self, a_entries, b_entries): + diff_size = 0 + new_count = 0 + for key, value in b_entries.items(): + if key not in a_entries: + new_count += 1 + diff_size += os.path.getsize(self.cache.local.get(b_entries[key])) + return new_count, diff_size + + def _get_tree_changes(self, a_entries, b_entries): result = { DIFF_DEL: 0, @@ -62,40 +100,23 @@ def _get_tree_changes(self, a_entries, b_entries): DIFF_NEW: 0, DIFF_MOVE: 0, } - keys = set(a_entries.keys()) - keys.update(b_entries.keys()) - diff_size = 0 - for key in keys: - if key in a_entries and key in b_entries: - if a_entries[key] == b_entries[key]: - result[DIFF_IDENT] += 1 - else: - result[DIFF_CHANGE] += 1 - diff_size += os.path.getsize( - self.cache.local.get(b_entries[key]) - ) - os.path.getsize(self.cache.local.get(a_entries[key])) - elif key in a_entries: - result[DIFF_DEL] += 1 - diff_size -= os.path.getsize(self.cache.local.get(a_entries[key])) - else: - result[DIFF_NEW] += 1 - diff_size += os.path.getsize(self.cache.local.get(b_entries[key])) + result[DIFF_IDENT] = _ident_files(a_entries, b_entries) + result[DIFF_CHANGE], diff_size = _modified_files( + self, a_entries, b_entries + ) result[DIFF_SIZE] = diff_size + result[DIFF_DEL], diff_size = _deleted_files(self, a_entries, b_entries) + result[DIFF_SIZE] += diff_size + result[DIFF_NEW], diff_size = _new_files(self, a_entries, b_entries) + result[DIFF_SIZE] += diff_size return result -def _check_local_cache(a_out, b_out, is_checked): - if not is_checked: - outputs = [] - if a_out is not None and a_out.scheme != "local": - is_checked = True - outputs.append(str(a_out)) - if b_out is not None and b_out.scheme != "local": - is_checked = True - outputs.append(str(b_out)) - if is_checked: - logger.warning("non-local output: {}".format(outputs)) - return is_checked +def _check_local_cache(a_out, is_checked): + if a_out is not None and a_out.scheme != "local": + is_checked.append(str(a_out)) + return True + return False def _get_diff_outs(self, diff_dct): @@ -106,13 +127,12 @@ def _get_diff_outs(self, diff_dct): outs_paths = set(a_outs.keys()) outs_paths.update(b_outs.keys()) results = {} - not_local_cache = False + non_local_cache = [] for path in outs_paths: - not_local_cache = _check_local_cache( - a_outs.get(path), b_outs.get(path), not_local_cache - ) - if b_outs.get(path) and b_outs[path].scheme != "local": - not_local_cache = True + check1 = _check_local_cache(a_outs.get(path), non_local_cache) + check2 = _check_local_cache(b_outs.get(path), non_local_cache) + # skip files/directories with non-local cache for now + if check1 or check2: continue if path in a_outs and path in b_outs: results[path] = { @@ -137,6 +157,13 @@ def _get_diff_outs(self, diff_dct): DIFF_DELETED: False, DIFF_IS_DIR: b_outs[path].is_dir_cache, } + if non_local_cache: + logger.warning( + "Diff is not supported for non-local outputs. Ignoring: {}".format( + non_local_cache + ) + ) + return results From fe043ce3a6c38d3a311d79cf52b10ad34bb246c7 Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Thu, 4 Apr 2019 16:48:05 +0600 Subject: [PATCH 23/25] repo: refactoring of _get_diff_outs funciton --- dvc/repo/diff.py | 62 +++++++++++++++++++++++++++++------------------- 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index 7a184c47e..0bd186d8c 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -32,6 +32,7 @@ DIFF_SIZE_UNKNOWN = "?" DIFF_A_OUTPUT = "a_output" DIFF_B_OUTPUT = "b_output" DIFF_DELETED = "deleted_file" +DIFF_IS_NEW = "created_file" def _file_not_exists(error, result): @@ -55,7 +56,9 @@ def _extract_dir(self, output): def _ident_files(a_entries, b_entries): - keys = [key for key in a_entries.keys() if key in b_entries] + keys = [ + key for key in a_entries.keys() if b_entries.get(key) == a_entries[key] + ] return len(keys) @@ -134,29 +137,38 @@ def _get_diff_outs(self, diff_dct): # skip files/directories with non-local cache for now if check1 or check2: continue - if path in a_outs and path in b_outs: - results[path] = { - DIFF_A_OUTPUT: a_outs[path], - DIFF_B_OUTPUT: b_outs[path], - DIFF_NEW_FILE: False, - DIFF_DELETED: False, - # possible drawback: regular file ->directory movement - DIFF_IS_DIR: a_outs[path].is_dir_cache, - } - elif path in a_outs: - results[path] = { - DIFF_A_OUTPUT: a_outs[path], - DIFF_NEW_FILE: False, - DIFF_DELETED: True, - DIFF_IS_DIR: a_outs[path].is_dir_cache, - } + results[path] = {} + results[path][DIFF_A_OUTPUT] = a_outs.get(path) + results[path][DIFF_B_OUTPUT] = b_outs.get(path) + results[path][DIFF_IS_NEW] = path not in a_outs + results[path][DIFF_DELETED] = path not in b_outs + if path in a_outs: + results[path][DIFF_IS_DIR] = a_outs[path].is_dir_cache else: - results[path] = { - DIFF_B_OUTPUT: b_outs[path], - DIFF_NEW_FILE: True, - DIFF_DELETED: False, - DIFF_IS_DIR: b_outs[path].is_dir_cache, - } + results[path][DIFF_IS_DIR] = b_outs[path].is_dir_cache + # if path in a_outs and path in b_outs: + # results[path] = { + # DIFF_A_OUTPUT: a_outs[path], + # DIFF_B_OUTPUT: b_outs[path], + # DIFF_IS_NEW: False, + # DIFF_DELETED: False, + # # possible drawback: regular file ->directory movement + # DIFF_IS_DIR: a_outs[path].is_dir_cache, + # } + # elif path in a_outs: + # results[path] = { + # DIFF_A_OUTPUT: a_outs[path], + # DIFF_IS_NEW: False, + # DIFF_DELETED: True, + # DIFF_IS_DIR: a_outs[path].is_dir_cache, + # } + # else: + # results[path] = { + # DIFF_B_OUTPUT: b_outs[path], + # DIFF_IS_NEW: True, + # DIFF_DELETED: False, + # DIFF_IS_DIR: b_outs[path].is_dir_cache, + # } if non_local_cache: logger.warning( "Diff is not supported for non-local outputs. Ignoring: {}".format( @@ -172,7 +184,7 @@ def _diff_dir(self, target, diff_dct): result[DIFF_IS_DIR] = True a_entries, b_entries = {}, {} try: - if not diff_dct[DIFF_NEW_FILE]: + if not diff_dct[DIFF_IS_NEW]: result[DIFF_OLD_FILE] = target result[DIFF_OLD_CHECKSUM] = diff_dct[DIFF_A_OUTPUT].checksum a_entries = _extract_dir(self, diff_dct[DIFF_A_OUTPUT]) @@ -190,7 +202,7 @@ def _diff_file(self, target, diff_dct): result = {DIFF_TARGET: target} size = 0 try: - if not diff_dct[DIFF_NEW_FILE]: + if not diff_dct[DIFF_IS_NEW]: result[DIFF_OLD_FILE] = target result[DIFF_OLD_CHECKSUM] = diff_dct[DIFF_A_OUTPUT].checksum size -= os.path.getsize( From a5c04e92962f18b037e1367d8b8358a4f6b7cc74 Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Fri, 5 Apr 2019 13:18:31 +0600 Subject: [PATCH 24/25] command: refactoring of _show function in diff.py --- dvc/command/diff.py | 114 +++++++++++++++++++++++++------------------- tests/test_diff.py | 4 ++ 2 files changed, 70 insertions(+), 48 deletions(-) diff --git a/dvc/command/diff.py b/dvc/command/diff.py index a092f6cf3..2eb73bd05 100644 --- a/dvc/command/diff.py +++ b/dvc/command/diff.py @@ -22,8 +22,62 @@ class CmdDiff(CmdBase): natur_size = humanize.naturalsize(abs(size)) return change.format(natur_size) - def _show(self, diff_dct): + def _get_md5_string(self, sign, file_name, checksum): + sample_msg = "" + if file_name: + sample_msg = "{}{} with md5 {}\n" + sample_msg = sample_msg.format(sign, file_name, checksum) + return sample_msg + + def _get_dir_changes(self, dct): engine = inflect.engine() + changes_msg = ( + "{} {} not changed, {} {} modified, {} {} added, " + "{} {} deleted, size was {}" + ) + changes_msg = changes_msg.format( + dct[diff.DIFF_IDENT], + engine.plural("file", dct[diff.DIFF_IDENT]), + dct[diff.DIFF_CHANGE], + engine.plural("file", dct[diff.DIFF_CHANGE]), + dct[diff.DIFF_NEW], + engine.plural("file", dct[diff.DIFF_NEW]), + dct[diff.DIFF_DEL], + engine.plural("file", dct[diff.DIFF_DEL]), + self._print_size(dct[diff.DIFF_SIZE]), + ) + return changes_msg + + def _get_file_changes(self, dct): + if ( + dct.get(diff.DIFF_OLD_FILE) + and dct.get(diff.DIFF_NEW_FILE) + and dct[diff.DIFF_SIZE] == 0 + ): + msg = "file size was not changed" + elif dct.get(diff.DIFF_NEW_FILE): + msg = "added file with size {}".format( + humanize.naturalsize(dct[diff.DIFF_SIZE]) + ) + elif dct.get(diff.DIFF_OLD_FILE): + msg = "deleted file with size {}".format( + humanize.naturalsize(abs(dct[diff.DIFF_SIZE])) + ) + else: + msg = "file was modified, file size {}".format( + self._print_size(dct[diff.DIFF_SIZE]) + ) + return msg + + def _get_royal_changes(self, dct): + if dct[diff.DIFF_SIZE] != diff.DIFF_SIZE_UNKNOWN: + if dct.get("is_dir"): + return self._get_dir_changes(dct) + else: + return self._get_file_changes(dct) + return "size is ?" + + def _show(self, diff_dct): msg = "dvc diff from {} to {}".format( diff_dct[diff.DIFF_A_REF], diff_dct[diff.DIFF_B_REF] ) @@ -32,54 +86,18 @@ class CmdDiff(CmdBase): return for dct in diff_dct[diff.DIFF_LIST]: msg += "\n\ndiff for '{}'\n".format(dct[diff.DIFF_TARGET]) - sample_msg = "{}{} with md5 {}\n" - if dct.get(diff.DIFF_OLD_FILE): - msg += sample_msg.format( - "-", dct[diff.DIFF_OLD_FILE], dct[diff.DIFF_OLD_CHECKSUM] - ) - if dct.get(diff.DIFF_NEW_FILE): - msg += sample_msg.format( - "+", dct[diff.DIFF_NEW_FILE], dct[diff.DIFF_NEW_CHECKSUM] - ) + msg += self._get_md5_string( + "-", + dct.get(diff.DIFF_OLD_FILE), + dct.get(diff.DIFF_OLD_CHECKSUM), + ) + msg += self._get_md5_string( + "+", + dct.get(diff.DIFF_NEW_FILE), + dct.get(diff.DIFF_NEW_CHECKSUM), + ) msg += "\n" - if dct[diff.DIFF_SIZE] != diff.DIFF_SIZE_UNKNOWN: - if dct.get("is_dir"): - changes_msg = ( - "{} {} not changed, {} {} modified, {} {} added, " - "{} {} deleted, size was {}" - ) - msg += changes_msg.format( - dct[diff.DIFF_IDENT], - engine.plural("file", dct[diff.DIFF_IDENT]), - dct[diff.DIFF_CHANGE], - engine.plural("file", dct[diff.DIFF_CHANGE]), - dct[diff.DIFF_NEW], - engine.plural("file", dct[diff.DIFF_NEW]), - dct[diff.DIFF_DEL], - engine.plural("file", dct[diff.DIFF_DEL]), - self._print_size(dct[diff.DIFF_SIZE]), - ) - else: - if ( - dct.get(diff.DIFF_OLD_FILE) - and dct.get(diff.DIFF_NEW_FILE) - and dct[diff.DIFF_SIZE] == 0 - ): - msg += "file size was not changed" - elif dct.get(diff.DIFF_NEW_FILE): - msg += "added file with size {}".format( - humanize.naturalsize(dct[diff.DIFF_SIZE]) - ) - elif dct.get(diff.DIFF_OLD_FILE): - msg += "deleted file with size {}".format( - humanize.naturalsize(abs(dct[diff.DIFF_SIZE])) - ) - else: - msg += "file was modified, file size {}".format( - self._print_size(dct[diff.DIFF_SIZE]) - ) - else: - msg += "size is ?" + msg += self._get_royal_changes(dct) logger.info(msg) return msg diff --git a/tests/test_diff.py b/tests/test_diff.py index 4b92c791a..8407fc42e 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -114,6 +114,8 @@ class TestDiffDir(TestDvc): class TestDiffDirRepo(TestDiffDir): + maxDiff = None + def test(self): result = self.dvc.diff(self.a_ref, target=self.DATA_DIR) test_dct = { @@ -140,6 +142,8 @@ class TestDiffDirRepo(TestDiffDir): class TestDiffDirRepoDeletedFile(TestDiffDir): + maxDiff = None + def setUp(self): super(TestDiffDirRepoDeletedFile, self).setUp() From 505b76a2b8b715193a8cad7ceae1a8260aee514d Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Fri, 5 Apr 2019 13:39:15 +0600 Subject: [PATCH 25/25] repo: refactoring of dvc/repo/diff.py --- dvc/repo/diff.py | 70 ++++++++++++++++++++++-------------------------- 1 file changed, 32 insertions(+), 38 deletions(-) diff --git a/dvc/repo/diff.py b/dvc/repo/diff.py index 0bd186d8c..534385f77 100644 --- a/dvc/repo/diff.py +++ b/dvc/repo/diff.py @@ -42,17 +42,26 @@ def _file_not_exists(error, result): raise error -def _extract_dir(self, output): +def _extract_dir(self, dir_not_exists, output): """Extract the content of dvc tree file Args: self(object) - Repo class instance + dir_not_exists(bool) - flag for directory existence output(object) - OutputLOCAL class instance Returns: dict - dictionary with keys - paths to file in .dvc/cache values -checksums for that files """ - lst = output.dir_cache - return {i["relpath"]: i["md5"] for i in lst} + if not dir_not_exists: + lst = output.dir_cache + return {i["relpath"]: i["md5"] for i in lst} + return {} + + +def _get_dir_info(dir_not_exists, a_output): + if not dir_not_exists: + return str(a_output), a_output.checksum + return "", "" def _ident_files(a_entries, b_entries): @@ -122,6 +131,13 @@ def _check_local_cache(a_out, is_checked): return False +def _is_dir(path, a_outs, b_outs): + if a_outs.get(path): + return a_outs[path].is_dir_cache + else: + return b_outs[path].is_dir_cache + + def _get_diff_outs(self, diff_dct): self.tree = diff_dct[DIFF_A_TREE] a_outs = {str(out): out for st in self.stages() for out in st.outs} @@ -142,33 +158,7 @@ def _get_diff_outs(self, diff_dct): results[path][DIFF_B_OUTPUT] = b_outs.get(path) results[path][DIFF_IS_NEW] = path not in a_outs results[path][DIFF_DELETED] = path not in b_outs - if path in a_outs: - results[path][DIFF_IS_DIR] = a_outs[path].is_dir_cache - else: - results[path][DIFF_IS_DIR] = b_outs[path].is_dir_cache - # if path in a_outs and path in b_outs: - # results[path] = { - # DIFF_A_OUTPUT: a_outs[path], - # DIFF_B_OUTPUT: b_outs[path], - # DIFF_IS_NEW: False, - # DIFF_DELETED: False, - # # possible drawback: regular file ->directory movement - # DIFF_IS_DIR: a_outs[path].is_dir_cache, - # } - # elif path in a_outs: - # results[path] = { - # DIFF_A_OUTPUT: a_outs[path], - # DIFF_IS_NEW: False, - # DIFF_DELETED: True, - # DIFF_IS_DIR: a_outs[path].is_dir_cache, - # } - # else: - # results[path] = { - # DIFF_B_OUTPUT: b_outs[path], - # DIFF_IS_NEW: True, - # DIFF_DELETED: False, - # DIFF_IS_DIR: b_outs[path].is_dir_cache, - # } + results[path][DIFF_IS_DIR] = _is_dir(path, a_outs, b_outs) if non_local_cache: logger.warning( "Diff is not supported for non-local outputs. Ignoring: {}".format( @@ -184,14 +174,18 @@ def _diff_dir(self, target, diff_dct): result[DIFF_IS_DIR] = True a_entries, b_entries = {}, {} try: - if not diff_dct[DIFF_IS_NEW]: - result[DIFF_OLD_FILE] = target - result[DIFF_OLD_CHECKSUM] = diff_dct[DIFF_A_OUTPUT].checksum - a_entries = _extract_dir(self, diff_dct[DIFF_A_OUTPUT]) - if not diff_dct[DIFF_DELETED]: - result[DIFF_NEW_FILE] = target - result[DIFF_NEW_CHECKSUM] = diff_dct[DIFF_B_OUTPUT].checksum - b_entries = _extract_dir(self, diff_dct[DIFF_B_OUTPUT]) + a_entries = _extract_dir( + self, diff_dct[DIFF_IS_NEW], diff_dct[DIFF_A_OUTPUT] + ) + b_entries = _extract_dir( + self, diff_dct[DIFF_DELETED], diff_dct[DIFF_B_OUTPUT] + ) + result[DIFF_OLD_FILE], result[DIFF_OLD_CHECKSUM] = _get_dir_info( + diff_dct[DIFF_IS_NEW], diff_dct[DIFF_A_OUTPUT] + ) + result[DIFF_NEW_FILE], result[DIFF_NEW_CHECKSUM] = _get_dir_info( + diff_dct[DIFF_DELETED], diff_dct[DIFF_B_OUTPUT] + ) result.update(_get_tree_changes(self, a_entries, b_entries)) except IOError as e: _file_not_exists(e, result)