From f0225d85cd9963bb2c9a5b055e15da11a184965c Mon Sep 17 00:00:00 2001 From: Zhanibek Date: Tue, 26 Mar 2019 19:14:32 +0600 Subject: [PATCH] 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