Merge lp:~jelmer/brz-git/iter-changes-missing into lp:brz-git

[Jelmer Vernooij]

Allow missing items in iter_changes.

Allow missing items in iter_changes.

[Jelmer Vernooij (jelmer)]

review: Approve

=== modified file 'filegraph.py'
--- filegraph.py	2018-03-17 19:20:55 +0000
+++ filegraph.py	2018-03-26 00:33:37 +0000
@@ -44,7 +44,7 @@
         target_mode, target_sha = tree_lookup_path(self.store.__getitem__,
             commit.tree, path)
         if path == '':
-            target_mode = stat.S_IFDIR | 0644
+            target_mode = stat.S_IFDIR
         if target_mode is None:
             raise AssertionError("sha %r for %r in %r" % (target_sha, path, commit_id))
         while True:
@@ -58,7 +58,7 @@
                 else:
                     parent_commits.append(parent_commit)
                 if path == '':
-                    mode = stat.S_IFDIR | 0644
+                    mode = stat.S_IFDIR
                 # Candidate found iff, mode or text changed,
                 # or is a directory that didn't previously exist.
                 if mode != target_mode or (

=== modified file 'tests/test_workingtree.py'
--- tests/test_workingtree.py	2018-03-22 17:40:16 +0000
+++ tests/test_workingtree.py	2018-03-26 00:33:37 +0000
@@ -19,9 +19,19 @@
 
 from __future__ import absolute_import
 
+import os
+import stat
+
+from dulwich.objects import (
+    Blob,
+    Tree,
+    ZERO_SHA,
+    )
+
 from .... import conflicts as _mod_conflicts
 from ..workingtree import (
     FLAG_STAGEMASK,
+    changes_between_git_tree_and_working_copy,
     )
 from ....tests import TestCaseWithTransport
 
@@ -51,3 +61,115 @@
         self.assertTrue(self.tree.is_versioned('a'))
         self.tree.revert(['a'])
         self.assertFalse(self.tree.is_versioned('a'))
+
+
+class ChangesBetweenGitTreeAndWorkingCopyTests(TestCaseWithTransport):
+
+    def setUp(self):
+        super(ChangesBetweenGitTreeAndWorkingCopyTests, self).setUp()
+        self.wt = self.make_branch_and_tree('.', format='git')
+
+    def expectDelta(self, expected_changes,
+                    expected_extras=None, want_unversioned=False):
+        store = self.wt.branch.repository._git.object_store
+        try:
+            tree_id = store[self.wt.branch.repository._git.head()].tree
+        except KeyError:
+            tree_id = None
+        changes, extras = changes_between_git_tree_and_working_copy(
+            store, tree_id, self.wt, want_unversioned=want_unversioned)
+        self.assertEqual(expected_changes, list(changes))
+        if expected_extras is None:
+            expected_extras = set()
+        self.assertEqual(set(expected_extras), set(extras))
+
+    def test_empty(self):
+        self.expectDelta(
+            [((None, ''), (None, stat.S_IFDIR), (None, Tree().id))])
+
+    def test_added_file(self):
+        self.build_tree(['a'])
+        self.wt.add(['a'])
+        a = Blob.from_string('contents of a\n')
+        t = Tree()
+        t.add("a", stat.S_IFREG | 0o644, a.id)
+        self.expectDelta(
+            [((None, ''), (None, stat.S_IFDIR), (None, t.id)),
+             ((None, 'a'), (None, stat.S_IFREG | 0o644), (None, a.id))])
+
+    def test_added_unknown_file(self):
+        self.build_tree(['a'])
+        t = Tree()
+        self.expectDelta(
+            [((None, ''), (None, stat.S_IFDIR), (None, t.id))])
+        a = Blob.from_string('contents of a\n')
+        t = Tree()
+        t.add("a", stat.S_IFREG | 0o644, a.id)
+        self.expectDelta(
+            [((None, ''), (None, stat.S_IFDIR), (None, t.id)),
+             ((None, 'a'), (None, stat.S_IFREG | 0o644), (None, a.id))],
+            ['a'],
+            want_unversioned=True)
+
+    def test_missing_added_file(self):
+        self.build_tree(['a'])
+        self.wt.add(['a'])
+        os.unlink('a')
+        a = Blob.from_string('contents of a\n')
+        t = Tree()
+        t.add("a", 0, ZERO_SHA)
+        self.expectDelta(
+            [((None, ''), (None, stat.S_IFDIR), (None, t.id)),
+             ((None, 'a'), (None, 0), (None, ZERO_SHA))],
+            [])
+
+    def test_missing_versioned_file(self):
+        self.build_tree(['a'])
+        self.wt.add(['a'])
+        self.wt.commit('')
+        os.unlink('a')
+        a = Blob.from_string('contents of a\n')
+        oldt = Tree()
+        oldt.add("a", stat.S_IFREG | 0o644, a.id)
+        newt = Tree()
+        newt.add("a", 0, ZERO_SHA)
+        self.expectDelta(
+                [(('', ''), (stat.S_IFDIR, stat.S_IFDIR), (oldt.id, newt.id)),
+                 (('a', 'a'), (stat.S_IFREG|0o644, 0), (a.id, ZERO_SHA))])
+
+    def test_versioned_replace_by_dir(self):
+        self.build_tree(['a'])
+        self.wt.add(['a'])
+        self.wt.commit('')
+        os.unlink('a')
+        os.mkdir('a')
+        olda = Blob.from_string('contents of a\n')
+        oldt = Tree()
+        oldt.add("a", stat.S_IFREG | 0o644, olda.id)
+        newt = Tree()
+        newa = Tree()
+        newt.add("a", stat.S_IFDIR, newa.id)
+        self.expectDelta([
+            (('', ''),
+            (stat.S_IFDIR, stat.S_IFDIR),
+            (oldt.id, newt.id)),
+            (('a', 'a'), (stat.S_IFREG | 0o644, stat.S_IFDIR), (olda.id, newa.id))
+            ], want_unversioned=False)
+        self.expectDelta([
+            (('', ''),
+            (stat.S_IFDIR, stat.S_IFDIR),
+            (oldt.id, newt.id)),
+            (('a', 'a'), (stat.S_IFREG | 0o644, stat.S_IFDIR), (olda.id, newa.id))
+            ], want_unversioned=True)
+
+    def test_extra(self):
+        self.build_tree(['a'])
+        newa = Blob.from_string('contents of a\n')
+        newt = Tree()
+        newt.add("a", stat.S_IFREG | 0o644, newa.id)
+        self.expectDelta([
+            ((None, ''),
+            (None, stat.S_IFDIR),
+            (None, newt.id)),
+            ((None, 'a'), (None, stat.S_IFREG | 0o644), (None, newa.id))
+            ], ['a'], want_unversioned=True)

=== modified file 'tree.py'
--- tree.py	2018-03-25 13:27:11 +0000
+++ tree.py	2018-03-26 00:33:37 +0000
@@ -559,12 +559,15 @@
     return ret
 
 
-def changes_from_git_changes(changes, mapping, specific_files=None, include_unchanged=False):
+def changes_from_git_changes(changes, mapping, specific_files=None, include_unchanged=False,
+                             target_extras=None):
     """Create a iter_changes-like generator from a git stream.
 
     source and target are iterators over tuples with:
         (filename, sha, mode)
     """
+    if target_extras is None:
+        target_extras = set()
     for (oldpath, newpath), (oldmode, newmode), (oldsha, newsha) in changes:
         if not (specific_files is None or
                 (oldpath is not None and osutils.is_inside_or_parent_of_any(specific_files, oldpath)) or
@@ -581,12 +584,16 @@
             oldkind = None
             oldname = None
             oldparent = None
+            oldversioned = False
         else:
+            oldversioned = True
             oldpath = oldpath.decode("utf-8")
-            if oldmode is None:
-                raise ValueError
-            oldexe = mode_is_executable(oldmode)
-            oldkind = mode_kind(oldmode)
+            if oldmode:
+                oldexe = mode_is_executable(oldmode)
+                oldkind = mode_kind(oldmode)
+            else:
+                oldexe = False
+                oldkind = None
             if oldpath == u'':
                 oldparent = None
                 oldname = ''
@@ -599,14 +606,16 @@
             newkind = None
             newname = None
             newparent = None
+            newversioned = False
         else:
-            newpath = newpath.decode("utf-8")
-            if newmode is not None:
+            newversioned = (newpath not in target_extras)
+            if newmode:
                 newexe = mode_is_executable(newmode)
                 newkind = mode_kind(newmode)
             else:
                 newexe = False
                 newkind = None
+            newpath = newpath.decode("utf-8")
             if newpath == u'':
                 newparent = None
                 newname = u''
@@ -618,7 +627,7 @@
             oldpath == newpath):
             continue
         yield (fileid, (oldpath, newpath), (oldsha != newsha),
-             (oldpath is not None, newpath is not None),
+             (oldversioned, newversioned),
              (oldparent, newparent), (oldname, newname),
              (oldkind, newkind), (oldexe, newexe))
 
@@ -638,28 +647,38 @@
     def compare(self, want_unchanged=False, specific_files=None,
                 extra_trees=None, require_versioned=False, include_root=False,
                 want_unversioned=False):
-        changes = self._iter_git_changes(want_unchanged=want_unchanged,
-                require_versioned=require_versioned,
-                specific_files=specific_files,
-                extra_trees=extra_trees)
-        source_fileid_map = self.source._fileid_map
-        target_fileid_map = self.target._fileid_map
-        return tree_delta_from_git_changes(changes, self.target.mapping,
-            (source_fileid_map, target_fileid_map),
-            specific_files=specific_files, include_root=include_root)
+        with self.lock_read():
+            changes, target_extras = self._iter_git_changes(
+                    want_unchanged=want_unchanged,
+                    require_versioned=require_versioned,
+                    specific_files=specific_files,
+                    extra_trees=extra_trees,
+                    want_unversioned=want_unversioned)
+            source_fileid_map = self.source._fileid_map
+            target_fileid_map = self.target._fileid_map
+            return tree_delta_from_git_changes(changes, self.target.mapping,
+                (source_fileid_map, target_fileid_map),
+                specific_files=specific_files, include_root=include_root)
 
     def iter_changes(self, include_unchanged=False, specific_files=None,
                      pb=None, extra_trees=[], require_versioned=True,
                      want_unversioned=False):
-        changes = self._iter_git_changes(want_unchanged=include_unchanged,
-                require_versioned=require_versioned,
-                specific_files=specific_files,
-                extra_trees=extra_trees)
-        return changes_from_git_changes(changes, self.target.mapping,
-            specific_files=specific_files, include_unchanged=include_unchanged)
+        with self.lock_read():
+            changes, target_extras = self._iter_git_changes(
+                    want_unchanged=include_unchanged,
+                    require_versioned=require_versioned,
+                    specific_files=specific_files,
+                    extra_trees=extra_trees,
+                    want_unversioned=want_unversioned)
+            return changes_from_git_changes(
+                    changes, self.target.mapping,
+                    specific_files=specific_files,
+                    include_unchanged=include_unchanged,
+                    target_extras=target_extras)
 
     def _iter_git_changes(self, want_unchanged=False, specific_files=None,
-            require_versioned=False, extra_trees=None):
+            require_versioned=False, extra_trees=None,
+            want_unversioned=False):
         raise NotImplementedError(self._iter_git_changes)
 
 
