From ff6b1dae1e9f2e7405690c1aeedf7e0c7d768460 Mon Sep 17 00:00:00 2001 From: LaMont Jones Date: Wed, 1 Jun 2022 21:03:34 +0000 Subject: [PATCH] Only sync superproject if it will be used. If the user says `--no-use-superproject`, then do not bother syncing the superproject. Also add/update docstrings and comments throughout. Change-Id: I9cdad706130501bab9a22d3099a1dae605e9c194 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/338975 Tested-by: LaMont Jones Reviewed-by: Mike Frysinger --- command.py | 12 +++++++++++ git_superproject.py | 17 +++++++++++----- manifest_xml.py | 49 ++++++++++++++++++++++++++++++++++++--------- project.py | 11 ++++++---- subcmds/sync.py | 2 +- 5 files changed, 71 insertions(+), 20 deletions(-) diff --git a/command.py b/command.py index bd6d0817..7c68ebc5 100644 --- a/command.py +++ b/command.py @@ -277,6 +277,18 @@ class Command(object): def GetProjects(self, args, manifest=None, groups='', missing_ok=False, submodules_ok=False, all_manifests=False): """A list of projects that match the arguments. + + Args: + args: a list of (case-insensitive) strings, projects to search for. + manifest: an XmlManifest, the manifest to use, or None for default. + groups: a string, the manifest groups in use. + missing_ok: a boolean, whether to allow missing projects. + submodules_ok: a boolean, whether to allow submodules. + all_manifests: a boolean, if True then all manifests and submanifests are + used. If False, then only the local (sub)manifest is used. + + Returns: + A list of matching Project instances. """ if all_manifests: if not manifest: diff --git a/git_superproject.py b/git_superproject.py index 07bc2645..5d00bd72 100644 --- a/git_superproject.py +++ b/git_superproject.py @@ -18,7 +18,7 @@ For more information on superproject, check out: https://en.wikibooks.org/wiki/Git/Submodules_and_Superprojects Examples: - superproject = Superproject() + superproject = Superproject(manifest, name, remote, revision) UpdateProjectsResult = superproject.UpdateProjectsRevisionId(projects) """ @@ -99,8 +99,8 @@ class Superproject(object): self._work_git_name = git_name + _SUPERPROJECT_GIT_NAME self._work_git = os.path.join(self._superproject_path, self._work_git_name) - # The following are command arguemnts, rather then superproject attributes, - # and where included here originally. They should eventually become + # The following are command arguemnts, rather than superproject attributes, + # and were included here originally. They should eventually become # arguments that are passed down from the public methods, instead of being # treated as attributes. self._git_event_log = None @@ -329,7 +329,8 @@ class Superproject(object): """Update revisionId of every project in projects with the commit id. Args: - projects: List of projects whose revisionId needs to be updated. + projects: a list of projects whose revisionId needs to be updated. + git_event_log: an EventLog, for git tracing. Returns: UpdateProjectsResult @@ -431,9 +432,15 @@ def UseSuperproject(use_superproject, manifest): Args: use_superproject: option value from optparse. manifest: manifest to use. + + Returns: + Whether the superproject should be used. """ - if use_superproject is not None: + if not manifest.superproject: + # This (sub) manifest does not have a superproject definition. + return False + elif use_superproject is not None: return use_superproject else: client_value = manifest.manifestProject.use_superproject diff --git a/manifest_xml.py b/manifest_xml.py index db7a9286..32f6b687 100644 --- a/manifest_xml.py +++ b/manifest_xml.py @@ -348,7 +348,7 @@ class XmlManifest(object): be |repodir|/|MANIFEST_FILE_NAME|. local_manifests: Full path to the directory of local override manifests. This will usually be |repodir|/|LOCAL_MANIFESTS_DIR_NAME|. - outer_client: RepoClient of the outertree. + outer_client: RepoClient of the outer manifest. parent_groups: a string, the groups to apply to this projects. submanifest_path: The submanifest root relative to the repo root. default_groups: a string, the default manifest groups to use. @@ -776,18 +776,21 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md @property def is_submanifest(self): - """Whether this manifest is a submanifest""" + """Whether this manifest is a submanifest. + + This is safe to use as long as the outermost manifest XML has been parsed. + """ return self._outer_client and self._outer_client != self @property def outer_client(self): - """The instance of the outermost manifest client""" + """The instance of the outermost manifest client.""" self._Load() return self._outer_client @property def all_manifests(self): - """Generator yielding all (sub)manifests.""" + """Generator yielding all (sub)manifests, in depth-first order.""" self._Load() outer = self._outer_client yield outer @@ -796,7 +799,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md @property def all_children(self): - """Generator yielding all child submanifests.""" + """Generator yielding all (present) child submanifests.""" self._Load() for child in self._submanifests.values(): if child.repo_client: @@ -813,7 +816,14 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md @property def all_paths(self): - """All project paths for all (sub)manifests. See `paths`.""" + """All project paths for all (sub)manifests. + + See also `paths`. + + Returns: + A dictionary of {path: Project()}. `path` is relative to the outer + manifest. + """ ret = {} for tree in self.all_manifests: prefix = tree.path_prefix @@ -829,7 +839,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md def paths(self): """Return all paths for this manifest. - Return: + Returns: A dictionary of {path: Project()}. `path` is relative to this manifest. """ self._Load() @@ -843,11 +853,13 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md @property def remotes(self): + """Return a list of remotes for this manifest.""" self._Load() return self._remotes @property def default(self): + """Return default values for this manifest.""" self._Load() return self._default @@ -1090,8 +1102,8 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md if override: self.manifestFile = savedManifestFile - # Now that we have loaded this manifest, load any submanifest manifests - # as well. We need to do this after self._loaded is set to avoid looping. + # Now that we have loaded this manifest, load any submanifests as well. + # We need to do this after self._loaded is set to avoid looping. for name in self._submanifests: tree = self._submanifests[name] spec = tree.ToSubmanifestSpec() @@ -1659,6 +1671,10 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md name: a string, the name of the project. path: a string, the path of the project. remote: a string, the remote.name of the project. + + Returns: + A tuple of (relpath, worktree, gitdir, objdir, use_git_worktrees) for the + project with |name| and |path|. """ # The manifest entries might have trailing slashes. Normalize them to avoid # unexpected filesystem behavior since we do string concatenation below. @@ -1666,7 +1682,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md name = name.rstrip('/') remote = remote.rstrip('/') use_git_worktrees = False - use_remote_name = bool(self._outer_client._submanifests) + use_remote_name = self.is_multimanifest relpath = path if self.IsMirror: worktree = None @@ -1696,6 +1712,9 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md name: a string, the name of the project. all_manifests: a boolean, if True, then all manifests are searched. If False, then only this manifest is searched. + + Returns: + A list of Project instances with name |name|. """ if all_manifests: return list(itertools.chain.from_iterable( @@ -1956,6 +1975,16 @@ class RepoClient(XmlManifest): """Manages a repo client checkout.""" def __init__(self, repodir, manifest_file=None, submanifest_path='', **kwargs): + """Initialize. + + Args: + repodir: Path to the .repo/ dir for holding all internal checkout state. + It must be in the top directory of the repo client checkout. + manifest_file: Full path to the manifest file to parse. This will usually + be |repodir|/|MANIFEST_FILE_NAME|. + submanifest_path: The submanifest root relative to the repo root. + **kwargs: Additional keyword arguments, passed to XmlManifest. + """ self.isGitcClient = False submanifest_path = submanifest_path or '' if submanifest_path: diff --git a/project.py b/project.py index 8668bae9..8fed8f5e 100644 --- a/project.py +++ b/project.py @@ -33,6 +33,7 @@ import fetch from git_command import GitCommand, git_require from git_config import GitConfig, IsId, GetSchemeFromUrl, GetUrlCookieFile, \ ID_RE +import git_superproject from git_trace2_event_log import EventLog from error import GitError, UploadError, DownloadError from error import ManifestInvalidRevisionError, ManifestInvalidPathError @@ -2180,6 +2181,8 @@ class Project(object): if prune: cmd.append('--prune') + # Always pass something for --recurse-submodules, git with GIT_DIR behaves + # incorrectly when not given `--recurse-submodules=no`. (b/218891912) cmd.append(f'--recurse-submodules={"on-demand" if submodules else "no"}') spec = [] @@ -3486,8 +3489,8 @@ class ManifestProject(MetaProject): git_event_log: an EventLog, for git tracing. """ # TODO(lamontjones): when refactoring sync (and init?) consider how to - # better get the init options that we should use when syncing uncovers a new - # submanifest. + # better get the init options that we should use for new submanifests that + # are added when syncing an existing workspace. git_event_log = git_event_log or EventLog() spec = submanifest.ToSubmanifestSpec() # Use the init options from the existing manifestProject, or the parent if @@ -3874,8 +3877,8 @@ class ManifestProject(MetaProject): ) # Lastly, if the manifest has a then have the superproject - # sync it if it will be used. - if self.manifest.superproject: + # sync it (if it will be used). + if git_superproject.UseSuperproject(use_superproject, self.manifest): sync_result = self.manifest.superproject.Sync(git_event_log) if not sync_result.success: print('warning: git update of superproject for ' diff --git a/subcmds/sync.py b/subcmds/sync.py index 0abe23d6..fa61d551 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -316,7 +316,7 @@ later is required to fix a server side protocol bug. if not have_superproject: return - if opt.local_only: + if opt.local_only and manifest.superproject: manifest_path = manifest.superproject.manifest_path if manifest_path: self._ReloadManifest(manifest_path, manifest)