Merge lp:~jelmer/brz/git-drop-roundtrip into lp:brz

[Jelmer Vernooij]

Drop file id roundtripping support in Git.

Drop file id roundtripping support in Git.

File ids will soon be an implementation detail of the Bazaar format. Stashing
file ids somewhere in Git repositories is fraught with issues.

[Martin Packman (gz)]

I agree it makes sense to drop this.

review: Approve

=== modified file 'breezy/git/commit.py'
--- breezy/git/commit.py	2019-06-15 17:53:40 +0000
+++ breezy/git/commit.py	2019-06-29 16:01:34 +0000
@@ -145,8 +145,6 @@
                 yield change.path[1], (entry.text_sha1, st)
             if self._mapping.generate_file_id(encoded_new_path) != change.file_id:
                 self._override_fileids[encoded_new_path] = change.file_id
-            else:
-                self._override_fileids[encoded_new_path] = None
         if not seen_root and len(self.parents) == 0:
             raise RootMissing()
         if getattr(workingtree, "basis_tree", False):
@@ -163,32 +161,8 @@
                 continue
             self._blobs[entry.path] = (entry.mode, entry.sha)
         if not self._lossy:
-            try:
-                fileid_map = dict(basis_tree._fileid_map.file_ids)
-            except AttributeError:
-                fileid_map = {}
-            for path, file_id in viewitems(self._override_fileids):
-                if not isinstance(path, bytes):
-                    raise TypeError(path)
-                if file_id is None:
-                    if path in fileid_map:
-                        del fileid_map[path]
-                else:
-                    if not isinstance(file_id, bytes):
-                        raise TypeError(file_id)
-                    fileid_map[path] = file_id
-            if fileid_map:
-                fileid_blob = self._mapping.export_fileid_map(fileid_map)
-            else:
-                fileid_blob = None
-            if fileid_blob is not None:
-                if self._mapping.BZR_FILE_IDS_FILE is None:
-                    raise SettingCustomFileIdsUnsupported(fileid_map)
-                self.store.add_object(fileid_blob)
-                self._blobs[self._mapping.BZR_FILE_IDS_FILE] = (
-                    stat.S_IFREG | 0o644, fileid_blob.id)
-            else:
-                self._blobs[self._mapping.BZR_FILE_IDS_FILE] = None
+            if self._override_fileids:
+                raise SettingCustomFileIdsUnsupported(self._override_fileids)
         self.new_inventory = None
 
     def update_basis(self, tree):

=== modified file 'breezy/git/fetch.py'
--- breezy/git/fetch.py	2018-11-29 23:42:41 +0000
+++ breezy/git/fetch.py	2019-06-29 16:01:34 +0000
@@ -402,12 +402,11 @@
         base_tree = lookup_object(o.parents[0]).tree
         base_mode = stat.S_IFDIR
     store_updater = target_git_object_retriever._get_updater(rev)
-    tree_supplement = mapping.get_fileid_map(lookup_object, o.tree)
     inv_delta, unusual_modes = import_git_tree(
         repo.texts, mapping, b"", b"", (base_tree, o.tree), base_bzr_tree,
         None, rev.revision_id, parent_trees, lookup_object,
         (base_mode, stat.S_IFDIR), store_updater,
-        tree_supplement.lookup_file_id,
+        mapping.generate_file_id,
         allow_submodules=repo._format.supports_tree_reference)
     if unusual_modes != {}:
         for path, mode in unusual_modes.iteritems():

=== modified file 'breezy/git/mapping.py'
--- breezy/git/mapping.py	2019-06-02 05:34:26 +0000
+++ breezy/git/mapping.py	2019-06-29 16:01:34 +0000
@@ -57,8 +57,6 @@
     extract_bzr_metadata,
     inject_bzr_metadata,
     CommitSupplement,
-    deserialize_fileid_map,
-    serialize_fileid_map,
     )
 
 
@@ -131,12 +129,10 @@
     """Class that maps between Git and Bazaar semantics."""
     experimental = False
 