@@ -676,7 +695,8 @@
                 isinstance(target, GitRevisionTree))
 
     def _iter_git_changes(self, want_unchanged=False, specific_files=None,
-            require_versioned=True, extra_trees=None):
+            require_versioned=True, extra_trees=None,
+            want_unversioned=False):
         trees = [self.source]
         if extra_trees is not None:
             trees.extend(extra_trees)
@@ -692,7 +712,7 @@
             store = self.source._repository._git.object_store
         return self.source._repository._git.object_store.tree_changes(
             self.source.tree, self.target.tree, want_unchanged=want_unchanged,
-            include_trees=True, change_type_same=True)
+            include_trees=True, change_type_same=True), set()
 
 
 _mod_tree.InterTree.register_optimiser(InterGitRevisionTrees)

=== modified file 'workingtree.py'
--- workingtree.py	2018-03-24 14:16:00 +0000
+++ workingtree.py	2018-03-26 00:33:37 +0000
@@ -35,8 +35,9 @@
     changes_from_tree,
     cleanup_mode,
     commit_tree,
+    index_entry_from_path,
     index_entry_from_stat,
-    iter_fresh_blobs,
+    iter_fresh_entries,
     blob_from_path_and_stat,
     FLAG_STAGEMASK,
     validate_path,
@@ -49,6 +50,7 @@
     Tree,
     S_IFGITLINK,
     S_ISGITLINK,
