android/git-repo
1
Fork 0
mirror of https://gerrit.googlesource.com/git-repo synced 2024-12-21 07:16:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

manifest: add optional base check on remove and extend

Browse Source 
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

<extend-project name="bla" revision="E" base-rev="B">

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 <sokcevic@google.com>
Tested-by: Fredrik de Groot <fredrik.de.groot@haleytek.com>
Commit-Queue: Josip Sokcevic <sokcevic@google.com>
This commit is contained in:
Fredrik de Groot 2024-09-09 15:54:57 +02:00 committed by LUCI
parent ae384f8623
commit 303bd963d5
3 changed files with 138 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
docs/manifest-format.md
View File

@ -107,11 +107,13 @@ following DTD:
<!ATTLIST extend-project remote CDATA #IMPLIED> <!ATTLIST extend-project remote CDATA #IMPLIED>
<!ATTLIST extend-project dest-branch CDATA #IMPLIED> <!ATTLIST extend-project dest-branch CDATA #IMPLIED>
<!ATTLIST extend-project upstream CDATA #IMPLIED> <!ATTLIST extend-project upstream CDATA #IMPLIED>
<!ATTLIST extend-project base-rev CDATA #IMPLIED>
<!ELEMENT remove-project EMPTY> <!ELEMENT remove-project EMPTY>
<!ATTLIST remove-project name CDATA #IMPLIED> <!ATTLIST remove-project name CDATA #IMPLIED>
<!ATTLIST remove-project path CDATA #IMPLIED> <!ATTLIST remove-project path CDATA #IMPLIED>
<!ATTLIST remove-project optional CDATA #IMPLIED> <!ATTLIST remove-project optional CDATA #IMPLIED>
<!ATTLIST remove-project base-rev CDATA #IMPLIED>
<!ELEMENT repo-hooks EMPTY> <!ELEMENT repo-hooks EMPTY>
<!ATTLIST repo-hooks in-project CDATA #REQUIRED> <!ATTLIST repo-hooks in-project CDATA #REQUIRED>
@ -433,6 +435,14 @@ project. Same syntax as the corresponding element of `project`.
Attribute `upstream`: If specified, overrides the upstream of the original Attribute `upstream`: If specified, overrides the upstream of the original
project. Same syntax as the corresponding element of `project`. 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 ### Element annotation
Zero or more annotation elements may be specified as children of a 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 Attribute `optional`: Set to true to ignore remove-project elements with no
matching `project` element. 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 ### Element repo-hooks
NB: See the [practical documentation](./repo-hooks.md) for using repo hooks. NB: See the [practical documentation](./repo-hooks.md) for using repo hooks.

35
manifest_xml.py
View File

