From e0b16a22a01c54a00a9e4a0c53dddfce3b0d59d6 Mon Sep 17 00:00:00 2001 From: Xin Li Date: Sun, 26 Sep 2021 23:20:32 -0700 Subject: [PATCH] superproject: support a new revision attribute. Tested: $ ./run_tests Verified that a manifest that specified superproject revision would use the specified revision, and superproject will use the default revision. Note that this is a slight behavior change from earlier repo versions, which would always use the branch name of the manifest itself. However, the new behavior would be more consisitent with regular "project" element and would allow superproject be used even if it is not enabled for the particular manifest branch, so we have decided to make the change as it would provide more flexibility and better matches what other elements would do. Bug: [google internal] b/187868160 Change-Id: I35255ee347aff6e65179f7879d52931f168b477e Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/317643 Tested-by: Xin Li Reviewed-by: Raman Tenneti Reviewed-by: Mike Frysinger --- docs/manifest-format.md | 10 ++++- git_superproject.py | 14 +------ manifest_xml.py | 10 +++++ tests/test_git_superproject.py | 2 +- tests/test_manifest_xml.py | 69 ++++++++++++++++++++++++++++++++++ 5 files changed, 90 insertions(+), 15 deletions(-) diff --git a/docs/manifest-format.md b/docs/manifest-format.md index 854e5e1b..ed297ae7 100644 --- a/docs/manifest-format.md +++ b/docs/manifest-format.md @@ -103,8 +103,9 @@ following DTD: - - + + + @@ -432,6 +433,11 @@ same meaning as project's name attribute. See the Attribute `remote`: Name of a previously defined remote element. If not supplied the remote given by the default element is used. +Attribute `revision`: Name of the Git branch the manifest wants +to track for this superproject. If not supplied the revision given +by the remote element is used if applicable, else the default +element is used. + ### Element contactinfo *** diff --git a/git_superproject.py b/git_superproject.py index 935e1250..ce68690f 100644 --- a/git_superproject.py +++ b/git_superproject.py @@ -90,7 +90,7 @@ class Superproject(object): self._git_event_log = git_event_log self._quiet = quiet self._print_messages = print_messages - self._branch = self._GetBranch() + self._branch = manifest.branch self._repodir = os.path.abspath(repodir) self._superproject_dir = superproject_dir self._superproject_path = os.path.join(self._repodir, superproject_dir) @@ -100,6 +100,7 @@ class Superproject(object): if self._manifest.superproject: remote = self._manifest.superproject['remote'] git_name = hashlib.md5(remote.name.encode('utf8')).hexdigest() + '-' + self._branch = self._manifest.superproject['revision'] self._remote_url = remote.url else: self._remote_url = None @@ -116,17 +117,6 @@ class Superproject(object): """Returns the manifest path if the path exists or None.""" return self._manifest_path if os.path.exists(self._manifest_path) else None - def _GetBranch(self): - """Returns the branch name for getting the approved manifest.""" - p = self._manifest.manifestProject - b = p.GetBranch(p.CurrentBranch) - if not b: - return None - branch = b.merge - if branch and branch.startswith(R_HEADS): - branch = branch[len(R_HEADS):] - return branch - def _LogMessage(self, message): """Logs message to stderr and _git_event_log.""" if self._print_messages: diff --git a/manifest_xml.py b/manifest_xml.py index 7099d5fe..135c91fb 100644 --- a/manifest_xml.py +++ b/manifest_xml.py @@ -507,6 +507,9 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md if not d.remote or remote.orig_name != remoteName: remoteName = remote.orig_name e.setAttribute('remote', remoteName) + revision = remote.revision or d.revisionExpr + if not revision or revision != self._superproject['revision']: + e.setAttribute('revision', self._superproject['revision']) root.appendChild(e) if self._contactinfo.bugurl != Wrapper().BUG_URL: @@ -914,6 +917,13 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md raise ManifestParseError("no remote for superproject %s within %s" % (name, self.manifestFile)) self._superproject['remote'] = remote.ToRemoteSpec(name) + revision = node.getAttribute('revision') or remote.revision + if not revision: + revision = self._default.revisionExpr + if not revision: + raise ManifestParseError('no revision for superproject %s within %s' % + (name, self.manifestFile)) + self._superproject['revision'] = revision if node.nodeName == 'contactinfo': bugurl = self._reqatt(node, 'bugurl') # This element can be repeated, later entries will clobber earlier ones. diff --git a/tests/test_git_superproject.py b/tests/test_git_superproject.py index e9b824d6..a24fc7f0 100644 --- a/tests/test_git_superproject.py +++ b/tests/test_git_superproject.py @@ -157,7 +157,7 @@ class SuperprojectTestCase(unittest.TestCase): """) self._superproject = git_superproject.Superproject(manifest, self.repodir, self.git_event_log) - with mock.patch.object(self._superproject, '_GetBranch', return_value='junk'): + with mock.patch.object(self._superproject, '_branch', 'junk'): sync_result = self._superproject.Sync() self.assertFalse(sync_result.success) self.assertTrue(sync_result.fatal) diff --git a/tests/test_manifest_xml.py b/tests/test_manifest_xml.py index 20459d1d..ce422536 100644 --- a/tests/test_manifest_xml.py +++ b/tests/test_manifest_xml.py @@ -572,6 +572,7 @@ class SuperProjectElementTests(ManifestParseTestCase): self.assertEqual(manifest.superproject['name'], 'superproject') self.assertEqual(manifest.superproject['remote'].name, 'test-remote') self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject') + self.assertEqual(manifest.superproject['revision'], 'refs/heads/main') self.assertEqual( sort_attributes(manifest.ToXml().toxml()), '' @@ -580,6 +581,72 @@ class SuperProjectElementTests(ManifestParseTestCase): '' '') + def test_superproject_revision(self): + """Check superproject settings with a different revision attribute""" + self.maxDiff = None + manifest = self.getXmlManifest(""" + + + + + +""") + self.assertEqual(manifest.superproject['name'], 'superproject') + self.assertEqual(manifest.superproject['remote'].name, 'test-remote') + self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject') + self.assertEqual(manifest.superproject['revision'], 'refs/heads/stable') + self.assertEqual( + sort_attributes(manifest.ToXml().toxml()), + '' + '' + '' + '' + '') + + def test_superproject_revision_default_negative(self): + """Check superproject settings with a same revision attribute""" + self.maxDiff = None + manifest = self.getXmlManifest(""" + + + + + +""") + self.assertEqual(manifest.superproject['name'], 'superproject') + self.assertEqual(manifest.superproject['remote'].name, 'test-remote') + self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject') + self.assertEqual(manifest.superproject['revision'], 'refs/heads/stable') + self.assertEqual( + sort_attributes(manifest.ToXml().toxml()), + '' + '' + '' + '' + '') + + def test_superproject_revision_remote(self): + """Check superproject settings with a same revision attribute""" + self.maxDiff = None + manifest = self.getXmlManifest(""" + + + + + +""") + self.assertEqual(manifest.superproject['name'], 'superproject') + self.assertEqual(manifest.superproject['remote'].name, 'test-remote') + self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/superproject') + self.assertEqual(manifest.superproject['revision'], 'refs/heads/stable') + self.assertEqual( + sort_attributes(manifest.ToXml().toxml()), + '' + '' + '' + '' + '') + def test_remote(self): """Check superproject settings with a remote.""" manifest = self.getXmlManifest(""" @@ -593,6 +660,7 @@ class SuperProjectElementTests(ManifestParseTestCase): self.assertEqual(manifest.superproject['name'], 'platform/superproject') self.assertEqual(manifest.superproject['remote'].name, 'superproject-remote') self.assertEqual(manifest.superproject['remote'].url, 'http://localhost/platform/superproject') + self.assertEqual(manifest.superproject['revision'], 'refs/heads/main') self.assertEqual( sort_attributes(manifest.ToXml().toxml()), '' @@ -613,6 +681,7 @@ class SuperProjectElementTests(ManifestParseTestCase): """) self.assertEqual(manifest.superproject['name'], 'superproject') self.assertEqual(manifest.superproject['remote'].name, 'default-remote') + self.assertEqual(manifest.superproject['revision'], 'refs/heads/main') self.assertEqual( sort_attributes(manifest.ToXml().toxml()), ''