From 75773b8b9d265bc8ef113586be5c3e2d36250983 Mon Sep 17 00:00:00 2001 From: Peter Kjellerstedt Date: Sat, 8 Nov 2025 02:36:56 +0100 Subject: [PATCH] manifest, project: Store project groups as sets This helps a lot when including common manifests with groups and they use extend-project. Change-Id: Ic574e7d6696139d0eb90d9915e8c7048d5e89c07 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/525323 Reviewed-by: Gavin Mak Tested-by: Peter Kjellerstedt Reviewed-by: Mike Frysinger Commit-Queue: Peter Kjellerstedt --- docs/manifest-format.md | 8 ++-- man/repo-manifest.1 | 10 ++--- manifest_xml.py | 90 +++++++++++++++++++------------------- project.py | 6 +-- tests/test_manifest_xml.py | 8 ++-- 5 files changed, 62 insertions(+), 60 deletions(-) diff --git a/docs/manifest-format.md b/docs/manifest-format.md index 42e1ab18a..b45b91809 100644 --- a/docs/manifest-format.md +++ b/docs/manifest-format.md @@ -287,7 +287,7 @@ should be placed. If not supplied, `revision` is used. `path` may not be an absolute path or use "." or ".." path components. -Attribute `groups`: List of additional groups to which all projects +Attribute `groups`: Set of additional groups to which all projects in the included submanifest belong. This appends and recurses, meaning all projects in submanifests carry all parent submanifest groups. Same syntax as the corresponding element of `project`. @@ -355,7 +355,7 @@ When using `repo upload`, changes will be submitted for code review on this branch. If unspecified both here and in the default element, `revision` is used instead. -Attribute `groups`: List of groups to which this project belongs, +Attribute `groups`: Set of groups to which this project belongs, whitespace or comma separated. All projects belong to the group "all", and each project automatically belongs to a group of its name:`name` and path:`path`. E.g. for @@ -403,7 +403,7 @@ of the repo client where the Git working directory for this project should be placed. This is used to move a project in the checkout by overriding the existing `path` setting. -Attribute `groups`: List of additional groups to which this project +Attribute `groups`: Set of additional groups to which this project belongs. Same syntax as the corresponding element of `project`. Attribute `revision`: If specified, overrides the revision of the original @@ -572,7 +572,7 @@ the manifest repository's root. "name" may not be an absolute path or use "." or ".." path components. These restrictions are not enforced for [Local Manifests]. -Attribute `groups`: List of additional groups to which all projects +Attribute `groups`: Set of additional groups to which all projects in the included manifest belong. This appends and recurses, meaning all projects in included manifests carry all parent include groups. This also applies to all extend-project elements in the included manifests. diff --git a/man/repo-manifest.1 b/man/repo-manifest.1 index 1a97ff7dd..38c728e6e 100644 --- a/man/repo-manifest.1 +++ b/man/repo-manifest.1 @@ -365,7 +365,7 @@ supplied, `revision` is used. .PP `path` may not be an absolute path or use "." or ".." path components. .PP -Attribute `groups`: List of additional groups to which all projects in the +Attribute `groups`: Set of additional groups to which all projects in the included submanifest belong. This appends and recurses, meaning all projects in submanifests carry all parent submanifest groups. Same syntax as the corresponding element of `project`. @@ -427,7 +427,7 @@ Attribute `dest\-branch`: Name of a Git branch (e.g. `main`). When using `repo upload`, changes will be submitted for code review on this branch. If unspecified both here and in the default element, `revision` is used instead. .PP -Attribute `groups`: List of groups to which this project belongs, whitespace or +Attribute `groups`: Set of groups to which this project belongs, whitespace or comma separated. All projects belong to the group "all", and each project automatically belongs to a group of its name:`name` and path:`path`. E.g. for ``, that project definition is @@ -471,8 +471,8 @@ repo client where the Git working directory for this project should be placed. This is used to move a project in the checkout by overriding the existing `path` setting. .PP -Attribute `groups`: List of additional groups to which this project belongs. -Same syntax as the corresponding element of `project`. +Attribute `groups`: Set of additional groups to which this project belongs. Same +syntax as the corresponding element of `project`. .PP Attribute `revision`: If specified, overrides the revision of the original project. Same syntax as the corresponding element of `project`. @@ -634,7 +634,7 @@ repository's root. "name" may not be an absolute path or use "." or ".." path components. These restrictions are not enforced for [Local Manifests]. .PP -Attribute `groups`: List of additional groups to which all projects in the +Attribute `groups`: Set of additional groups to which all projects in the included manifest belong. This appends and recurses, meaning all projects in included manifests carry all parent include groups. This also applies to all extend\-project elements in the included manifests. Same syntax as the diff --git a/manifest_xml.py b/manifest_xml.py index 33a552827..2d476748c 100644 --- a/manifest_xml.py +++ b/manifest_xml.py @@ -255,7 +255,7 @@ class _XmlSubmanifest: project: a string, the name of the manifest project. revision: a string, the commitish. manifestName: a string, the submanifest file name. - groups: a list of strings, the groups to add to all projects in the + groups: a set of strings, the groups to add to all projects in the submanifest. default_groups: a list of strings, the default groups to sync. path: a string, the relative path for the submanifest checkout. @@ -281,7 +281,7 @@ class _XmlSubmanifest: self.project = project self.revision = revision self.manifestName = manifestName - self.groups = groups + self.groups = groups or set() self.default_groups = default_groups self.path = path self.parent = parent @@ -304,7 +304,7 @@ class _XmlSubmanifest: self.repo_client = RepoClient( parent.repodir, linkFile, - parent_groups=",".join(groups) or "", + parent_groups=groups, submanifest_path=os.path.join(parent.path_prefix, self.relpath), outer_client=outer_client, default_groups=default_groups, @@ -345,7 +345,7 @@ class _XmlSubmanifest: manifestName = self.manifestName or "default.xml" revision = self.revision or self.name path = self.path or revision.split("/")[-1] - groups = self.groups or [] + groups = self.groups return SubmanifestSpec( self.name, manifestUrl, manifestName, revision, path, groups @@ -359,9 +359,7 @@ class _XmlSubmanifest: def GetGroupsStr(self): """Returns the `groups` given for this submanifest.""" - if self.groups: - return ",".join(self.groups) - return "" + return ",".join(sorted(self.groups)) def GetDefaultGroupsStr(self): """Returns the `default-groups` given for this submanifest.""" @@ -381,7 +379,7 @@ class SubmanifestSpec: self.manifestName = manifestName self.revision = revision self.path = path - self.groups = groups or [] + self.groups = groups class XmlManifest: @@ -393,7 +391,7 @@ class XmlManifest: manifest_file, local_manifests=None, outer_client=None, - parent_groups="", + parent_groups=None, submanifest_path="", default_groups=None, ): @@ -409,7 +407,8 @@ class XmlManifest: manifests. This will usually be |repodir|/|LOCAL_MANIFESTS_DIR_NAME|. outer_client: RepoClient of the outer manifest. - parent_groups: a string, the groups to apply to this projects. + parent_groups: a set of strings, the groups to apply to this + manifest. submanifest_path: The submanifest root relative to the repo root. default_groups: a string, the default manifest groups to use. """ @@ -432,7 +431,7 @@ class XmlManifest: self.manifestFileOverrides = {} self.local_manifests = local_manifests self._load_local_manifests = True - self.parent_groups = parent_groups + self.parent_groups = parent_groups or set() self.default_groups = default_groups if submanifest_path and not outer_client: @@ -567,6 +566,14 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md """ return [x for x in re.split(r"[,\s]+", field) if x] + def _ParseSet(self, field): + """Parse fields that contain flattened sets. + + These are whitespace & comma separated. Empty elements will be + discarded. + """ + return set(self._ParseList(field)) + def ToXml( self, peg_rev=False, @@ -725,10 +732,9 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md le.setAttribute("dest", lf.dest) e.appendChild(le) - default_groups = ["all", "name:%s" % p.name, "path:%s" % p.relpath] - egroups = [g for g in p.groups if g not in default_groups] + egroups = p.groups - {"all", f"name:{p.name}", f"path:{p.relpath}"} if egroups: - e.setAttribute("groups", ",".join(egroups)) + e.setAttribute("groups", ",".join(sorted(egroups))) for a in p.annotations: if a.keep == "true": @@ -1171,12 +1177,12 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md b = b[len(R_HEADS) :] self.branch = b - parent_groups = self.parent_groups + parent_groups = self.parent_groups.copy() if self.path_prefix: - parent_groups = ( + parent_groups |= { f"{SUBMANIFEST_GROUP_PREFIX}:path:" - f"{self.path_prefix},{parent_groups}" - ) + f"{self.path_prefix}" + } # The manifestFile was specified by the user which is why we # allow include paths to point anywhere. @@ -1202,16 +1208,16 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md # Since local manifests are entirely managed by # the user, allow them to point anywhere the # user wants. - local_group = ( + local_group = { f"{LOCAL_MANIFEST_GROUP_PREFIX}:" f"{local_file[:-4]}" - ) + } nodes.append( self._ParseManifestXml( local, self.subdir, parent_groups=( - f"{local_group},{parent_groups}" + local_group | parent_groups ), restrict_includes=False, ) @@ -1262,7 +1268,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md self, path, include_root, - parent_groups="", + parent_groups=None, restrict_includes=True, parent_node=None, ): @@ -1271,11 +1277,11 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md Args: path: The XML file to read & parse. include_root: The path to interpret include "name"s relative to. - parent_groups: The groups to apply to this projects. + parent_groups: The set of groups to apply to this manifest. restrict_includes: Whether to constrain the "name" attribute of includes. - parent_node: The parent include node, to apply attribute to this - projects. + parent_node: The parent include node, to apply attributes to this + manifest. Returns: List of XML nodes. @@ -1307,12 +1313,10 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md raise ManifestInvalidPathError( f' invalid "name": {name}: {msg}' ) - include_groups = "" - if parent_groups: - include_groups = parent_groups + include_groups = (parent_groups or set()).copy() if node.hasAttribute("groups"): - include_groups = ( - node.getAttribute("groups") + "," + include_groups + include_groups |= self._ParseSet( + node.getAttribute("groups") ) fp = os.path.join(include_root, name) if not os.path.isfile(fp): @@ -1339,12 +1343,12 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md "project", "extend-project", ): - nodeGroups = parent_groups + nodeGroups = parent_groups.copy() if node.hasAttribute("groups"): - nodeGroups = ( - node.getAttribute("groups") + "," + nodeGroups + nodeGroups |= self._ParseSet( + node.getAttribute("groups") ) - node.setAttribute("groups", nodeGroups) + node.setAttribute("groups", ",".join(sorted(nodeGroups))) if ( parent_node and node.nodeName == "project" @@ -1461,7 +1465,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md dest_path = node.getAttribute("dest-path") groups = node.getAttribute("groups") if groups: - groups = self._ParseList(groups) + groups = self._ParseSet(groups or "") revision = node.getAttribute("revision") remote_name = node.getAttribute("remote") if not remote_name: @@ -1482,7 +1486,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md if path and p.relpath != path: continue if groups: - p.groups.extend(groups) + p.groups |= groups if revision: if base_revision: if p.revisionExpr != base_revision: @@ -1813,7 +1817,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md groups = "" if node.hasAttribute("groups"): groups = node.getAttribute("groups") - groups = self._ParseList(groups) + groups = self._ParseSet(groups) default_groups = self._ParseList(node.getAttribute("default-groups")) path = node.getAttribute("path") if path == "": @@ -1922,11 +1926,6 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md upstream = node.getAttribute("upstream") or self._default.upstreamExpr - groups = "" - if node.hasAttribute("groups"): - groups = node.getAttribute("groups") - groups = self._ParseList(groups) - if parent is None: ( relpath, @@ -1941,8 +1940,11 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md parent, name, path ) - default_groups = ["all", "name:%s" % name, "path:%s" % relpath] - groups.extend(set(default_groups).difference(groups)) + groups = "" + if node.hasAttribute("groups"): + groups = node.getAttribute("groups") + groups = self._ParseSet(groups) + groups |= {"all", f"name:{name}", f"path:{relpath}"} if self.IsMirror and node.hasAttribute("force-path"): if XmlBool(node, "force-path", False): diff --git a/project.py b/project.py index 416064426..b65cb1aeb 100644 --- a/project.py +++ b/project.py @@ -554,7 +554,7 @@ class Project: revisionExpr, revisionId, rebase=True, - groups=None, + groups=set(), sync_c=False, sync_s=False, sync_tags=True, @@ -839,9 +839,9 @@ class Project: """ default_groups = self.manifest.default_groups or ["default"] expanded_manifest_groups = manifest_groups or default_groups - expanded_project_groups = ["all"] + (self.groups or []) + expanded_project_groups = {"all"} | self.groups if "notdefault" not in expanded_project_groups: - expanded_project_groups += ["default"] + expanded_project_groups |= {"default"} matched = False for group in expanded_manifest_groups: diff --git a/tests/test_manifest_xml.py b/tests/test_manifest_xml.py index f59915158..960d0cd85 100644 --- a/tests/test_manifest_xml.py +++ b/tests/test_manifest_xml.py @@ -605,12 +605,12 @@ class ProjectElementTests(ManifestParseTestCase): manifest.projects[0].name: manifest.projects[0].groups, manifest.projects[1].name: manifest.projects[1].groups, } - self.assertCountEqual( - result["test-name"], ["name:test-name", "all", "path:test-path"] + self.assertEqual( + result["test-name"], {"name:test-name", "all", "path:test-path"} ) - self.assertCountEqual( + self.assertEqual( result["extras"], - ["g1", "g2", "g1", "name:extras", "all", "path:path"], + {"g1", "g2", "name:extras", "all", "path:path"}, ) groupstr = "default,platform-" + platform.system().lower() self.assertEqual(groupstr, manifest.GetGroupsStr())