-    BZR_FILE_IDS_FILE = None
-
     BZR_DUMMY_FILE = None
 
     def is_special_file(self, filename):
-        return (filename in (self.BZR_FILE_IDS_FILE, self.BZR_DUMMY_FILE))
+        return (filename in (self.BZR_DUMMY_FILE, ))
 
     def __init__(self):
         super(BzrGitMapping, self).__init__(foreign_vcs_git)
@@ -257,17 +253,6 @@
     def _encode_commit_message(self, rev, message, encoding):
         return message.encode(encoding)
 
-    def export_fileid_map(self, fileid_map):
-        """Export a file id map to a fileid map.
-
-        :param fileid_map: File id map, mapping paths to file ids
-        :return: A Git blob object (or None if there are no entries)
-        """
-        from dulwich.objects import Blob
-        b = Blob()
-        b.set_raw_chunks(serialize_fileid_map(fileid_map))
-        return b
-
     def export_commit(self, rev, tree_sha, parent_lookup, lossy,
                       verifiers):
         """Turn a Bazaar revision in to a Git commit
@@ -370,14 +355,6 @@
                  for l in rev.properties[u'git-extra'].splitlines()])
         return commit
 
-    def import_fileid_map(self, blob):
-        """Convert a git file id map blob.
-
-        :param blob: Git blob object with fileid map
-        :return: Dictionary mapping paths to file ids
-        """
-        return deserialize_fileid_map(blob.data)
-
     def get_revision_id(self, commit):
         if commit.encoding:
             encoding = commit.encoding.decode('ascii')
@@ -480,22 +457,6 @@
             rev.properties[u'git-extra'] = b''.join(extra_lines)
         return rev, roundtrip_revid, verifiers
 
-    def get_fileid_map(self, lookup_object, tree_sha):
-        """Obtain a fileid map for a particular tree.
-
-        :param lookup_object: Function for looking up an object
-        :param tree_sha: SHA of the root tree
-        :return: GitFileIdMap instance
-        """
-        try:
-            file_id_map_sha = lookup_object(
-                tree_sha)[self.BZR_FILE_IDS_FILE][1]
-        except KeyError:
-            file_ids = {}
-        else:
-            file_ids = self.import_fileid_map(lookup_object(file_id_map_sha))
-        return GitFileIdMap(file_ids, self)
-
 
 class BzrGitMappingv1(BzrGitMapping):
     revid_prefix = b'git-v1'
@@ -508,9 +469,7 @@
 class BzrGitMappingExperimental(BzrGitMappingv1):
     revid_prefix = b'git-experimental'
     experimental = True
-    roundtripping = True
-
-    BZR_FILE_IDS_FILE = '.bzrfileids'
+    roundtripping = False
 
     BZR_DUMMY_FILE = '.bzrdummy'
 
@@ -674,49 +633,6 @@
     return (full_url, int(rev), uuid)
 
 
-class GitFileIdMap(object):
-
-    def __init__(self, file_ids, mapping):
-        self.file_ids = file_ids
-        self.paths = None
-        self.mapping = mapping
-
-    def set_file_id(self, path, file_id):
-        if type(path) is not str:
-            raise TypeError(path)
-        if not isinstance(file_id, bytes):
-            raise TypeError(file_id)
-        self.file_ids[path] = file_id
-
-    def lookup_file_id(self, path):
-        if not isinstance(path, text_type):
-            raise TypeError(path)
-        try:
-            file_id = self.file_ids[path]
-        except KeyError:
-            file_id = self.mapping.generate_file_id(path)
-        if not isinstance(file_id, bytes):
-            raise TypeError(file_id)
-        return file_id
-
-    def lookup_path(self, file_id):
-        if self.paths is None:
-            self.paths = {}
-            for k, v in viewitems(self.file_ids):
-                self.paths[v] = k
-        try:
-            path = self.paths[file_id]
-        except KeyError:
-            return self.mapping.parse_file_id(file_id)
-        else:
-            if not isinstance(path, text_type):
-                raise TypeError(path)
-            return path
-
-    def copy(self):
-        return self.__class__(dict(self.file_ids), self.mapping)
-
-
 def needs_roundtripping(repo, revid):
     try:
         mapping_registry.parse_revision_id(revid)

=== modified file 'breezy/git/memorytree.py'
--- breezy/git/memorytree.py	2019-03-04 01:31:43 +0000
+++ breezy/git/memorytree.py	2019-06-29 16:01:34 +0000
@@ -41,7 +41,6 @@
     )
 from breezy.transport.memory import MemoryTransport
 
-from .mapping import GitFileIdMap
 from .tree import MutableGitIndexTree
 
 
@@ -89,13 +88,9 @@
         self._file_transport = MemoryTransport()
         if self.branch.head is None:
             tree = Tree()
-            self._basis_fileid_map = GitFileIdMap({}, self.mapping)
         else:
             tree_id = self.store[self.branch.head].tree
-            self._basis_fileid_map = self.mapping.get_fileid_map(
-                self.store.__getitem__, tree_id)
             tree = self.store[tree_id]
-        self._fileid_map = self._basis_fileid_map.copy()
 
         trees = [("", tree)]
         while trees:

=== modified file 'breezy/git/object_store.py'
--- breezy/git/object_store.py	2019-06-16 16:12:56 +0000
+++ breezy/git/object_store.py	2019-06-29 16:01:34 +0000
@@ -492,15 +492,6 @@
         return self.mapping.export_commit(rev, tree_sha, parent_lookup,
                                           lossy, verifiers)
 
-    def _create_fileid_map_blob(self, tree):
-        # FIXME: This can probably be a lot more efficient,
-        # not all files necessarily have to be processed.
-        file_ids = {}
-        for (path, ie) in tree.iter_entries_by_dir():
-            if self.mapping.generate_file_id(path) != ie.file_id:
-                file_ids[path] = ie.file_id
-        return self.mapping.export_fileid_map(file_ids)
-
     def _revision_to_objects(self, rev, tree, lossy, add_cache_entry=None):
         """Convert a revision to a set of git objects.
 
