Merge lp:~jelmer/brz/per-tree-git-revtree into lp:brz

[Jelmer Vernooij]

Run per_tree tests against GitRevisionTree.

Run per_tree tests against GitRevisionTree.

[Martin Packman (gz)]

Changes look reasonable enough to me.

review: Approve

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/land-brz/475/

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/land-brz/481/

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/land-brz/482/

=== modified file 'breezy/git/mapping.py'
--- breezy/git/mapping.py	2018-08-04 17:32:58 +0000
+++ breezy/git/mapping.py	2018-09-15 00:21:08 +0000
@@ -429,7 +429,7 @@
                     parents.append(lookup_parent_revid(p))
                 except KeyError:
                     parents.append(self.revision_id_foreign_to_bzr(p))
-            rev.parent_ids = tuple(parents)
+            rev.parent_ids = list(parents)
         unknown_extra_fields = []
         extra_lines = []
         for k, v in commit.extra:

=== modified file 'breezy/git/tests/test_mapping.py'
--- breezy/git/tests/test_mapping.py	2018-08-04 17:32:58 +0000
+++ breezy/git/tests/test_mapping.py	2018-09-15 00:21:08 +0000
@@ -117,7 +117,7 @@
         self.assertEqual(u"Committer", rev.committer)
         self.assertEqual(u"Author", rev.properties[u'author'])
         self.assertEqual(300, rev.timezone)
-        self.assertEqual((), rev.parent_ids)
+        self.assertEqual([], rev.parent_ids)
         self.assertEqual("5", rev.properties[u'author-timestamp'])
         self.assertEqual("180", rev.properties[u'author-timezone'])
         self.assertEqual(b"git-v1:" + c.id, rev.revision_id)

=== modified file 'breezy/git/tree.py'
--- breezy/git/tree.py	2018-08-18 21:01:10 +0000
+++ breezy/git/tree.py	2018-09-15 00:21:08 +0000
@@ -19,6 +19,7 @@
 
 from __future__ import absolute_import
 
+from collections import deque
 import errno
 from io import BytesIO
 import os
@@ -289,7 +290,7 @@
         try:
             revid = self.get_file_revision(path, file_id)
         except KeyError:
-            raise _mod_tree.FileTimestampUnavailable(path)
+            raise errors.NoSuchFile(path)
         try:
             rev = self._repository.get_revision(revid)
         except errors.NoSuchRevision:
@@ -311,13 +312,15 @@
     def path2id(self, path):
         if self.mapping.is_special_file(path):
             return None
+        if not self.is_versioned(path):
+            return None
         return self._fileid_map.lookup_file_id(osutils.safe_unicode(path))
 
     def all_file_ids(self):
-        return set(self._fileid_map.all_file_ids())
+        return {self.path2id(path) for path in self.all_versioned_paths()}
 
     def all_versioned_paths(self):
-        ret = set()
+        ret = {u''}
         todo = [(self.store, b'', self.tree)]
         while todo:
             (store, path, tree_id) = todo.pop()
@@ -326,10 +329,9 @@
             tree = store[tree_id]
             for name, mode, hexsha in tree.items():
                 subpath = posixpath.join(path, name)
+                ret.add(subpath.decode('utf-8'))
                 if stat.S_ISDIR(mode):
                     todo.append((store, subpath, hexsha))
-                else:
-                    ret.add(subpath.decode('utf-8'))
         return ret
 
     def get_root_id(self):
@@ -393,32 +395,33 @@
         if mode is None: # Root
             root_ie = self._get_dir_ie(b"", None)
         else:
-            parent_path = posixpath.dirname(from_dir.encode("utf-8"))
+            parent_path = posixpath.dirname(from_dir)
             parent_id = self._fileid_map.lookup_file_id(parent_path)
             if mode_kind(mode) == 'directory':
                 root_ie = self._get_dir_ie(from_dir.encode("utf-8"), parent_id)
             else:
                 root_ie = self._get_file_ie(store, from_dir.encode("utf-8"),
                     posixpath.basename(from_dir), mode, hexsha)
-        if from_dir != "" or include_root:
+        if include_root:
             yield (from_dir, "V", root_ie.kind, root_ie.file_id, root_ie)
         todo = []
         if root_ie.kind == 'directory':
-            todo.append((store, from_dir.encode("utf-8"), hexsha, root_ie.file_id))
+            todo.append((store, from_dir.encode("utf-8"), b"", hexsha, root_ie.file_id))
         while todo:
