From 303bd963d57936873f62c7b61a885911afc46788 Mon Sep 17 00:00:00 2001 From: Fredrik de Groot Date: Mon, 9 Sep 2024 15:54:57 +0200 Subject: [PATCH] manifest: add optional base check on remove and extend This adds an optional, built-in checker for guarding against patches hanging on wrong base revisions, which is useful if a lower layer of the manifest changes after a patch was done. When adding a patch with a new revision using extend-project or remove-project/project: C---D---E patches in project bla / A---B project bla in manifest state 1 If project bla gets updated, in a new snap ID or by a supplier or similar, to a new state: C---D---E patches in project bla / A---B---F---G project bla in manifest state 2 Parsing will fail because revision of bla is now G, giving the choice to create a new patch branch from G and updating base-rev, or keeping previous branch for some reason and only updating base-rev. Intended for use in a layered manifest with hashed revisions. Named refs like branches and tags also work fine when comparing, but will be misleading if a branch is used as base-rev. Change-Id: Ic6211550a7d3cc9656057f6a2087c505b40cad2b Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/436777 Reviewed-by: Josip Sokcevic Tested-by: Fredrik de Groot Commit-Queue: Josip Sokcevic --- docs/manifest-format.md | 18 ++++++++ manifest_xml.py | 35 ++++++++++++++++ tests/test_manifest_xml.py | 85 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 138 insertions(+) diff --git a/docs/manifest-format.md b/docs/manifest-format.md index 36dae6de..cfb80164 100644 --- a/docs/manifest-format.md +++ b/docs/manifest-format.md @@ -107,11 +107,13 @@ following DTD: + + @@ -433,6 +435,14 @@ project. Same syntax as the corresponding element of `project`. Attribute `upstream`: If specified, overrides the upstream of the original project. Same syntax as the corresponding element of `project`. +Attribute `base-rev`: If specified, adds a check against the revision +to be extended. Manifest parse will fail and give a list of mismatch extends +if the revisions being extended have changed since base-rev was set. +Intended for use with layered manifests using hash revisions to prevent +patch branches hiding newer upstream revisions. Also compares named refs +like branches or tags but is misleading if branches are used as base-rev. +Same syntax as the corresponding element of `project`. + ### Element annotation Zero or more annotation elements may be specified as children of a @@ -496,6 +506,14 @@ name. Logic otherwise behaves like both are specified. Attribute `optional`: Set to true to ignore remove-project elements with no matching `project` element. +Attribute `base-rev`: If specified, adds a check against the revision +to be removed. Manifest parse will fail and give a list of mismatch removes +if the revisions being removed have changed since base-rev was set. +Intended for use with layered manifests using hash revisions to prevent +patch branches hiding newer upstream revisions. Also compares named refs +like branches or tags but is misleading if branches are used as base-rev. +Same syntax as the corresponding element of `project`. + ### Element repo-hooks NB: See the [practical documentation](./repo-hooks.md) for using repo hooks. diff --git a/manifest_xml.py b/manifest_xml.py index b26b0cac..cdab31e6 100644 --- a/manifest_xml.py +++ b/manifest_xml.py @@ -1445,6 +1445,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md repo_hooks_project = None enabled_repo_hooks = None + failed_revision_changes = [] for node in itertools.chain(*node_list): if node.nodeName == "project": project = self._ParseProject(node) @@ -1471,6 +1472,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md remote = self._get_remote(node) dest_branch = node.getAttribute("dest-branch") upstream = node.getAttribute("upstream") + base_revision = node.getAttribute("base-rev") named_projects = self._projects[name] if dest_path and not path and len(named_projects) > 1: @@ -1484,6 +1486,13 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md if groups: p.groups.extend(groups) if revision: + if base_revision: + if p.revisionExpr != base_revision: + failed_revision_changes.append( + "extend-project name %s mismatch base " + "%s vs revision %s" + % (name, base_revision, p.revisionExpr) + ) p.SetRevision(revision) if remote_name: @@ -1558,6 +1567,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md if node.nodeName == "remove-project": name = node.getAttribute("name") path = node.getAttribute("path") + base_revision = node.getAttribute("base-rev") # Name or path needed. if not name and not path: @@ -1571,6 +1581,13 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md for projname, projects in list(self._projects.items()): for p in projects: if name == projname and not path: + if base_revision: + if p.revisionExpr != base_revision: + failed_revision_changes.append( + "remove-project name %s mismatch base " + "%s vs revision %s" + % (name, base_revision, p.revisionExpr) + ) del self._paths[p.relpath] if not removed_project: del self._projects[name] @@ -1578,6 +1595,17 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md elif path == p.relpath and ( name == projname or not name ): + if base_revision: + if p.revisionExpr != base_revision: + failed_revision_changes.append( + "remove-project path %s mismatch base " + "%s vs revision %s" + % ( + p.relpath, + base_revision, + p.revisionExpr, + ) + ) self._projects[projname].remove(p) del self._paths[p.relpath] removed_project = p.name @@ -1597,6 +1625,13 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md "project: %s" % node.toxml() ) + if failed_revision_changes: + raise ManifestParseError( + "revision base check failed, rebase patches and update " + "base revs for: ", + failed_revision_changes, + ) + # Store repo hooks project information. if repo_hooks_project: # Store a reference to the Project. diff --git a/tests/test_manifest_xml.py b/tests/test_manifest_xml.py index 3f03272a..3d1fde96 100644 --- a/tests/test_manifest_xml.py +++ b/tests/test_manifest_xml.py @@ -1049,6 +1049,91 @@ class RemoveProjectElementTests(ManifestParseTestCase): self.assertTrue(found_proj1_path1) self.assertTrue(found_proj2) + def test_base_revision_checks_on_patching(self): + manifest_fail_wrong_tag = self.getXmlManifest( + """ + + + + + + +""" + ) + with self.assertRaises(error.ManifestParseError): + manifest_fail_wrong_tag.ToXml() + + manifest_fail_remove = self.getXmlManifest( + """ + + + + + + +""" + ) + with self.assertRaises(error.ManifestParseError): + manifest_fail_remove.ToXml() + + manifest_fail_extend = self.getXmlManifest( + """ + + + + + + +""" + ) + with self.assertRaises(error.ManifestParseError): + manifest_fail_extend.ToXml() + + manifest_fail_unknown = self.getXmlManifest( + """ + + + + + + +""" + ) + with self.assertRaises(error.ManifestParseError): + manifest_fail_unknown.ToXml() + + manifest_ok = self.getXmlManifest( + """ + + + + + + + + + + + + + + + +""" + ) + found_proj2 = False + found_proj3 = False + for proj in manifest_ok.projects: + if proj.name == "project2": + found_proj2 = True + if proj.name == "project3": + found_proj3 = True + self.assertNotEqual(proj.name, "project1") + self.assertNotEqual(proj.name, "project4") + self.assertTrue(found_proj2) + self.assertTrue(found_proj3) + self.assertTrue(len(manifest_ok.projects) == 2) + class ExtendProjectElementTests(ManifestParseTestCase): """Tests for ."""