From 6ea0caea86f4c6b1f934b682a3aa7722e98a46f9 Mon Sep 17 00:00:00 2001 From: Jack Neus Date: Tue, 20 Jul 2021 20:52:33 +0000 Subject: [PATCH] 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)