-            (store, path, hexsha, parent_id) = todo.pop()
+            (store, path, relpath, hexsha, parent_id) = todo.pop()
             tree = store[hexsha]
             for name, mode, hexsha in tree.iteritems():
                 if self.mapping.is_special_file(name):
                     continue
                 child_path = posixpath.join(path, name)
+                child_relpath = posixpath.join(relpath, name)
                 if stat.S_ISDIR(mode):
                     ie = self._get_dir_ie(child_path, parent_id)
                     if recursive:
-                        todo.append((store, child_path, hexsha, ie.file_id))
+                        todo.append((store, child_path, child_relpath, hexsha, ie.file_id))
                 else:
                     ie = self._get_file_ie(store, child_path, name, mode, hexsha, parent_id)
-                yield child_path.decode('utf-8'), "V", ie.kind, ie.file_id, ie
+                yield child_relpath.decode('utf-8'), "V", ie.kind, ie.file_id, ie
 
     def _get_file_ie(self, store, path, name, mode, hexsha, parent_id):
         if not isinstance(path, bytes):
@@ -449,7 +452,7 @@
     def iter_child_entries(self, path, file_id=None):
         (store, mode, tree_sha) = self._lookup_path(path)
 
-        if not stat.S_ISDIR(mode):
+        if mode is not None and not stat.S_ISDIR(mode):
             return
 
         encoded_path = path.encode('utf-8')
@@ -476,25 +479,38 @@
                 specific_files = None
             else:
                 specific_files = set([p.encode('utf-8') for p in specific_files])
-        todo = [(self.store, b"", self.tree, None)]
+        todo = deque([(self.store, b"", self.tree, self.get_root_id())])
+        if specific_files is None or u"" in specific_files:
+            yield u"", self._get_dir_ie(b"", None)
         while todo:
-            store, path, tree_sha, parent_id = todo.pop()
-            ie = self._get_dir_ie(path, parent_id)
-            if specific_files is None or path in specific_files:
-                yield path.decode("utf-8"), ie
+            store, path, tree_sha, parent_id = todo.popleft()
             tree = store[tree_sha]
+            extradirs = []
             for name, mode, hexsha in tree.iteritems():
                 if self.mapping.is_special_file(name):
                     continue
                 child_path = posixpath.join(path, name)
+                child_path_decoded = child_path.decode('utf-8')
                 if stat.S_ISDIR(mode):
                     if (specific_files is None or
                         any(filter(lambda p: p.startswith(child_path), specific_files))):
-                        todo.append((store, child_path, hexsha, ie.file_id))
-                elif specific_files is None or child_path in specific_files:
-                    yield (child_path.decode("utf-8"),
-                            self._get_file_ie(store, child_path, name, mode, hexsha,
-                           ie.file_id))
+                        extradirs.append(
+                            (store, child_path, hexsha, self.path2id(child_path_decoded)))
+                if specific_files is None or child_path in specific_files:
+                    if stat.S_ISDIR(mode):
+                        yield (child_path_decoded,
+                               self._get_dir_ie(child_path, parent_id))
+                    else:
+                        yield (child_path_decoded,
+                               self._get_file_ie(store, child_path, name, mode,
+                                   hexsha, parent_id))
+            todo.extendleft(reversed(extradirs))
+
+    def iter_references(self):
+        if self.supports_tree_reference():
+            for path, entry in self.iter_entries_by_dir():
+                if entry.kind == 'tree-reference':
+                    yield path, self.mapping.generate_file_id(b'')
 
     def get_revision_id(self):
         """See RevisionTree.get_revision_id."""
@@ -509,6 +525,12 @@
         (store, mode, hexsha) = self._lookup_path(path)
         return ("GIT", hexsha)
 
+    def get_file_size(self, path, file_id=None):
+        (store, mode, hexsha) = self._lookup_path(path)
+        if stat.S_ISREG(mode):
+            return len(store[hexsha].data)
+        return None
+
     def get_file_text(self, path, file_id=None):
         """See RevisionTree.get_file_text."""
         (store, mode, hexsha) = self._lookup_path(path)
@@ -551,7 +573,7 @@
             contents = store[hexsha].data
             return (kind, len(contents), executable, osutils.sha_string(contents))
         elif kind == 'symlink':
