Merge lp:~jelmer/brz/extra-fields into lp:brz

[Jelmer Vernooij]

Allow unknown extras in git commits when just inspecting revisions, rather than importing.

Allow unknown extras in git commits when just inspecting revisions, rather than importing.

[Jelmer Vernooij (jelmer)]

review: Approve

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

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

=== modified file 'breezy/git/errors.py'
--- breezy/git/errors.py	2019-01-13 14:16:35 +0000
+++ breezy/git/errors.py	2019-11-03 00:18:07 +0000
@@ -61,21 +61,3 @@
 
 class GitSmartRemoteNotSupported(brz_errors.UnsupportedOperation):
     _fmt = "This operation is not supported by the Git smart server protocol."
-
-
-class UnknownCommitExtra(brz_errors.BzrError):
-    _fmt = "Unknown extra fields in %(object)r: %(fields)r."
-
-    def __init__(self, object, fields):
-        brz_errors.BzrError.__init__(self)
-        self.object = object
-        self.fields = ",".join(fields)
-
-
-class UnknownMercurialCommitExtra(brz_errors.BzrError):
-    _fmt = "Unknown mercurial extra fields in %(object)r: %(fields)r."
-
-    def __init__(self, object, fields):
-        brz_errors.BzrError.__init__(self)
-        self.object = object
-        self.fields = b",".join(fields)

=== modified file 'breezy/git/fetch.py'
--- breezy/git/fetch.py	2019-06-29 15:50:22 +0000
+++ breezy/git/fetch.py	2019-11-03 00:18:07 +0000
@@ -378,13 +378,13 @@
 
 
 def import_git_commit(repo, mapping, head, lookup_object,
-                      target_git_object_retriever, trees_cache):
+                      target_git_object_retriever, trees_cache, strict):
     o = lookup_object(head)
     # Note that this uses mapping.revision_id_foreign_to_bzr. If the parents
     # were bzr roundtripped revisions they would be specified in the
     # roundtrip data.
     rev, roundtrip_revid, verifiers = mapping.import_commit(
-        o, mapping.revision_id_foreign_to_bzr)
+        o, mapping.revision_id_foreign_to_bzr, strict)
     if roundtrip_revid is not None:
         original_revid = rev.revision_id
         rev.revision_id = roundtrip_revid
@@ -482,7 +482,7 @@
             continue
         if isinstance(o, Commit):
             rev, roundtrip_revid, verifiers = mapping.import_commit(
-                o, mapping.revision_id_foreign_to_bzr)
+                o, mapping.revision_id_foreign_to_bzr, strict=True)
             if (repo.has_revision(rev.revision_id)
                     or (roundtrip_revid and
                         repo.has_revision(roundtrip_revid))):
@@ -515,7 +515,8 @@
                         pb.update("fetching revisions", offset + i,
                                   len(revision_ids))
                     import_git_commit(repo, mapping, head, lookup_object,
-                                      target_git_object_retriever, trees_cache)
+                                      target_git_object_retriever, trees_cache,
+                                      strict=True)
                     last_imported = head
             except BaseException:
                 repo.abort_write_group()