@@ -530,12 +521,6 @@
                 base_sha1 = self._lookup_revision_sha1(rev.parent_ids[0])
                 root_tree = self[self[base_sha1].tree]
             root_key_data = (tree.get_root_id(), tree.get_revision_id())
-        if not lossy and self.mapping.BZR_FILE_IDS_FILE is not None:
-            b = self._create_fileid_map_blob(tree)
-            if b is not None:
-                root_tree[self.mapping.BZR_FILE_IDS_FILE] = (
-                    (stat.S_IFREG | 0o644), b.id)
-                yield self.mapping.BZR_FILE_IDS_FILE, b
         if add_cache_entry is not None:
             add_cache_entry(root_tree, root_key_data, "")
         yield "", root_tree
@@ -632,14 +617,6 @@
             bzr_tree.iter_child_entries(path),
             get_ie_sha1, unusual_modes, self.mapping.BZR_DUMMY_FILE,
             bzr_tree.get_root_id() == fileid)
-        if (bzr_tree.get_root_id() == fileid and
-                self.mapping.BZR_FILE_IDS_FILE is not None):
-            if tree is None:
-                tree = Tree()
-            b = self._create_fileid_map_blob(bzr_tree)
-            # If this is the root tree, add the file ids
-            tree[self.mapping.BZR_FILE_IDS_FILE] = (
-                (stat.S_IFREG | 0o644), b.id)
         if tree is not None:
             _check_expected_sha(expected_sha, tree)
         return tree

=== modified file 'breezy/git/roundtrip.py'
--- breezy/git/roundtrip.py	2018-11-11 14:23:06 +0000
+++ breezy/git/roundtrip.py	2019-06-29 16:01:34 +0000
@@ -147,30 +147,3 @@
     if not isinstance(rt_data, bytes):
         raise TypeError(rt_data)
     return message + b"\n--BZR--\n" + rt_data