+    ZERO_SHA,
     )
 from dulwich.repo import Repo
 import os
@@ -1185,7 +1187,8 @@
                 isinstance(target, GitWorkingTree))
 
     def _iter_git_changes(self, want_unchanged=False, specific_files=None,
-            require_versioned=False, include_root=False, extra_trees=None):
+            require_versioned=False, extra_trees=None,
+            want_unversioned=False):
         trees = [self.source]
         if extra_trees is not None:
             trees.extend(extra_trees)
@@ -1198,85 +1201,57 @@
             return changes_between_git_tree_and_working_copy(
                 self.source.store, self.source.tree,
                 self.target, want_unchanged=want_unchanged,
-                include_root=include_root)
-
-    def compare(self, want_unchanged=False, specific_files=None,
-                extra_trees=None, require_versioned=False, include_root=False,
-                want_unversioned=False):
-        with self.lock_read():
-            changes = self._iter_git_changes(
-                    want_unchanged=want_unchanged,
-                    specific_files=specific_files,
-                    require_versioned=require_versioned,
-                    include_root=include_root,
-                    extra_trees=extra_trees)
-            source_fileid_map = self.source._fileid_map
-            target_fileid_map = self.target._fileid_map
-            ret = tree_delta_from_git_changes(changes, self.target.mapping,
-                (source_fileid_map, target_fileid_map),
-                specific_files=specific_files, require_versioned=require_versioned,
-                include_root=include_root)
-            if want_unversioned:
-                for e in self.target.extras():
-                    ret.unversioned.append(
-                        (osutils.normalized_filename(e)[0], None,
-                        osutils.file_kind(self.target.abspath(e))))
-            return ret
-
-    def iter_changes(self, include_unchanged=False, specific_files=None,
-                     pb=None, extra_trees=[], require_versioned=True,
-                     want_unversioned=False):
-        with self.lock_read():
-            changes = self._iter_git_changes(
-                    want_unchanged=include_unchanged,
-                    specific_files=specific_files,
-                    require_versioned=require_versioned,
-                    extra_trees=extra_trees)
-            if want_unversioned:
-                changes = itertools.chain(
-                        changes,
-                        untracked_changes(self.target))
-            return changes_from_git_changes(
-                    changes, self.target.mapping,
-                    specific_files=specific_files,
-                    include_unchanged=include_unchanged)
+                want_unversioned=want_unversioned)
 
 
 tree.InterTree.register_optimiser(InterIndexGitTree)
 
 