-            return (kind, None, None, store[hexsha].data)
+            return (kind, None, None, store[hexsha].data.decode('utf-8'))
         elif kind == 'tree-reference':
             nested_repo = self._get_nested_repository(path)
             return (kind, None, None,
@@ -605,6 +627,30 @@
                            for key, line in annotator.annotate_flat(this_key)]
             return annotations
 
+    def _get_rules_searcher(self, default_searcher):
+        return default_searcher
+
+    def walkdirs(self, prefix=u""):
+        (store, mode, hexsha) = self._lookup_path(prefix)
+        todo = deque([(store, prefix.encode('utf-8'), hexsha, self.path2id(prefix))])
+        while todo:
+            store, path, tree_sha, parent_id = todo.popleft()
+            path_decoded = path.decode('utf-8')
+            tree = store[tree_sha]
+            children = []
+            for name, mode, hexsha in tree.iteritems():
+                if self.mapping.is_special_file(name):
+                    continue
+                child_path = posixpath.join(path, name)
+                file_id = self.path2id(child_path.decode('utf-8'))
+                if stat.S_ISDIR(mode):
+                    todo.append((store, child_path, hexsha, file_id))
+                children.append(
+                    (child_path.decode('utf-8'), name.decode('utf-8'),
+                        mode_kind(mode), None,
+                        file_id, mode_kind(mode)))
+            yield (path_decoded, parent_id), children
+
 
 def tree_delta_from_git_changes(changes, mapping,
         fileid_maps, specific_files=None,

=== modified file 'breezy/tests/per_tree/__init__.py'
--- breezy/tests/per_tree/__init__.py	2018-09-14 14:13:30 +0000
+++ breezy/tests/per_tree/__init__.py	2018-09-15 00:21:08 +0000
@@ -31,6 +31,8 @@
     transform,
     transport,
     )
