From c34b91c9d80d6d6510c500cb8a8a9ee448963f09 Mon Sep 17 00:00:00 2001 From: Michael Kelly Date: Fri, 2 Jul 2021 09:25:48 -0700 Subject: [PATCH 1/3] manifest: Support ignoring local manifests with 'repo manifest' Currently users need to look up the baseline manifest by loading the specific manifest file. This exposes them to the internals of how the manifest is stored which may potentially be fragile (eg: It was switched from a symlink pointing at the file in the report to an actual file with an 'include' tag). Instead of doing this, we can provide an option to the 'repo manifest' command which will emit the baseline manifest and decouple users from the internal manifest details. Change-Id: I12ee9160feaa591484ae71f404bc529be500ae4e Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/311202 Tested-by: Michael Kelly Reviewed-by: Mike Frysinger --- manifest_xml.py | 7 +++++++ subcmds/manifest.py | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/manifest_xml.py b/manifest_xml.py index be74bf49..22758cfd 100644 --- a/manifest_xml.py +++ b/manifest_xml.py @@ -625,6 +625,13 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md 'repo.partialcloneexclude') or '' return set(x.strip() for x in exclude.split(',')) + @property + def UseLocalManifests(self): + return self._load_local_manifests + + def SetUseLocalManifests(self, value): + self._load_local_manifests = value + @property def HasLocalManifests(self): return self._load_local_manifests and self.local_manifests diff --git a/subcmds/manifest.py b/subcmds/manifest.py index 00587d8d..0fbdeac0 100644 --- a/subcmds/manifest.py +++ b/subcmds/manifest.py @@ -70,6 +70,8 @@ to indicate the remote ref to push changes to via 'repo upload'. help='output manifest in JSON format (experimental)') p.add_option('--pretty', default=False, action='store_true', help='format output for humans to read') + p.add_option('--no-local-manifests', default=False, action='store_true', + dest='ignore_local_manifests', help='ignore local manifests') p.add_option('-o', '--output-file', dest='output_file', default='-', @@ -85,6 +87,9 @@ to indicate the remote ref to push changes to via 'repo upload'. fd = sys.stdout else: fd = open(opt.output_file, 'w') + + self.manifest.SetUseLocalManifests(not opt.ignore_local_manifests) + if opt.json: print('warning: --json is experimental!', file=sys.stderr) doc = self.manifest.ToDict(peg_rev=opt.peg_rev, From 8e983bbc0f5f48aa38d0e1c5a37766ce121d28eb Mon Sep 17 00:00:00 2001 From: Xin Li Date: Tue, 20 Jul 2021 20:15:30 +0000 Subject: [PATCH 2/3] Force a fetch when superproject has a newer SHA1 for remote branch. For older git-repo versions, we might have only fetched the SHA1 revision that was provided by the project, but have remote branch left intact as long as they exist. When the remote branch become stale, some repo operations like rebase would fail, and repo sync would not correct this situation. Fix this by tightening the requirement to also require the superproject provided SHA1 be an ancestor or equal to the tip-of-tree of the remote branch. Bug: [google internal] b/193798453 Change-Id: Ie34c5d860dabb1cbd9f822da929088ec69c79cf6 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/312642 Tested-by: Xin Li Reviewed-by: Raman Tenneti --- project.py | 1 + 1 file changed, 1 insertion(+) diff --git a/project.py b/project.py index e777dbd2..a55e7600 100644 --- a/project.py +++ b/project.py @@ -1971,6 +1971,7 @@ class Project(object): rev = self.GetRemote(self.remote.name).ToLocal(self.upstream) self.bare_git.rev_list('-1', '--missing=allow-any', '%s^0' % rev, '--') + self.bare_git.merge_base('--is-ancestor', self.revisionExpr, rev) return True except GitError: # There is no such persistent revision. We have to fetch it. From 6ea0caea86f4c6b1f934b682a3aa7722e98a46f9 Mon Sep 17 00:00:00 2001 From: Jack Neus Date: Tue, 20 Jul 2021 20:52:33 +0000 Subject: [PATCH 3/3] repo: properly handle remote annotations in manifest_xml BUG=b:192664812 TEST=tests/ Change-Id: I1aa50260f4a00d3cebbd531141e1626825e70127 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/312643 Tested-by: Jack Neus Reviewed-by: Mike Frysinger --- docs/manifest-format.md | 14 +++++++------- manifest_xml.py | 35 +++++++++++++++++++++++++++-------- project.py | 20 ++++++++++++++++++-- tests/test_manifest_xml.py | 29 ++++++++++++++++++++++++++++- 4 files changed, 80 insertions(+), 18 deletions(-) diff --git a/docs/manifest-format.md b/docs/manifest-format.md index c3bfcff0..854e5e1b 100644 --- a/docs/manifest-format.md +++ b/docs/manifest-format.md @@ -36,7 +36,7 @@ following DTD: - + @@ -348,12 +348,12 @@ project. Same syntax as the corresponding element of `project`. ### Element annotation Zero or more annotation elements may be specified as children of a -project element. Each element describes a name-value pair that will be -exported into each project's environment during a 'forall' command, -prefixed with REPO__. In addition, there is an optional attribute -"keep" which accepts the case insensitive values "true" (default) or -"false". This attribute determines whether or not the annotation will -be kept when exported with the manifest subcommand. +project or remote element. Each element describes a name-value pair. +For projects, this name-value pair will be exported into each project's +environment during a 'forall' command, prefixed with `REPO__`. In addition, +there is an optional attribute "keep" which accepts the case insensitive values +"true" (default) or "false". This attribute determines whether or not the +annotation will be kept when exported with the manifest subcommand. ### Element copyfile diff --git a/manifest_xml.py b/manifest_xml.py index 22758cfd..55ad6c08 100644 --- a/manifest_xml.py +++ b/manifest_xml.py @@ -25,7 +25,7 @@ import gitc_utils from git_config import GitConfig, IsId from git_refs import R_HEADS, HEAD import platform_utils -from project import RemoteSpec, Project, MetaProject +from project import Annotation, RemoteSpec, Project, MetaProject from error import (ManifestParseError, ManifestInvalidPathError, ManifestInvalidRevisionError) from wrapper import Wrapper @@ -149,16 +149,18 @@ class _XmlRemote(object): self.reviewUrl = review self.revision = revision self.resolvedFetchUrl = self._resolveFetchUrl() + self.annotations = [] def __eq__(self, other): if not isinstance(other, _XmlRemote): return False - return self.__dict__ == other.__dict__ + return (sorted(self.annotations) == sorted(other.annotations) and + self.name == other.name and self.fetchUrl == other.fetchUrl and + self.pushUrl == other.pushUrl and self.remoteAlias == other.remoteAlias + and self.reviewUrl == other.reviewUrl and self.revision == other.revision) def __ne__(self, other): - if not isinstance(other, _XmlRemote): - return True - return self.__dict__ != other.__dict__ + return not self.__eq__(other) def _resolveFetchUrl(self): if self.fetchUrl is None: @@ -191,6 +193,9 @@ class _XmlRemote(object): orig_name=self.name, fetchUrl=self.fetchUrl) + def AddAnnotation(self, name, value, keep): + self.annotations.append(Annotation(name, value, keep)) + class XmlManifest(object): """manages the repo configuration file""" @@ -300,6 +305,13 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md if r.revision is not None: e.setAttribute('revision', r.revision) + for a in r.annotations: + if a.keep == 'true': + ae = doc.createElement('annotation') + ae.setAttribute('name', a.name) + ae.setAttribute('value', a.value) + e.appendChild(ae) + def _ParseList(self, field): """Parse fields that contain flattened lists. @@ -995,7 +1007,14 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md if revision == '': revision = None manifestUrl = self.manifestProject.config.GetString('remote.origin.url') - return _XmlRemote(name, alias, fetch, pushUrl, manifestUrl, review, revision) + + remote = _XmlRemote(name, alias, fetch, pushUrl, manifestUrl, review, revision) + + for n in node.childNodes: + if n.nodeName == 'annotation': + self._ParseAnnotation(remote, n) + + return remote def _ParseDefault(self, node): """ @@ -1362,7 +1381,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md self._ValidateFilePaths('linkfile', src, dest) project.AddLinkFile(src, dest, self.topdir) - def _ParseAnnotation(self, project, node): + def _ParseAnnotation(self, element, node): name = self._reqatt(node, 'name') value = self._reqatt(node, 'value') try: @@ -1372,7 +1391,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md if keep != "true" and keep != "false": raise ManifestParseError('optional "keep" attribute must be ' '"true" or "false"') - project.AddAnnotation(name, value, keep) + element.AddAnnotation(name, value, keep) def _get_remote(self, node): name = node.getAttribute('remote') diff --git a/project.py b/project.py index a55e7600..fe88a505 100644 --- a/project.py +++ b/project.py @@ -251,13 +251,29 @@ class DiffColoring(Coloring): self.fail = self.printer('fail', fg='red') -class _Annotation(object): +class Annotation(object): def __init__(self, name, value, keep): self.name = name self.value = value self.keep = keep + def __eq__(self, other): + if not isinstance(other, Annotation): + return False + return self.__dict__ == other.__dict__ + + def __lt__(self, other): + # This exists just so that lists of Annotation objects can be sorted, for + # use in comparisons. + if not isinstance(other, Annotation): + raise ValueError('comparison is not between two Annotation objects') + if self.name == other.name: + if self.value == other.value: + return self.keep < other.keep + return self.value < other.value + return self.name < other.name + def _SafeExpandPath(base, subpath, skipfinal=False): """Make sure |subpath| is completely safe under |base|. @@ -1448,7 +1464,7 @@ class Project(object): self.linkfiles.append(_LinkFile(self.worktree, src, topdir, dest)) def AddAnnotation(self, name, value, keep): - self.annotations.append(_Annotation(name, value, keep)) + self.annotations.append(Annotation(name, value, keep)) def DownloadPatchSet(self, change_id, patch_id): """Download a single patch set of a single change to FETCH_HEAD. diff --git a/tests/test_manifest_xml.py b/tests/test_manifest_xml.py index 96ee4c4a..59f2a779 100644 --- a/tests/test_manifest_xml.py +++ b/tests/test_manifest_xml.py @@ -286,6 +286,25 @@ class XmlManifestTests(ManifestParseTestCase): '' '') + def test_remote_annotations(self): + """Check remote settings.""" + manifest = self.getXmlManifest(""" + + + + + +""") + self.assertEqual(manifest.remotes['test-remote'].annotations[0].name, 'foo') + self.assertEqual(manifest.remotes['test-remote'].annotations[0].value, 'bar') + self.assertEqual( + sort_attributes(manifest.ToXml().toxml()), + '' + '' + '' + '' + '') + class IncludeElementTests(ManifestParseTestCase): """Tests for .""" @@ -632,9 +651,17 @@ class RemoteElementTests(ManifestParseTestCase): def test_remote(self): """Check remote settings.""" a = manifest_xml._XmlRemote(name='foo') - b = manifest_xml._XmlRemote(name='bar') + a.AddAnnotation('key1', 'value1', 'true') + b = manifest_xml._XmlRemote(name='foo') + b.AddAnnotation('key2', 'value1', 'true') + c = manifest_xml._XmlRemote(name='foo') + c.AddAnnotation('key1', 'value2', 'true') + d = manifest_xml._XmlRemote(name='foo') + d.AddAnnotation('key1', 'value1', 'false') self.assertEqual(a, a) self.assertNotEqual(a, b) + self.assertNotEqual(a, c) + self.assertNotEqual(a, d) self.assertNotEqual(a, manifest_xml._Default()) self.assertNotEqual(a, 123) self.assertNotEqual(a, None)