=== modified file 'breezy/git/mapping.py'
--- breezy/git/mapping.py	2019-06-29 15:50:22 +0000
+++ breezy/git/mapping.py	2019-11-03 00:18:07 +0000
@@ -46,8 +46,6 @@
     )
 from .errors import (
     NoPushSupport,
-    UnknownCommitExtra,
-    UnknownMercurialCommitExtra,
     )
 from .hg import (
     format_hg_metadata,
@@ -73,6 +71,24 @@
 ROOT_ID = b"TREE_ROOT"
 
 
+class UnknownCommitExtra(errors.BzrError):
+    _fmt = "Unknown extra fields in %(object)r: %(fields)r."
+
+    def __init__(self, object, fields):
+        errors.BzrError.__init__(self)
+        self.object = object
+        self.fields = ",".join(fields)
+
+
+class UnknownMercurialCommitExtra(errors.BzrError):
+    _fmt = "Unknown mercurial extra fields in %(object)r: %(fields)r."
+
+    def __init__(self, object, fields):
+        errors.BzrError.__init__(self)
+        self.object = object
+        self.fields = b",".join(fields)
+
+
 def escape_file_id(file_id):
     file_id = file_id.replace(b'_', b'__')
     file_id = file_id.replace(b' ', b'_s')
@@ -370,7 +386,7 @@
                 return metadata.revision_id
         return self.revision_id_foreign_to_bzr(commit.id)
 
-    def import_commit(self, commit, lookup_parent_revid):
+    def import_commit(self, commit, lookup_parent_revid, strict=True):
         """Convert a git commit to a bzr revision.
 
         :return: a `breezy.revision.Revision` object, foreign revid and a
@@ -444,12 +460,12 @@
                 extra_lines.append(k + b' ' + v + b'\n')
             elif k == HG_EXTRA:
                 hgk, hgv = v.split(b':', 1)
-                if hgk not in (HG_EXTRA_AMEND_SOURCE, ):
+                if hgk not in (HG_EXTRA_AMEND_SOURCE, ) and strict:
                     raise UnknownMercurialCommitExtra(commit, [hgk])
                 extra_lines.append(k + b' ' + v + b'\n')
             else:
                 unknown_extra_fields.append(k)
-        if unknown_extra_fields:
+        if unknown_extra_fields and strict:
             raise UnknownCommitExtra(
                 commit,
                 [f.decode('ascii', 'replace') for f in unknown_extra_fields])
@@ -487,10 +503,10 @@
         ret += self._generate_git_svn_metadata(rev, encoding)
         return ret
 
-    def import_commit(self, commit, lookup_parent_revid):
+    def import_commit(self, commit, lookup_parent_revid, strict=True):
         rev, roundtrip_revid, verifiers = super(
             BzrGitMappingExperimental, self).import_commit(
-                commit, lookup_parent_revid)
+                commit, lookup_parent_revid, strict)
         rev.properties[u'converted_revision'] = "git %s\n" % commit.id
         return rev, roundtrip_revid, verifiers
 

=== modified file 'breezy/git/repository.py'
--- breezy/git/repository.py	2019-06-02 02:35:46 +0000
+++ breezy/git/repository.py	2019-11-03 00:18:07 +0000
@@ -539,7 +539,7 @@
         except KeyError:
             raise errors.NoSuchRevision(self, revision_id)
         revision, roundtrip_revid, verifiers = mapping.import_commit(
-            commit, self.lookup_foreign_revision_id)
+            commit, self.lookup_foreign_revision_id, strict=False)
         if revision is None:
             raise AssertionError
         # FIXME: check verifiers ?

=== modified file 'breezy/git/tests/test_mapping.py'
--- breezy/git/tests/test_mapping.py	2019-06-02 05:34:26 +0000
+++ breezy/git/tests/test_mapping.py	2019-11-03 00:18:07 +0000
@@ -34,15 +34,13 @@
     )
 
 from .. import tests
-from ..errors import (
-    UnknownCommitExtra,
-    UnknownMercurialCommitExtra,
-    )
 from ..mapping import (
     BzrGitMappingv1,
     escape_file_id,
     fix_person_identifier,
     unescape_file_id,
+    UnknownCommitExtra,
+    UnknownMercurialCommitExtra,
     )
 
 
@@ -111,8 +109,8 @@
         c.author_timezone = 60 * 3
         c.author = b"Author"
         mapping = BzrGitMappingv1()
-        rev, roundtrip_revid, verifiers = mapping.import_commit(c,
-                                                                mapping.revision_id_foreign_to_bzr)
+        rev, roundtrip_revid, verifiers = mapping.import_commit(
+            c, mapping.revision_id_foreign_to_bzr)
         self.assertEqual(None, roundtrip_revid)
         self.assertEqual({}, verifiers)
         self.assertEqual(u"Some message", rev.message)
@@ -136,8 +134,8 @@
         c.author = u"Authér".encode("iso8859-1")
         c.encoding = b"iso8859-1"
         mapping = BzrGitMappingv1()
-        rev, roundtrip_revid, verifiers = mapping.import_commit(c,
-                                                                mapping.revision_id_foreign_to_bzr)
+        rev, roundtrip_revid, verifiers = mapping.import_commit(
+            c, mapping.revision_id_foreign_to_bzr)
         self.assertEqual(None, roundtrip_revid)
         self.assertEqual({}, verifiers)
         self.assertEqual(u"Authér", rev.properties[u'author'])
@@ -155,8 +153,8 @@
         c.author_timezone = 60 * 3
         c.author = u"Authér".encode("latin1")
         mapping = BzrGitMappingv1()
-        rev, roundtrip_revid, verifiers = mapping.import_commit(c,
-                                                                mapping.revision_id_foreign_to_bzr)
+        rev, roundtrip_revid, verifiers = mapping.import_commit(
+            c, mapping.revision_id_foreign_to_bzr)
         self.assertEqual(None, roundtrip_revid)
         self.assertEqual({}, verifiers)
         self.assertEqual(u"Authér", rev.properties[u'author'])
@@ -174,8 +172,8 @@
         c.author_timezone = 60 * 3
         c.author = u"Authér".encode("utf-8")
         mapping = BzrGitMappingv1()
-        rev, roundtrip_revid, verifiers = mapping.import_commit(c,
-                                                                mapping.revision_id_foreign_to_bzr)
+        rev, roundtrip_revid, verifiers = mapping.import_commit(
+            c, mapping.revision_id_foreign_to_bzr)
         self.assertEqual(None, roundtrip_revid)
         self.assertEqual({}, verifiers)
         self.assertEqual(u"Authér", rev.properties[u'author'])
@@ -196,6 +194,7 @@
         mapping = BzrGitMappingv1()
         self.assertRaises(UnknownCommitExtra, mapping.import_commit, c,
                           mapping.revision_id_foreign_to_bzr)
+        mapping.import_commit(c, mapping.revision_id_foreign_to_bzr, strict=False)
 
     def test_mergetag(self):
         c = Commit()
@@ -235,6 +234,8 @@
         self.assertRaises(
             UnknownMercurialCommitExtra,
             mapping.import_commit, c, mapping.revision_id_foreign_to_bzr)
+        mapping.import_commit(
+            c, mapping.revision_id_foreign_to_bzr, strict=False)
         self.assertEqual(
             mapping.revision_id_foreign_to_bzr(c.id),
             mapping.get_revision_id(c))
@@ -267,7 +268,7 @@
         commit = self.mapping.export_commit(orig_rev, b"mysha",
                                             self._lookup_parent, True, b"testamentsha")
         rev, roundtrip_revid, verifiers = self.mapping.import_commit(
-            commit, self.mapping.revision_id_foreign_to_bzr)
+            commit, self.mapping.revision_id_foreign_to_bzr, strict=True)
         self.assertEqual(rev.revision_id,
                          self.mapping.revision_id_foreign_to_bzr(commit.id))
         if self.mapping.roundtripping:
@@ -338,7 +339,7 @@
 
     def assertRoundtripCommit(self, commit1):
         rev, roundtrip_revid, verifiers = self.mapping.import_commit(
-            commit1, self.mapping.revision_id_foreign_to_bzr)
+            commit1, self.mapping.revision_id_foreign_to_bzr, strict=True)
         commit2 = self.mapping.export_commit(rev, "12341212121212", None,
                                              True, None)
         self.assertEqual(commit1.committer, commit2.committer)