-def untracked_changes(tree):
-    for e in tree.extras():
-        ap = tree.abspath(e)
-        st = os.lstat(ap)
-        try:
-            np, accessible  = osutils.normalized_filename(e)
-        except UnicodeDecodeError:
-            raise errors.BadFilenameEncoding(
-                e, osutils._fs_enc)
-        if stat.S_ISDIR(st.st_mode):
-            obj_id = Tree().id
-        else:
-            obj_id = blob_from_path_and_stat(ap.encode('utf-8'), st).id
-        yield ((None, np), (None, st.st_mode), (None, obj_id))
-
-
-def changes_between_git_tree_and_index(store, from_tree_sha, target,
-        want_unchanged=False, update_index=False):
-    """Determine the changes between a git tree and a working tree with index.
-
-    """
-    to_tree_sha = target.index.commit(store)
-    return store.tree_changes(from_tree_sha, to_tree_sha, include_trees=True,
-            want_unchanged=want_unchanged, change_type_same=True)
-
-
 def changes_between_git_tree_and_working_copy(store, from_tree_sha, target,
-        want_unchanged=False, update_index=False, include_root=False):
+        want_unchanged=False, want_unversioned=False):
     """Determine the changes between a git tree and a working tree with index.
 
     """
-    blobs = iter_fresh_blobs(target.index, target.abspath('.').encode(sys.getfilesystemencoding()))
-    to_tree_sha = commit_tree(store, blobs)
-    return store.tree_changes(from_tree_sha, to_tree_sha, include_trees=True,
-            want_unchanged=want_unchanged, change_type_same=True)
+    extras = set()
+    blobs = {}
+    # Report dirified directories to commit_tree first, so that they can be
+    # replaced with non-empty directories if they have contents.
+    dirified = []
+    target_root_path = target.abspath('.').encode(sys.getfilesystemencoding())
+    for path, index_entry in target.index.iteritems():
+        try:
+            live_entry = index_entry_from_path(
+                    target.abspath(path.decode('utf-8')).encode(osutils._fs_enc))
+        except EnvironmentError as e:
+            if e.errno == errno.ENOENT:
+                # Entry was removed; keep it listed, but mark it as gone.
+                blobs[path] = (ZERO_SHA, 0)
+            elif e.errno == errno.EISDIR:
+                # Entry was turned into a directory
+                dirified.append((path, Tree().id, stat.S_IFDIR))
+                store.add_object(Tree())
+            else:
+                raise
+        else:
+            blobs[path] = (live_entry.sha, cleanup_mode(live_entry.mode))
+    if want_unversioned:
+        for e in target.extras():
+            ap = target.abspath(e)
+            st = os.lstat(ap)
+            try:
+                np, accessible = osutils.normalized_filename(e)
+            except UnicodeDecodeError:
+                raise errors.BadFilenameEncoding(
+                    e, osutils._fs_enc)
+            if stat.S_ISDIR(st.st_mode):
+                blob = Tree()
+            else:
+                blob = blob_from_path_and_stat(ap.encode('utf-8'), st)
+            store.add_object(blob)
+            np = np.encode('utf-8')
+            blobs[np] = (blob.id, st.st_mode)
+            extras.add(np)
+    to_tree_sha = commit_tree(store, dirified + [(p, s, m) for (p, (s, m)) in blobs.iteritems()])
+    return store.tree_changes(
+        from_tree_sha, to_tree_sha, include_trees=True,
+        want_unchanged=want_unchanged, change_type_same=True), extras