+from breezy.git.tree import GitRevisionTree
+from breezy.git.workingtree import GitWorkingTreeFormat
 from breezy.tests.per_controldir.test_controldir import TestCaseWithControlDir
 from breezy.tests.per_workingtree import (
     make_scenarios as wt_make_scenarios,
@@ -91,10 +93,15 @@
 
 class TestTreeImplementationSupport(tests.TestCaseWithTransport):
 
+    def test_revision_tree_from_workingtree_bzr(self):
+        tree = self.make_branch_and_tree('.', format='bzr')
+        tree = revision_tree_from_workingtree(self, tree)
+        self.assertIsInstance(tree, RevisionTree)
+
     def test_revision_tree_from_workingtree(self):
-        tree = self.make_branch_and_tree('.')
+        tree = self.make_branch_and_tree('.', format='git')
         tree = revision_tree_from_workingtree(self, tree)
-        self.assertIsInstance(tree, RevisionTree)
+        self.assertIsInstance(tree, GitRevisionTree)
 
 
 class TestCaseWithTree(TestCaseWithControlDir):
@@ -279,6 +286,8 @@
     DirStateRevisionTree by committing a working tree to create the revision
     tree.
     """
+    # TODO(jelmer): Test MemoryTree here
+    # TODO(jelmer): Test GitMemoryTree here
     scenarios = wt_make_scenarios(transport_server, transport_readonly_server,
         formats)
     # now adjust the scenarios and add the non-working-tree tree scenarios.
@@ -290,6 +299,9 @@
     scenarios.append((RevisionTree.__name__,
         create_tree_scenario(transport_server, transport_readonly_server,
         workingtree_format, revision_tree_from_workingtree,)))
+    scenarios.append((GitRevisionTree.__name__,
+        create_tree_scenario(transport_server, transport_readonly_server,
+        GitWorkingTreeFormat(), revision_tree_from_workingtree,)))
 
     # also test WorkingTree4/5's RevisionTree implementation which is
     # specialised.
@@ -347,7 +359,7 @@
         'walkdirs',
         ]
     submod_tests = loader.loadTestsFromModuleNames(
-        ['breezy.tests.per_tree.test_' + name
+        [__name__ + '.test_' + name
          for name in per_tree_mod_names])
     scenarios = make_scenarios(
         tests.default_transport,

=== modified file 'breezy/tests/per_tree/test_list_files.py'
--- breezy/tests/per_tree/test_list_files.py	2018-02-26 13:36:15 +0000
+++ breezy/tests/per_tree/test_list_files.py	2018-09-15 00:21:08 +0000
@@ -92,7 +92,7 @@
         try:
             actual = [(path, status, kind, file_id)
                       for path, status, kind, file_id, ie in
-                          tree.list_files(from_dir='b')]
+                          tree.list_files(from_dir=u'b')]
         finally:
             tree.unlock()
         self.assertEqual(expected, actual)

=== modified file 'breezy/tests/per_tree/test_path_content_summary.py'
--- breezy/tests/per_tree/test_path_content_summary.py	2018-07-27 18:57:21 +0000
+++ breezy/tests/per_tree/test_path_content_summary.py	2018-09-15 00:21:08 +0000
@@ -142,8 +142,12 @@
         tree = self.make_branch_and_tree('tree')
         self.build_tree(['tree/path/'])
         tree.add(['path'])
-        summary = self._convert_tree(tree).path_content_summary('path')
-        self.assertEqual(('directory', None, None, None), summary)
+        converted_tree = self._convert_tree(tree)
+        summary = converted_tree.path_content_summary('path')
+        if converted_tree.has_versioned_directories() or converted_tree.has_filename('path'):
+            self.assertEqual(('directory', None, None, None), summary)
+        else:
+            self.assertEqual(('missing', None, None, None), summary)
 
     def test_tree_content_summary(self):
         tree = self.make_branch_and_tree('tree')

=== modified file 'breezy/tests/per_tree/test_test_trees.py'
--- breezy/tests/per_tree/test_test_trees.py	2018-07-27 18:57:21 +0000
+++ breezy/tests/per_tree/test_test_trees.py	2018-09-15 00:21:08 +0000
@@ -141,7 +141,7 @@
         self.addCleanup(tree.unlock)
         self.assertEqual([], tree.get_parent_ids())
         self.assertEqual([], tree.conflicts())
-        if tree.has_versioned_directories():
+        if tree.has_versioned_directories() or not tree.has_filename('b'):
             self.assertEqual([], list(tree.unknowns()))
         else:
             self.assertEqual(['b'], list(tree.unknowns()))
@@ -169,7 +169,7 @@
         self.addCleanup(tree.unlock)
         self.assertEqual([], tree.get_parent_ids())
         self.assertEqual([], tree.conflicts())
-        if tree.has_versioned_directories():
+        if tree.has_versioned_directories() or not tree.has_filename('1top-dir/1dir-in-1topdir'):
             self.assertEqual([], list(tree.unknowns()))
         else:
             self.assertEqual(['1top-dir/1dir-in-1topdir'], list(tree.unknowns()))
@@ -218,7 +218,7 @@
         self.addCleanup(tree.unlock)
         self.assertEqual([], tree.get_parent_ids())
         self.assertEqual([], tree.conflicts())
-        if tree.has_versioned_directories():
+        if tree.has_versioned_directories() or not tree.has_filename('1top-dir/1dir-in-1topdir'):
             self.assertEqual([], list(tree.unknowns()))
         else:
             self.assertEqual(['1top-dir/1dir-in-1topdir'], list(tree.unknowns()))

=== modified file 'breezy/tests/per_tree/test_walkdirs.py'
--- breezy/tests/per_tree/test_walkdirs.py	2018-03-17 21:00:51 +0000
+++ breezy/tests/per_tree/test_walkdirs.py	2018-09-15 00:21:08 +0000
@@ -19,6 +19,7 @@
 import os
 
 from breezy import tests
+from breezy.mutabletree import MutableTree
 from breezy.osutils import has_symlinks
 from breezy.tests.per_tree import TestCaseWithTree
 
@@ -26,36 +27,42 @@
 class TestWalkdirs(TestCaseWithTree):
 
     def get_all_subdirs_expected(self, tree, symlinks):
-        dirblocks = [
-            (('', tree.path2id('')),
-             [('0file', '0file', 'file', None, tree.path2id('0file'), 'file'),
-              ('1top-dir', '1top-dir', 'directory', None,
-               tree.path2id('1top-dir'), 'directory'),
-              (u'2utf\u1234file', u'2utf\u1234file', 'file', None,
-               tree.path2id(u'2utf\u1234file'), 'file'),
-              ]),
-            (('1top-dir', tree.path2id('1top-dir')),
-             [('1top-dir/0file-in-1topdir', '0file-in-1topdir',
-               'file', None, tree.path2id('1top-dir/0file-in-1topdir'), 'file'),
-              ('1top-dir/1dir-in-1topdir', '1dir-in-1topdir',
-               'directory',
-               None if tree.has_versioned_directories() else os.stat(tree.abspath('1top-dir/1dir-in-1topdir')),
+        empty_dirs_present = (tree.has_versioned_directories() or
+                              isinstance(tree, MutableTree))
+        empty_dirs_are_versioned = tree.has_versioned_directories()
+        dirblocks = {}
+
+        dirblocks[''] = [
+            ('0file', '0file', 'file', None,
+                tree.path2id('0file'), 'file'),
+            ('1top-dir', '1top-dir', 'directory', None,
+                tree.path2id('1top-dir'), 'directory'),
+            (u'2utf\u1234file', u'2utf\u1234file', 'file', None,
+                tree.path2id(u'2utf\u1234file'), 'file')]
+
+        dirblocks['1top-dir'] = [
+            ('1top-dir/0file-in-1topdir', '0file-in-1topdir',
+               'file', None, tree.path2id('1top-dir/0file-in-1topdir'), 'file')]
+        if empty_dirs_present:
+            dirblocks['1top-dir'].append(
+            ('1top-dir/1dir-in-1topdir', '1dir-in-1topdir', 'directory',
+                None if empty_dirs_are_versioned else os.stat(tree.abspath('1top-dir/1dir-in-1topdir')),
                tree.path2id('1top-dir/1dir-in-1topdir'),
-               'directory' if tree.has_versioned_directories() else None,
-              )]),
-            (('1top-dir/1dir-in-1topdir', tree.path2id('1top-dir/1dir-in-1topdir')),
-             []),
-            ]
+               'directory' if empty_dirs_are_versioned else None))
+            dirblocks['1top-dir/1dir-in-1topdir'] = []
         if symlinks:
-            dirblocks[0][1].append(('symlink', 'symlink', 'symlink', None,
-                                    tree.path2id('symlink'), 'symlink'))
-        return dirblocks
+            dirblocks[''].append(
+                ('symlink', 'symlink', 'symlink', None,
+                 tree.path2id('symlink'), 'symlink'))
+        return [((path, tree.path2id(path)), list(sorted(entries)))
+                for (path, entries) in dirblocks.items()]
 
     def test_walkdir_root(self):
         tree = self.get_tree_with_subdirs_and_all_supported_content_types(
             has_symlinks())
         tree.lock_read()
-        expected_dirblocks = self.get_all_subdirs_expected(tree, has_symlinks())
+        expected_dirblocks = self.get_all_subdirs_expected(
+            tree, has_symlinks())
         # test that its iterable by iterating
         result = []
         for dirinfo, block in tree.walkdirs():

=== modified file 'breezy/tests/test_selftest.py'
--- breezy/tests/test_selftest.py	2018-08-26 14:16:26 +0000
+++ breezy/tests/test_selftest.py	2018-09-15 00:21:08 +0000
@@ -64,6 +64,9 @@
 from ..bzr import (
     groupcompress_repo,
     )
+from ..git import (
+    workingtree as git_workingtree,
+    )
 from ..sixish import (
     PY3,
     StringIO,
@@ -402,11 +405,12 @@
         formats = [workingtree_4.WorkingTreeFormat4(),
                    workingtree_3.WorkingTreeFormat3(),]
         scenarios = make_scenarios(server1, server2, formats)
-        self.assertEqual(8, len(scenarios))
+        self.assertEqual(9, len(scenarios))
         default_wt_format = workingtree.format_registry.get_default()
         wt4_format = workingtree_4.WorkingTreeFormat4()
         wt5_format = workingtree_4.WorkingTreeFormat5()
         wt6_format = workingtree_4.WorkingTreeFormat6()
+        git_wt_format = git_workingtree.GitWorkingTreeFormat()
         expected_scenarios = [
             ('WorkingTreeFormat4',
              {'bzrdir_format': formats[0]._matchingcontroldir,
@@ -438,6 +442,14 @@
               'transport_server': 'a',
               'workingtree_format': default_wt_format,
              }),
+             ('GitRevisionTree',
+              {'_workingtree_to_test_tree': revision_tree_from_workingtree,
+               'bzrdir_format': git_wt_format._matchingcontroldir,
+               'transport_readonly_server': 'b',
+               'transport_server': 'a',
+               'workingtree_format': git_wt_format,
+             }
+            ),
             ('DirStateRevisionTree,WT4',
              {'_workingtree_to_test_tree': _dirstate_tree_from_workingtree,
               'bzrdir_format': wt4_format._matchingcontroldir,