-
-
-def serialize_fileid_map(file_ids):
-    """Serialize a fileid map.
-
-    :param file_ids: Path -> fileid map
-    :return: Serialized fileid map, as sequence of chunks
-    """
-    lines = []
-    for path in sorted(file_ids.keys()):
-        lines.append(b"%s\0%s\n" % (path.encode('utf-8'), file_ids[path]))
-    return lines
-
-
-def deserialize_fileid_map(filetext):
-    """Deserialize a file id map.
-
-    :param file: File
-    :return: Fileid map (path -> fileid)
-    """
-    ret = {}
-    f = BytesIO(filetext)
-    lines = f.readlines()
-    for l in lines:
-        (path, file_id) = l.rstrip(b"\n").split(b"\0")
-        ret[path.decode('utf-8')] = file_id
-    return ret

=== modified file 'breezy/git/tests/test_roundtrip.py'
--- breezy/git/tests/test_roundtrip.py	2018-11-11 04:08:32 +0000
+++ breezy/git/tests/test_roundtrip.py	2019-06-29 16:01:34 +0000
@@ -24,12 +24,10 @@
 
 from ..roundtrip import (
     CommitSupplement,
-    deserialize_fileid_map,
     extract_bzr_metadata,
     generate_roundtripping_metadata,
     inject_bzr_metadata,
     parse_roundtripping_metadata,
-    serialize_fileid_map,
     )
 
 
@@ -100,14 +98,3 @@
         metadata = CommitSupplement()
         msg = inject_bzr_metadata(b"Foo", metadata, "utf-8")
         self.assertEqual(b"Foo", msg)
-
-
-class FileIdRoundTripTests(TestCase):
-
-    def test_deserialize(self):
-        self.assertEqual({"bar/bla": b"fid"},
-                         deserialize_fileid_map(b"bar/bla\0fid\n"))
-
-    def test_serialize(self):
-        self.assertEqual([b"bar/bla\0fid\n"],
-                         serialize_fileid_map({"bar/bla": b"fid"}))

=== modified file 'breezy/git/tests/test_workingtree.py'
--- breezy/git/tests/test_workingtree.py	2019-06-02 05:13:10 +0000
+++ breezy/git/tests/test_workingtree.py	2019-06-29 16:01:34 +0000
@@ -37,7 +37,6 @@
 from ...delta import TreeDelta
 from ..mapping import (
     default_mapping,
-    GitFileIdMap,
     )
 from ..tree import (
     changes_between_git_tree_and_working_copy,
@@ -137,20 +136,16 @@
         changes = []
         self.assertEqual(
             delta,
-            tree_delta_from_git_changes(changes, default_mapping,
-                                        (GitFileIdMap({}, default_mapping),
-                                         GitFileIdMap({}, default_mapping))))
+            tree_delta_from_git_changes(changes, (default_mapping, default_mapping)))
 
     def test_missing(self):
         delta = TreeDelta()
-        delta.removed.append(('a', b'a-id', 'file'))
+        delta.removed.append(('a', b'git:a', 'file'))
         changes = [((b'a', b'a'), (stat.S_IFREG | 0o755, 0),
                     (b'a' * 40, b'a' * 40))]
         self.assertEqual(
             delta,
-            tree_delta_from_git_changes(changes, default_mapping,
-                                        (GitFileIdMap({u'a': b'a-id'}, default_mapping),
-                                         GitFileIdMap({u'a': b'a-id'}, default_mapping))))
+            tree_delta_from_git_changes(changes, (default_mapping, default_mapping)))
 
 
 class ChangesBetweenGitTreeAndWorkingCopyTests(TestCaseWithTransport):

=== modified file 'breezy/git/tree.py'
--- breezy/git/tree.py	2019-06-21 17:49:36 +0000
+++ breezy/git/tree.py	2019-06-29 16:01:34 +0000
@@ -67,7 +67,6 @@
 from .mapping import (
     mode_is_executable,
     mode_kind,
-    GitFileIdMap,
     default_mapping,
     )
 