@ -1445,6 +1445,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
repo_hooks_project = None repo_hooks_project = None
enabled_repo_hooks = None enabled_repo_hooks = None
failed_revision_changes = []
for node in itertools.chain(*node_list): for node in itertools.chain(*node_list):
if node.nodeName == "project": if node.nodeName == "project":
project = self._ParseProject(node) project = self._ParseProject(node)
@ -1471,6 +1472,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
remote = self._get_remote(node) remote = self._get_remote(node)
dest_branch = node.getAttribute("dest-branch") dest_branch = node.getAttribute("dest-branch")
upstream = node.getAttribute("upstream") upstream = node.getAttribute("upstream")
base_revision = node.getAttribute("base-rev")
named_projects = self._projects[name] named_projects = self._projects[name]
if dest_path and not path and len(named_projects) > 1: 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: if groups:
p.groups.extend(groups) p.groups.extend(groups)
if revision: 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) p.SetRevision(revision)
if remote_name: if remote_name:
@ -1558,6 +1567,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
if node.nodeName == "remove-project": if node.nodeName == "remove-project":
name = node.getAttribute("name") name = node.getAttribute("name")
path = node.getAttribute("path") path = node.getAttribute("path")
base_revision = node.getAttribute("base-rev")
# Name or path needed. # Name or path needed.
if not name and not path: 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 projname, projects in list(self._projects.items()):
for p in projects: for p in projects:
if name == projname and not path: 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] del self._paths[p.relpath]
if not removed_project: if not removed_project:
del self._projects[name] del self._projects[name]
@ -1578,6 +1595,17 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
elif path == p.relpath and ( elif path == p.relpath and (
name == projname or not name 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) self._projects[projname].remove(p)
del self._paths[p.relpath] del self._paths[p.relpath]
removed_project = p.name removed_project = p.name
@ -1597,6 +1625,13 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
"project: %s" % node.toxml() "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. # Store repo hooks project information.
if repo_hooks_project: if repo_hooks_project:
# Store a reference to the Project. # Store a reference to the Project.

85
tests/test_manifest_xml.py
View File

@ -1049,6 +1049,91 @@ class RemoveProjectElementTests(ManifestParseTestCase):
self.assertTrue(found_proj1_path1) self.assertTrue(found_proj1_path1)
self.assertTrue(found_proj2) self.assertTrue(found_proj2)
def test_base_revision_checks_on_patching(self):
manifest_fail_wrong_tag = self.getXmlManifest(
"""
<manifest>
<remote name="default-remote" fetch="http://localhost" />
<default remote="default-remote" revision="tag.002" />
<project name="project1" path="tests/path1" />
<extend-project name="project1" revision="new_hash" base-rev="tag.001" />
</manifest>
"""
)
with self.assertRaises(error.ManifestParseError):
manifest_fail_wrong_tag.ToXml()
manifest_fail_remove = self.getXmlManifest(
"""
<manifest>
<remote name="default-remote" fetch="http://localhost" />
<default remote="default-remote" revision="refs/heads/main" />
<project name="project1" path="tests/path1" revision="hash1" />
<remove-project name="project1" base-rev="wrong_hash" />
</manifest>
"""
)
with self.assertRaises(error.ManifestParseError):
manifest_fail_remove.ToXml()
manifest_fail_extend = self.getXmlManifest(
"""
<manifest>
<remote name="default-remote" fetch="http://localhost" />
<default remote="default-remote" revision="refs/heads/main" />
<project name="project1" path="tests/path1" revision="hash1" />
<extend-project name="project1" revision="new_hash" base-rev="wrong_hash" />
</manifest>
"""
)
with self.assertRaises(error.ManifestParseError):
manifest_fail_extend.ToXml()
manifest_fail_unknown = self.getXmlManifest(
"""
<manifest>
<remote name="default-remote" fetch="http://localhost" />
<default remote="default-remote" revision="refs/heads/main" />
<project name="project1" path="tests/path1" />
<extend-project name="project1" revision="new_hash" base-rev="any_hash" />
</manifest>
"""
)
with self.assertRaises(error.ManifestParseError):
manifest_fail_unknown.ToXml()
manifest_ok = self.getXmlManifest(
"""
<manifest>
<remote name="default-remote" fetch="http://localhost" />
<default remote="default-remote" revision="refs/heads/main" />
<project name="project1" path="tests/path1" revision="hash1" />
<project name="project2" path="tests/path2" revision="hash2" />
<project name="project3" path="tests/path3" revision="hash3" />
<project name="project4" path="tests/path4" revision="hash4" />
<remove-project name="project1" />
<remove-project name="project2" base-rev="hash2" />
<project name="project2" path="tests/path2" revision="new_hash2" />
<extend-project name="project3" base-rev="hash3" revision="new_hash3" />
<extend-project name="project3" base-rev="new_hash3" revision="newer_hash3" />
<remove-project path="tests/path4" base-rev="hash4" />
</manifest>
"""
)
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): class ExtendProjectElementTests(ManifestParseTestCase):
"""Tests for <extend-project>.""" """Tests for <extend-project>."""