=== modified file 'xfail'
--- xfail	2018-03-25 14:14:01 +0000
+++ xfail	2018-03-26 00:33:37 +0000
@@ -40,7 +40,6 @@
 breezy.tests.per_workingtree.test_annotate_iter.TestAnnotateIter.test_annotate_same_as_merge_parent(GitWorkingTreeFormat)
 breezy.tests.per_workingtree.test_annotate_iter.TestAnnotateIter.test_annotate_same_as_merge_parent_supersedes(GitWorkingTreeFormat)
 breezy.tests.per_workingtree.test_annotate_iter.TestAnnotateIter.test_annotate_same_as_parent(GitWorkingTreeFormat)
-breezy.tests.per_workingtree.test_commit.TestCommit.test_commit_aborted_does_not_apply_automatic_changes_bug_282402(GitWorkingTreeFormat)
 breezy.tests.per_workingtree.test_merge_from_branch.TestMergedBranch.test_file1_deleted_in_dir(GitWorkingTreeFormat)
 breezy.tests.per_workingtree.test_merge_from_branch.TestMergedBranch.test_file3_deleted_in_root(GitWorkingTreeFormat)
 breezy.tests.per_workingtree.test_merge_from_branch.TestMergedBranch.test_file3_in_root_conflicted(GitWorkingTreeFormat)