@@ -261,17 +260,12 @@
         if revision_id == NULL_REVISION:
             self.tree = None
             self.mapping = default_mapping
-            self._fileid_map = GitFileIdMap(
-                {},
-                default_mapping)
         else:
             try:
                 commit = self.store[self.commit_id]
             except KeyError:
                 raise errors.NoSuchRevision(repository, revision_id)
             self.tree = commit.tree
-            self._fileid_map = self.mapping.get_fileid_map(
-                self.store.__getitem__, self.tree)
 
     def _get_nested_repository(self, path):
         nested_repo_transport = self._repository.user_transport.clone(path)
@@ -304,7 +298,7 @@
 
     def id2path(self, file_id):
         try:
-            path = self._fileid_map.lookup_path(file_id)
+            path = self.mapping.parse_file_id(file_id)
         except ValueError:
             raise errors.NoSuchId(self, file_id)
         if self.is_versioned(path):
@@ -319,7 +313,7 @@
             return None
         if not self.is_versioned(path):
             return None
-        return self._fileid_map.lookup_file_id(osutils.safe_unicode(path))
+        return self.mapping.generate_file_id(osutils.safe_unicode(path))
 
     def all_file_ids(self):
         raise errors.UnsupportedOperation(self.all_file_ids, self)
@@ -401,7 +395,7 @@
             root_ie = self._get_dir_ie(b"", None)
         else:
             parent_path = posixpath.dirname(from_dir)
-            parent_id = self._fileid_map.lookup_file_id(parent_path)
+            parent_id = self.mapping.generate_file_id(parent_path)
             if mode_kind(mode) == 'directory':
                 root_ie = self._get_dir_ie(from_dir.encode("utf-8"), parent_id)
             else:
@@ -441,7 +435,7 @@
         kind = mode_kind(mode)
         path = path.decode('utf-8')
         name = name.decode("utf-8")
-        file_id = self._fileid_map.lookup_file_id(path)
+        file_id = self.mapping.generate_file_id(path)
         ie = entry_factory[kind](file_id, name, parent_id)
         if kind == 'symlink':
             ie.symlink_target = store[hexsha].data.decode('utf-8')
@@ -457,7 +451,7 @@
 
     def _get_dir_ie(self, path, parent_id):
         path = path.decode('utf-8')
-        file_id = self._fileid_map.lookup_file_id(path)
+        file_id = self.mapping.generate_file_id(path)
         return GitTreeDirectory(file_id, posixpath.basename(path), parent_id)
 
     def iter_child_entries(self, path):
@@ -666,8 +660,8 @@
             yield (path_decoded, parent_id), children
 
 
-def tree_delta_from_git_changes(changes, mapping,
-                                fileid_maps, specific_files=None,
+def tree_delta_from_git_changes(changes, mappings,
+                                specific_files=None,
                                 require_versioned=False, include_root=False,
                                 target_extras=None):
     """Create a TreeDelta from two git trees.
@@ -675,7 +669,7 @@
     source and target are iterators over tuples with:
         (filename, sha, mode)
     """
-    (old_fileid_map, new_fileid_map) = fileid_maps
+    (old_mapping, new_mapping) = mappings
     if target_extras is None:
         target_extras = set()
     ret = delta.TreeDelta()
@@ -699,37 +693,37 @@
                     osutils.is_inside_or_parent_of_any(
                         specific_files, newpath_decoded))):
             continue
-        if mapping.is_special_file(oldpath):
+        if old_mapping.is_special_file(oldpath):
             oldpath = None
-        if mapping.is_special_file(newpath):
+        if new_mapping.is_special_file(newpath):
             newpath = None
         if oldpath is None and newpath is None:
             continue
         if oldpath is None:
             added.append((newpath, mode_kind(newmode)))
         elif newpath is None or newmode == 0:
-            file_id = old_fileid_map.lookup_file_id(oldpath_decoded)
+            file_id = old_mapping.generate_file_id(oldpath_decoded)
             ret.removed.append((oldpath_decoded, file_id, mode_kind(oldmode)))
         elif oldpath != newpath:
-            file_id = old_fileid_map.lookup_file_id(oldpath_decoded)
+            file_id = old_mapping.generate_file_id(oldpath_decoded)
             ret.renamed.append(
                 (oldpath_decoded, newpath.decode('utf-8'), file_id,
                  mode_kind(newmode), (oldsha != newsha),
                  (oldmode != newmode)))
         elif mode_kind(oldmode) != mode_kind(newmode):
-            file_id = new_fileid_map.lookup_file_id(newpath_decoded)
+            file_id = new_mapping.generate_file_id(newpath_decoded)
             ret.kind_changed.append(
                 (newpath_decoded, file_id, mode_kind(oldmode),
                  mode_kind(newmode)))
         elif oldsha != newsha or oldmode != newmode:
             if stat.S_ISDIR(oldmode) and stat.S_ISDIR(newmode):
                 continue
-            file_id = new_fileid_map.lookup_file_id(newpath_decoded)
+            file_id = new_mapping.generate_file_id(newpath_decoded)
             ret.modified.append(
                 (newpath_decoded, file_id, mode_kind(newmode),
                  (oldsha != newsha), (oldmode != newmode)))
         else:
-            file_id = new_fileid_map.lookup_file_id(newpath_decoded)
+            file_id = new_mapping.generate_file_id(newpath_decoded)
             ret.unchanged.append(
                 (newpath_decoded, file_id, mode_kind(newmode)))
 
@@ -746,7 +740,7 @@
         if path in target_extras:
             ret.unversioned.append((path_decoded, None, kind))
         else:
-            file_id = new_fileid_map.lookup_file_id(path_decoded)
+            file_id = new_mapping.generate_file_id(path_decoded)
             ret.added.append((path_decoded, file_id, kind))
 
     return ret
@@ -857,11 +851,8 @@
                 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),
+                changes, (self.source.mapping, self.target.mapping),
                 specific_files=specific_files,
                 include_root=include_root, target_extras=target_extras)
 
@@ -969,7 +960,7 @@
         with self.lock_read():
             path = path.rstrip('/')
             if self.is_versioned(path.rstrip('/')):
-                return self._fileid_map.lookup_file_id(
+                return self.mapping.generate_file_id(
                     osutils.safe_unicode(path))
             return None
 
@@ -988,7 +979,7 @@
             raise TypeError(file_id)
         with self.lock_read():
             try:
-                path = self._fileid_map.lookup_path(file_id)
+                path = self.mapping.parse_file_id(file_id)
             except ValueError:
                 raise errors.NoSuchId(self, file_id)
             if self.is_versioned(path):

=== modified file 'breezy/git/workingtree.py'
--- breezy/git/workingtree.py	2019-06-21 17:49:36 +0000
+++ breezy/git/workingtree.py	2019-06-29 16:01:34 +0000
@@ -79,7 +79,6 @@
     MutableGitIndexTree,
     )
 from .mapping import (
-    GitFileIdMap,
     mode_kind,
     )
 
@@ -603,7 +602,10 @@
         return False
 
     def had_id(self, file_id):
-        path = self._basis_fileid_map.lookup_path(file_id)
+        try:
+            path = self.mapping.parse_file_id(file_id)
+        except ValueError:
+            return False
         try:
             head = self.repository._git.head()
         except KeyError:
@@ -682,14 +684,7 @@
             raise errors.GhostRevisionUnusableHere(revid)
 
     def _reset_data(self):
-        try:
-            head = self.repository._git.head()
-        except KeyError:
-            self._basis_fileid_map = GitFileIdMap({}, self.mapping)
-        else:
-            self._basis_fileid_map = self.mapping.get_fileid_map(
-                self.store.__getitem__, self.store[head].tree)
-        self._fileid_map = self._basis_fileid_map.copy()
+        pass
 
     def get_file_verifier(self, path, stat_value=None):
         with self.lock_read():