Fix Projects.shareable_dirs

If this tree is not using alternates for object sharing, then we need to
continue to call it a shared directory.

Bug: https://bugs.chromium.org/p/gerrit/issues/detail?id=15982
Test: manual
Change-Id: I1750f10b192504ac67f552222f8ddb9809d344fe
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/338974
Tested-by: LaMont Jones <lamontjones@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>

command.py

@@ -144,11 +144,10 @@ class Command(object):
           help=f'number of jobs to run in parallel (default: {default})')
 
     m = p.add_option_group('Multi-manifest options')
-    m.add_option('--outer-manifest', action='store_true',
+    m.add_option('--outer-manifest', action='store_true', default=None,
                  help='operate starting at the outermost manifest')
     m.add_option('--no-outer-manifest', dest='outer_manifest',
-                 action='store_false', default=None,
-                 help='do not operate on outer manifests')
+                 action='store_false', help='do not operate on outer manifests')
     m.add_option('--this-manifest-only', action='store_true', default=None,
                  help='only operate on this (sub)manifest')
     m.add_option('--no-this-manifest-only', '--all-manifests',
@@ -186,6 +185,10 @@ class Command(object):
     """Validate common options."""
     opt.quiet = opt.output_mode is False
     opt.verbose = opt.output_mode is True
+    if opt.outer_manifest is None:
+      # By default, treat multi-manifest instances as a single manifest from
+      # the user's perspective.
+      opt.outer_manifest = True
 
   def ValidateOptions(self, opt, args):
     """Validate the user options & arguments before executing.
@@ -274,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:
@@ -385,7 +400,7 @@ class Command(object):
       opt: The command options.
     """
     top = self.outer_manifest
-    if opt.outer_manifest is False or opt.this_manifest_only:
+    if not opt.outer_manifest or opt.this_manifest_only:
       top = self.manifest
     yield top
     if not opt.this_manifest_only:

docs/manifest-format.md

@@ -66,6 +66,7 @@ following DTD:
   <!ATTLIST submanifest revision       CDATA #IMPLIED>
   <!ATTLIST submanifest path           CDATA #IMPLIED>
   <!ATTLIST submanifest groups         CDATA #IMPLIED>
+  <!ATTLIST submanifest default-groups CDATA #IMPLIED>
 
   <!ELEMENT project (annotation*,
                      project*,
@@ -302,6 +303,9 @@ 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`.
 
+Attribute `default-groups`: The list of manifest groups to sync if no
+`--groups=` parameter was specified at init.  When that list is empty, use this
+list instead of "default" as the list of groups to sync.
 
 ### Element project
 

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

main.py

@@ -294,8 +294,7 @@ class _Repo(object):
       cmd.ValidateOptions(copts, cargs)
 
       this_manifest_only = copts.this_manifest_only
-      # If not specified, default to using the outer manifest.
-      outer_manifest = copts.outer_manifest is not False
+      outer_manifest = copts.outer_manifest
       if cmd.MULTI_MANIFEST_SUPPORT or this_manifest_only:
         result = cmd.Execute(copts, cargs)
       elif outer_manifest and repo_client.manifest.is_submanifest:

manifest_xml.py

@@ -214,6 +214,7 @@ class _XmlSubmanifest:
     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 submanifest.
+    default_groups: a list of strings, the default groups to sync.
     path: a string, the relative path for the submanifest checkout.
     parent: an XmlManifest, the parent manifest.
     annotations: (derived) a list of annotations.
@@ -226,6 +227,7 @@ class _XmlSubmanifest:
                revision=None,
                manifestName=None,
                groups=None,
+               default_groups=None,
                path=None,
                parent=None):
     self.name = name
@@ -234,6 +236,7 @@ class _XmlSubmanifest:
     self.revision = revision
     self.manifestName = manifestName
     self.groups = groups
+    self.default_groups = default_groups
     self.path = path
     self.parent = parent
     self.annotations = []
@@ -250,7 +253,8 @@ class _XmlSubmanifest:
         os.path.join(parent.path_prefix, self.relpath), MANIFEST_FILE_NAME)
     rc = self.repo_client = RepoClient(
         parent.repodir, linkFile, parent_groups=','.join(groups) or '',
-        submanifest_path=self.relpath, outer_client=outer_client)
+        submanifest_path=self.relpath, outer_client=outer_client,
+        default_groups=default_groups)
 
     self.present = os.path.exists(manifestFile)
 
@@ -264,6 +268,7 @@ class _XmlSubmanifest:
         self.revision == other.revision and
         self.manifestName == other.manifestName and
         self.groups == other.groups and
+        self.default_groups == other.default_groups and
         self.path == other.path and
         sorted(self.annotations) == sorted(other.annotations))
 
@@ -284,6 +289,7 @@ class _XmlSubmanifest:
     revision = self.revision or self.name
     path = self.path or revision.split('/')[-1]
     groups = self.groups or []
+    default_groups = self.default_groups or []
 
     return SubmanifestSpec(self.name, manifestUrl, manifestName, revision, path,
                            groups)
@@ -300,6 +306,10 @@ class _XmlSubmanifest:
       return ','.join(self.groups)
     return ''
 
+  def GetDefaultGroupsStr(self):
+    """Returns the `default-groups` given for this submanifest."""
+    return ','.join(self.default_groups or [])
+
   def AddAnnotation(self, name, value, keep):
     """Add annotations to the submanifest."""
     self.annotations.append(Annotation(name, value, keep))
@@ -327,7 +337,8 @@ class XmlManifest(object):
   """manages the repo configuration file"""
 
   def __init__(self, repodir, manifest_file, local_manifests=None,
-               outer_client=None, parent_groups='', submanifest_path=''):
+               outer_client=None, parent_groups='', submanifest_path='',
+               default_groups=None):
     """Initialize.
 
     Args:
@@ -337,9 +348,10 @@ 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.
     """
     # TODO(vapier): Move this out of this class.
     self.globalConfig = GitConfig.ForUser()
@@ -358,6 +370,7 @@ class XmlManifest(object):
     self.local_manifests = local_manifests
     self._load_local_manifests = True
     self.parent_groups = parent_groups
+    self.default_groups = default_groups
 
     if outer_client and self.isGitcClient:
       raise ManifestParseError('Multi-manifest is incompatible with `gitc-init`')
@@ -472,6 +485,8 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
       e.setAttribute('path', r.path)
     if r.groups:
       e.setAttribute('groups', r.GetGroupsStr())
+    if r.default_groups:
+      e.setAttribute('default-groups', r.GetDefaultGroupsStr())
 
     for a in r.annotations:
       if a.keep == 'true':
@@ -753,23 +768,29 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
 
   @property
   def is_multimanifest(self):
-    """Whether this is a multimanifest checkout"""
-    return bool(self.outer_client.submanifests)
+    """Whether this is a multimanifest checkout.
+
+    This is safe to use as long as the outermost manifest XML has been parsed.
+    """
+    return bool(self._outer_client._submanifests)
 
   @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
@@ -778,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:
@@ -795,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
@@ -811,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()
@@ -825,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
 
@@ -967,16 +997,21 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
                          worktree=os.path.join(subdir, 'manifests'))
     return mp
 
-  def GetDefaultGroupsStr(self):
-    """Returns the default group string for the platform."""
-    return 'default,platform-' + platform.system().lower()
+  def GetDefaultGroupsStr(self, with_platform=True):
+    """Returns the default group string to use.
+
+    Args:
+      with_platform: a boolean, whether to include the group for the
+                     underlying platform.
+    """
+    groups = ','.join(self.default_groups or ['default'])
+    if with_platform:
+      groups += f',platform-{platform.system().lower()}'
+    return groups
 
   def GetGroupsStr(self):
     """Returns the manifest group string that should be synced."""
-    groups = self.manifestProject.manifest_groups
-    if not groups:
-      groups = self.GetDefaultGroupsStr()
-    return groups
+    return self.manifestProject.manifest_groups or self.GetDefaultGroupsStr()
 
   def Unload(self):
     """Unload the manifest.
@@ -1067,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()
@@ -1491,6 +1526,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
     if node.hasAttribute('groups'):
       groups = node.getAttribute('groups')
     groups = self._ParseList(groups)
+    default_groups = self._ParseList(node.getAttribute('default-groups'))
     path = node.getAttribute('path')
     if path == '':
       path = None
@@ -1511,7 +1547,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
             '<submanifest> invalid "path": %s: %s' % (path, msg))
 
     submanifest = _XmlSubmanifest(name, remote, project, revision, manifestName,
-                                  groups, path, self)
+                                  groups, default_groups, path, self)
 
     for n in node.childNodes:
       if n.nodeName == 'annotation':
@@ -1635,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.
@@ -1642,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
@@ -1658,7 +1698,7 @@ https://gerrit.googlesource.com/git-repo/+/HEAD/docs/manifest-format.md
       # We allow people to mix git worktrees & non-git worktrees for now.
       # This allows for in situ migration of repo clients.
       if os.path.exists(gitdir) or not self.UseGitWorktrees:
-        objdir = os.path.join(self.subdir, 'project-objects', namepath)
+        objdir = os.path.join(self.repodir, 'project-objects', namepath)
       else:
         use_git_worktrees = True
         gitdir = os.path.join(self.repodir, 'worktrees', namepath)
@@ -1672,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(
@@ -1932,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:

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
@@ -49,6 +50,9 @@ MAXIMUM_RETRY_SLEEP_SEC = 3600.0
 # +-10% random jitter is added to each Fetches retry sleep duration.
 RETRY_JITTER_PERCENT = 0.1
 
+# Whether to use alternates.
+# TODO(vapier): Remove knob once behavior is verified.
+_ALTERNATES = os.environ.get('REPO_USE_ALTERNATES') == '1'
 
 def _lwrite(path, content):
   lock = '%s.lock' % path
@@ -460,7 +464,13 @@ class RemoteSpec(object):
 
 class Project(object):
   # These objects can be shared between several working trees.
-  shareable_dirs = ['hooks', 'objects', 'rr-cache']
+  @property
+  def shareable_dirs(self):
+    """Return the shareable directories"""
+    if self.UseAlternates:
+      return ['hooks', 'rr-cache']
+    else:
+      return ['hooks', 'objects', 'rr-cache']
 
   def __init__(self,
                manifest,
@@ -590,6 +600,14 @@ class Project(object):
     self.bare_ref = GitRefs(self.gitdir)
     self.bare_objdir = self._GitGetByExec(self, bare=True, gitdir=self.objdir)
 
+  @property
+  def UseAlternates(self):
+    """Whether git alternates are in use.
+
+    This will be removed once migration to alternates is complete.
+    """
+    return _ALTERNATES or self.manifest.is_multimanifest
+
   @property
   def Derived(self):
     return self.is_derived
@@ -715,7 +733,8 @@ class Project(object):
        The special manifest group "default" will match any project that
        does not have the special project group "notdefault"
     """
-    expanded_manifest_groups = manifest_groups or ['default']
+    default_groups = self.manifest.default_groups or ['default']
+    expanded_manifest_groups = manifest_groups or default_groups
     expanded_project_groups = ['all'] + (self.groups or [])
     if 'notdefault' not in expanded_project_groups:
       expanded_project_groups += ['default']
@@ -995,6 +1014,13 @@ class Project(object):
     if not branch.remote.review:
       raise GitError('remote %s has no review url' % branch.remote.name)
 
+    # Basic validity check on label syntax.
+    for label in labels:
+      if not re.match(r'^.+[+-][0-9]+$', label):
+        raise UploadError(
+            f'invalid label syntax "{label}": labels use forms like '
+            'CodeReview+1 or Verified-1')
+
     if dest_branch is None:
       dest_branch = self.dest_branch
     if dest_branch is None:
@@ -1030,6 +1056,7 @@ class Project(object):
     if auto_topic:
       opts += ['topic=' + branch.name]
     opts += ['t=%s' % p for p in hashtags]
+    # NB: No need to encode labels as they've been validated above.
     opts += ['l=%s' % p for p in labels]
 
     opts += ['r=%s' % p for p in people[0]]
@@ -1134,6 +1161,17 @@ class Project(object):
       self._UpdateHooks(quiet=quiet)
     self._InitRemote()
 
+    if self.UseAlternates:
+      # If gitdir/objects is a symlink, migrate it from the old layout.
+      gitdir_objects = os.path.join(self.gitdir, 'objects')
+      if platform_utils.islink(gitdir_objects):
+        platform_utils.remove(gitdir_objects, missing_ok=True)
+      gitdir_alt = os.path.join(self.gitdir, 'objects/info/alternates')
+      if not os.path.exists(gitdir_alt):
+        os.makedirs(os.path.dirname(gitdir_alt), exist_ok=True)
+        _lwrite(gitdir_alt, os.path.join(
+            os.path.relpath(self.objdir, gitdir_objects), 'objects') + '\n')
+
     if is_new:
       alt = os.path.join(self.objdir, 'objects/info/alternates')
       try:
@@ -2157,6 +2195,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 = []
@@ -3444,6 +3484,67 @@ class ManifestProject(MetaProject):
     """Return the name of the platform."""
     return platform.system().lower()
 
+  def SyncWithPossibleInit(self, submanifest, verbose=False,
+                           current_branch_only=False, tags='', git_event_log=None):
+    """Sync a manifestProject, possibly for the first time.
+
+    Call Sync() with arguments from the most recent `repo init`.  If this is a
+    new sub manifest, then inherit options from the parent's manifestProject.
+
+    This is used by subcmds.Sync() to do an initial download of new sub
+    manifests.
+
+    Args:
+      submanifest: an XmlSubmanifest, the submanifest to re-sync.
+      verbose: a boolean, whether to show all output, rather than only errors.
+      current_branch_only: a boolean, whether to only fetch the current manifest
+          branch from the server.
+      tags: a boolean, whether to fetch tags.
+      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 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
+    # it doesn't exist.
+    #
+    # Today, we only support changing manifest_groups on the sub-manifest, with
+    # no supported-for-the-user way to change the other arguments from those
+    # specified by the outermost manifest.
+    #
+    # TODO(lamontjones): determine which of these should come from the outermost
+    # manifest and which should come from the parent manifest.
+    mp = self if self.Exists else submanifest.parent.manifestProject
+    return self.Sync(
+        manifest_url=spec.manifestUrl,
+        manifest_branch=spec.revision,
+        standalone_manifest=mp.standalone_manifest_url,
+        groups=mp.manifest_groups,
+        platform=mp.manifest_platform,
+        mirror=mp.mirror,
+        dissociate=mp.dissociate,
+        reference=mp.reference,
+        worktree=mp.use_worktree,
+        submodules=mp.submodules,
+        archive=mp.archive,
+        partial_clone=mp.partial_clone,
+        clone_filter=mp.clone_filter,
+        partial_clone_exclude=mp.partial_clone_exclude,
+        clone_bundle=mp.clone_bundle,
+        git_lfs=mp.git_lfs,
+        use_superproject=mp.use_superproject,
+        verbose=verbose,
+        current_branch_only=current_branch_only,
+        tags=tags,
+        depth=mp.depth,
+        git_event_log=git_event_log,
+        manifest_name=spec.manifestName,
+        this_manifest_only=True,
+        outer_manifest=False,
+    )
+
   def Sync(self, _kwargs_only=(), manifest_url='', manifest_branch=None,
            standalone_manifest=False, groups='', mirror=False, reference='',
            dissociate=False, worktree=False, submodules=False, archive=False,
@@ -3496,7 +3597,7 @@ class ManifestProject(MetaProject):
     """
     assert _kwargs_only == (), 'Sync only accepts keyword arguments.'
 
-    groups = groups or 'default'
+    groups = groups or self.manifest.GetDefaultGroupsStr(with_platform=False)
     platform = platform or 'auto'
     git_event_log = git_event_log or EventLog()
     if outer_manifest and self.manifest.is_submanifest:
@@ -3710,46 +3811,45 @@ class ManifestProject(MetaProject):
     if use_superproject is not None:
       self.config.SetBoolean('repo.superproject', use_superproject)
 
-    if standalone_manifest:
-      if is_new:
-        manifest_name = 'default.xml'
-        manifest_data = fetch.fetch_file(manifest_url, verbose=verbose)
-        dest = os.path.join(self.worktree, manifest_name)
-        os.makedirs(os.path.dirname(dest), exist_ok=True)
-        with open(dest, 'wb') as f:
-          f.write(manifest_data)
-      return
+    if not standalone_manifest:
+      if not self.Sync_NetworkHalf(
+          is_new=is_new, quiet=not verbose, verbose=verbose,
+          clone_bundle=clone_bundle, current_branch_only=current_branch_only,
+          tags=tags, submodules=submodules, clone_filter=clone_filter,
+          partial_clone_exclude=self.manifest.PartialCloneExclude):
+        r = self.GetRemote(self.remote.name)
+        print('fatal: cannot obtain manifest %s' % r.url, file=sys.stderr)
 
-    if not self.Sync_NetworkHalf(is_new=is_new, quiet=not verbose, verbose=verbose,
-                              clone_bundle=clone_bundle,
-                              current_branch_only=current_branch_only,
-                              tags=tags, submodules=submodules,
-                              clone_filter=clone_filter,
-                              partial_clone_exclude=self.manifest.PartialCloneExclude):
-      r = self.GetRemote(self.remote.name)
-      print('fatal: cannot obtain manifest %s' % r.url, file=sys.stderr)
-
-      # Better delete the manifest git dir if we created it; otherwise next
-      # time (when user fixes problems) we won't go through the "is_new" logic.
-      if is_new:
-        platform_utils.rmtree(self.gitdir)
-      return False
-
-    if manifest_branch:
-      self.MetaBranchSwitch(submodules=submodules)
-
-    syncbuf = SyncBuffer(self.config)
-    self.Sync_LocalHalf(syncbuf, submodules=submodules)
-    syncbuf.Finish()
-
-    if is_new or self.CurrentBranch is None:
-      if not self.StartBranch('default'):
-        print('fatal: cannot create default in manifest', file=sys.stderr)
+        # Better delete the manifest git dir if we created it; otherwise next
+        # time (when user fixes problems) we won't go through the "is_new" logic.
+        if is_new:
+          platform_utils.rmtree(self.gitdir)
         return False
 
-    if not manifest_name:
-      print('fatal: manifest name (-m) is required.', file=sys.stderr)
-      return False
+      if manifest_branch:
+        self.MetaBranchSwitch(submodules=submodules)
+
+      syncbuf = SyncBuffer(self.config)
+      self.Sync_LocalHalf(syncbuf, submodules=submodules)
+      syncbuf.Finish()
+
+      if is_new or self.CurrentBranch is None:
+        if not self.StartBranch('default'):
+          print('fatal: cannot create default in manifest', file=sys.stderr)
+          return False
+
+      if not manifest_name:
+        print('fatal: manifest name (-m) is required.', file=sys.stderr)
+        return False
+
+    elif is_new:
+      # This is a new standalone manifest.
+      manifest_name = 'default.xml'
+      manifest_data = fetch.fetch_file(manifest_url, verbose=verbose)
+      dest = os.path.join(self.worktree, manifest_name)
+      os.makedirs(os.path.dirname(dest), exist_ok=True)
+      with open(dest, 'wb') as f:
+        f.write(manifest_data)
 
     try:
       self.manifest.Link(manifest_name)
@@ -3790,10 +3890,10 @@ class ManifestProject(MetaProject):
             outer_manifest=False,
         )
 
-    # Lastly, clone the superproject(s).
-    if self.manifest.manifestProject.use_superproject:
-      sync_result = Superproject(
-          self.manifest, self.manifest.repodir, git_event_log, quiet=not verbose).Sync()
+    # Lastly, if the manifest has a <superproject> then have the 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 '
               f'{self.manifest.path_prefix} failed, repo sync will not use '

subcmds/info.py

@@ -65,8 +65,7 @@ class Info(PagedCommand):
       self.manifest = self.manifest.outer_client
     manifestConfig = self.manifest.manifestProject.config
     mergeBranch = manifestConfig.GetBranch("default").merge
-    manifestGroups = (manifestConfig.GetString('manifest.groups')
-                      or 'all,-notdefault')
+    manifestGroups = self.manifest.GetGroupsStr()
 
     self.heading("Manifest branch: ")
     if self.manifest.default.revisionExpr:

subcmds/init.py

@@ -89,11 +89,10 @@ to update the working directory files.
   def _Options(self, p, gitc_init=False):
     Wrapper().InitParser(p, gitc_init=gitc_init)
     m = p.add_option_group('Multi-manifest')
-    m.add_option('--outer-manifest', action='store_true',
+    m.add_option('--outer-manifest', action='store_true', default=True,
                  help='operate starting at the outermost manifest')
     m.add_option('--no-outer-manifest', dest='outer_manifest',
-                 action='store_false', default=None,
-                 help='do not operate on outer manifests')
+                 action='store_false', help='do not operate on outer manifests')
     m.add_option('--this-manifest-only', action='store_true', default=None,
                  help='only operate on this (sub)manifest')
     m.add_option('--no-this-manifest-only', '--all-manifests',

subcmds/sync.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import collections
 import functools
 import http.cookiejar as cookielib
 import io
@@ -66,7 +67,7 @@ _ONE_DAY_S = 24 * 60 * 60
 class Sync(Command, MirrorSafeCommand):
   jobs = 1
   COMMON = True
-  MULTI_MANIFEST_SUPPORT = False
+  MULTI_MANIFEST_SUPPORT = True
   helpSummary = "Update working tree to the latest revision"
   helpUsage = """
 %prog [<project>...]
@@ -295,52 +296,92 @@ later is required to fix a server side protocol bug.
     """
     return git_superproject.UseSuperproject(opt.use_superproject, manifest) or opt.current_branch_only
 
-  def _UpdateProjectsRevisionId(self, opt, args, load_local_manifests, superproject_logging_data, manifest):
-    """Update revisionId of every project with the SHA from superproject.
+  def _UpdateProjectsRevisionId(self, opt, args, superproject_logging_data,
+                                manifest):
+    """Update revisionId of projects with the commit hash from the superproject.
 
-    This function updates each project's revisionId with SHA from superproject.
-    It writes the updated manifest into a file and reloads the manifest from it.
+    This function updates each project's revisionId with the commit hash from
+    the superproject.  It writes the updated manifest into a file and reloads
+    the manifest from it.  When appropriate, sub manifests are also processed.
 
     Args:
       opt: Program options returned from optparse.  See _Options().
       args: Arguments to pass to GetProjects. See the GetProjects
           docstring for details.
-      load_local_manifests: Whether to load local manifests.
-      superproject_logging_data: A dictionary of superproject data that is to be logged.
+      superproject_logging_data: A dictionary of superproject data to log.
       manifest: The manifest to use.
-
-    Returns:
-      Returns path to the overriding manifest file instead of None.
     """
-    superproject = self.manifest.superproject
-    superproject.SetQuiet(opt.quiet)
-    print_messages = git_superproject.PrintMessages(opt.use_superproject,
-                                                    self.manifest)
-    superproject.SetPrintMessages(print_messages)
-    if opt.local_only:
-      manifest_path = superproject.manifest_path
+    have_superproject = manifest.superproject or any(
+        m.superproject for m in manifest.all_children)
+    if not have_superproject:
+      return
+
+    if opt.local_only and manifest.superproject:
+      manifest_path = manifest.superproject.manifest_path
       if manifest_path:
-        self._ReloadManifest(manifest_path, manifest, load_local_manifests)
-      return manifest_path
+        self._ReloadManifest(manifest_path, manifest)
+      return
 
     all_projects = self.GetProjects(args,
                                     missing_ok=True,
-                                    submodules_ok=opt.fetch_submodules)
-    update_result = superproject.UpdateProjectsRevisionId(
-        all_projects, git_event_log=self.git_event_log)
-    manifest_path = update_result.manifest_path
-    superproject_logging_data['updatedrevisionid'] = bool(manifest_path)
-    if manifest_path:
-      self._ReloadManifest(manifest_path, manifest, load_local_manifests)
+                                    submodules_ok=opt.fetch_submodules,
+                                    manifest=manifest,
+                                    all_manifests=not opt.this_manifest_only)
+
+    per_manifest = collections.defaultdict(list)
+    manifest_paths = {}
+    if opt.this_manifest_only:
+      per_manifest[manifest.path_prefix] = all_projects
     else:
-      if print_messages:
-        print('warning: Update of revisionId from superproject has failed, '
-              'repo sync will not use superproject to fetch the source. ',
-              'Please resync with the --no-use-superproject option to avoid this repo warning.',
-              file=sys.stderr)
-      if update_result.fatal and opt.use_superproject is not None:
-        sys.exit(1)
-    return manifest_path
+      for p in all_projects:
+        per_manifest[p.manifest.path_prefix].append(p)
+
+    superproject_logging_data = {}
+    need_unload = False
+    for m in self.ManifestList(opt):
+      if not m.path_prefix in per_manifest:
+        continue
+      use_super = git_superproject.UseSuperproject(opt.use_superproject, m)
+      if superproject_logging_data:
+        superproject_logging_data['multimanifest'] = True
+      superproject_logging_data.update(
+          superproject=use_super,
+          haslocalmanifests=bool(m.HasLocalManifests),
+          hassuperprojecttag=bool(m.superproject),
+      )
+      if use_super and (m.IsMirror or m.IsArchive):
+        # Don't use superproject, because we have no working tree.
+        use_super = False
+        superproject_logging_data['superproject'] = False
+        superproject_logging_data['noworktree'] = True
+        if opt.use_superproject is not False:
+          print(f'{m.path_prefix}: not using superproject because there is no '
+                'working tree.')
+
+      if not use_super:
+        continue
+      m.superproject.SetQuiet(opt.quiet)
+      print_messages = git_superproject.PrintMessages(opt.use_superproject, m)
+      m.superproject.SetPrintMessages(print_messages)
+      update_result = m.superproject.UpdateProjectsRevisionId(
+          per_manifest[m.path_prefix], git_event_log=self.git_event_log)
+      manifest_path = update_result.manifest_path
+      superproject_logging_data['updatedrevisionid'] = bool(manifest_path)
+      if manifest_path:
+        m.SetManifestOverride(manifest_path)
+        need_unload = True
+      else:
+        if print_messages:
+          print(f'{m.path_prefix}: warning: Update of revisionId from '
+                'superproject has failed, repo sync will not use superproject '
+                'to fetch the source. ',
+                'Please resync with the --no-use-superproject option to avoid '
+                'this repo warning.',
+                file=sys.stderr)
+        if update_result.fatal and opt.use_superproject is not None:
+          sys.exit(1)
+    if need_unload:
+      m.outer_client.manifest.Unload()
 
   def _FetchProjectList(self, opt, projects):
     """Main function of the fetch worker.
@@ -485,8 +526,8 @@ later is required to fix a server side protocol bug.
 
     return (ret, fetched)
 
-  def _FetchMain(self, opt, args, all_projects, err_event, manifest_name,
-                 load_local_manifests, ssh_proxy, manifest):
+  def _FetchMain(self, opt, args, all_projects, err_event,
+                 ssh_proxy, manifest):
     """The main network fetch loop.
 
     Args:
@@ -494,8 +535,6 @@ later is required to fix a server side protocol bug.
       args: Command line args used to filter out projects.
       all_projects: List of all projects that should be fetched.
       err_event: Whether an error was hit while processing.
-      manifest_name: Manifest file to be reloaded.
-      load_local_manifests: Whether to load local manifests.
       ssh_proxy: SSH manager for clients & masters.
       manifest: The manifest to use.
 
@@ -526,10 +565,12 @@ later is required to fix a server side protocol bug.
     # Iteratively fetch missing and/or nested unregistered submodules
     previously_missing_set = set()
     while True:
-      self._ReloadManifest(manifest_name, self.manifest, load_local_manifests)
+      self._ReloadManifest(None, manifest)
       all_projects = self.GetProjects(args,
                                       missing_ok=True,
-                                      submodules_ok=opt.fetch_submodules)
+                                      submodules_ok=opt.fetch_submodules,
+                                      manifest=manifest,
+                                      all_manifests=not opt.this_manifest_only)
       missing = []
       for project in all_projects:
         if project.gitdir not in fetched:
@@ -624,7 +665,7 @@ later is required to fix a server side protocol bug.
     for project in projects:
       # Make sure pruning never kicks in with shared projects.
       if (not project.use_git_worktrees and
-              len(project.manifest.GetProjectsWithName(project.name)) > 1):
+              len(project.manifest.GetProjectsWithName(project.name, all_manifests=True)) > 1):
         if not opt.quiet:
           print('\r%s: Shared project %s found, disabling pruning.' %
                 (project.relpath, project.name))
@@ -698,7 +739,7 @@ later is required to fix a server side protocol bug.
       t.join()
     pm.end()
 
-  def _ReloadManifest(self, manifest_name, manifest, load_local_manifests=True):
+  def _ReloadManifest(self, manifest_name, manifest):
     """Reload the manfiest from the file specified by the |manifest_name|.
 
     It unloads the manifest if |manifest_name| is None.
@@ -706,17 +747,29 @@ later is required to fix a server side protocol bug.
     Args:
       manifest_name: Manifest file to be reloaded.
       manifest: The manifest to use.
-      load_local_manifests: Whether to load local manifests.
     """
     if manifest_name:
       # Override calls Unload already
-      manifest.Override(manifest_name, load_local_manifests=load_local_manifests)
+      manifest.Override(manifest_name)
     else:
       manifest.Unload()
 
   def UpdateProjectList(self, opt, manifest):
+    """Update the cached projects list for |manifest|
+
+    In a multi-manifest checkout, each manifest has its own project.list.
+
+    Args:
+      opt: Program options returned from optparse.  See _Options().
+      manifest: The manifest to use.
+
+    Returns:
+      0: success
+      1: failure
+    """
     new_project_paths = []
-    for project in self.GetProjects(None, missing_ok=True):
+    for project in self.GetProjects(None, missing_ok=True, manifest=manifest,
+                                    all_manifests=False):
       if project.relpath:
         new_project_paths.append(project.relpath)
     file_name = 'project.list'
@@ -766,7 +819,8 @@ later is required to fix a server side protocol bug.
     new_paths = {}
     new_linkfile_paths = []
     new_copyfile_paths = []
-    for project in self.GetProjects(None, missing_ok=True):
+    for project in self.GetProjects(None, missing_ok=True,
+                                    manifest=manifest, all_manifests=False):
       new_linkfile_paths.extend(x.dest for x in project.linkfiles)
       new_copyfile_paths.extend(x.dest for x in project.copyfiles)
 
@@ -897,8 +951,40 @@ later is required to fix a server side protocol bug.
 
     return manifest_name
 
+  def _UpdateAllManifestProjects(self, opt, mp, manifest_name):
+    """Fetch & update the local manifest project.
+
+    After syncing the manifest project, if the manifest has any sub manifests,
+    those are recursively processed.
+
+    Args:
+      opt: Program options returned from optparse.  See _Options().
+      mp: the manifestProject to query.
+      manifest_name: Manifest file to be reloaded.
+    """
+    if not mp.standalone_manifest_url:
+      self._UpdateManifestProject(opt, mp, manifest_name)
+
+    if mp.manifest.submanifests:
+      for submanifest in mp.manifest.submanifests.values():
+        child = submanifest.repo_client.manifest
+        child.manifestProject.SyncWithPossibleInit(
+            submanifest,
+            current_branch_only=self._GetCurrentBranchOnly(opt, child),
+            verbose=opt.verbose,
+            tags=opt.tags,
+            git_event_log=self.git_event_log,
+        )
+        self._UpdateAllManifestProjects(opt, child.manifestProject, None)
+
   def _UpdateManifestProject(self, opt, mp, manifest_name):
-    """Fetch & update the local manifest project."""
+    """Fetch & update the local manifest project.
+
+    Args:
+      opt: Program options returned from optparse.  See _Options().
+      mp: the manifestProject to query.
+      manifest_name: Manifest file to be reloaded.
+    """
     if not opt.local_only:
       start = time.time()
       success = mp.Sync_NetworkHalf(quiet=opt.quiet, verbose=opt.verbose,
@@ -924,6 +1010,7 @@ later is required to fix a server side protocol bug.
       if not clean:
         sys.exit(1)
       self._ReloadManifest(manifest_name, mp.manifest)
+
       if opt.jobs is None:
         self.jobs = mp.manifest.default.sync_j
 
@@ -948,9 +1035,6 @@ later is required to fix a server side protocol bug.
     if opt.prune is None:
       opt.prune = True
 
-    if self.outer_client.manifest.is_multimanifest and not opt.this_manifest_only and args:
-      self.OptionParser.error('partial syncs must use --this-manifest-only')
-
   def Execute(self, opt, args):
     if opt.jobs:
       self.jobs = opt.jobs
@@ -959,7 +1043,7 @@ later is required to fix a server side protocol bug.
       self.jobs = min(self.jobs, (soft_limit - 5) // 3)
 
     manifest = self.outer_manifest
-    if opt.this_manifest_only or not opt.outer_manifest:
+    if not opt.outer_manifest:
       manifest = self.manifest
 
     if opt.manifest_name:
@@ -994,39 +1078,26 @@ later is required to fix a server side protocol bug.
               'receive updates; run `repo init --repo-rev=stable` to fix.',
               file=sys.stderr)
 
-    mp = manifest.manifestProject
-    is_standalone_manifest = bool(mp.standalone_manifest_url)
-    if not is_standalone_manifest:
-      mp.PreSync()
+    for m in self.ManifestList(opt):
+      mp = m.manifestProject
+      is_standalone_manifest = bool(mp.standalone_manifest_url)
+      if not is_standalone_manifest:
+        mp.PreSync()
 
-    if opt.repo_upgraded:
-      _PostRepoUpgrade(manifest, quiet=opt.quiet)
+      if opt.repo_upgraded:
+        _PostRepoUpgrade(m, quiet=opt.quiet)
 
-    if not opt.mp_update:
+    if opt.mp_update:
+      self._UpdateAllManifestProjects(opt, mp, manifest_name)
+    else:
       print('Skipping update of local manifest project.')
-    elif not is_standalone_manifest:
-      self._UpdateManifestProject(opt, mp, manifest_name)
 
-    load_local_manifests = not manifest.HasLocalManifests
-    use_superproject = git_superproject.UseSuperproject(opt.use_superproject, manifest)
-    if use_superproject and (manifest.IsMirror or manifest.IsArchive):
-      # Don't use superproject, because we have no working tree.
-      use_superproject = False
-      if opt.use_superproject is not None:
-        print('Defaulting to no-use-superproject because there is no working tree.')
-    superproject_logging_data = {
-        'superproject': use_superproject,
-        'haslocalmanifests': bool(manifest.HasLocalManifests),
-        'hassuperprojecttag': bool(manifest.superproject),
-    }
-    if use_superproject:
-      manifest_name = self._UpdateProjectsRevisionId(
-          opt, args, load_local_manifests, superproject_logging_data,
-          manifest) or opt.manifest_name
+    superproject_logging_data = {}
+    self._UpdateProjectsRevisionId(opt, args, superproject_logging_data,
+                                   manifest)
 
     if self.gitc_manifest:
-      gitc_manifest_projects = self.GetProjects(args,
-                                                missing_ok=True)
+      gitc_manifest_projects = self.GetProjects(args, missing_ok=True)
       gitc_projects = []
       opened_projects = []
       for project in gitc_manifest_projects:
@@ -1059,9 +1130,12 @@ later is required to fix a server side protocol bug.
               for path in opened_projects]
       if not args:
         return
+
     all_projects = self.GetProjects(args,
                                     missing_ok=True,
-                                    submodules_ok=opt.fetch_submodules)
+                                    submodules_ok=opt.fetch_submodules,
+                                    manifest=manifest,
+                                    all_manifests=not opt.this_manifest_only)
 
     err_network_sync = False
     err_update_projects = False
@@ -1073,7 +1147,6 @@ later is required to fix a server side protocol bug.
           # Initialize the socket dir once in the parent.
           ssh_proxy.sock()
           all_projects = self._FetchMain(opt, args, all_projects, err_event,
-                                         manifest_name, load_local_manifests,
                                          ssh_proxy, manifest)
 
       if opt.network_only:
@@ -1090,23 +1163,24 @@ later is required to fix a server side protocol bug.
                 file=sys.stderr)
           sys.exit(1)
 
-    if manifest.IsMirror or manifest.IsArchive:
-      # bail out now, we have no working tree
-      return
+    for m in self.ManifestList(opt):
+      if m.IsMirror or m.IsArchive:
+        # bail out now, we have no working tree
+        continue
 
-    if self.UpdateProjectList(opt, manifest):
-      err_event.set()
-      err_update_projects = True
-      if opt.fail_fast:
-        print('\nerror: Local checkouts *not* updated.', file=sys.stderr)
-        sys.exit(1)
+      if self.UpdateProjectList(opt, m):
+        err_event.set()
+        err_update_projects = True
+        if opt.fail_fast:
+          print('\nerror: Local checkouts *not* updated.', file=sys.stderr)
+          sys.exit(1)
 
-    err_update_linkfiles = not self.UpdateCopyLinkfileList(manifest)
-    if err_update_linkfiles:
-      err_event.set()
-      if opt.fail_fast:
-        print('\nerror: Local update copyfile or linkfile failed.', file=sys.stderr)
-        sys.exit(1)
+      err_update_linkfiles = not self.UpdateCopyLinkfileList(m)
+      if err_update_linkfiles:
+        err_event.set()
+        if opt.fail_fast:
+          print('\nerror: Local update copyfile or linkfile failed.', file=sys.stderr)
+          sys.exit(1)
 
     err_results = []
     # NB: We don't exit here because this is the last step.
@@ -1114,10 +1188,14 @@ later is required to fix a server side protocol bug.
     if err_checkout:
       err_event.set()
 
-    # If there's a notice that's supposed to print at the end of the sync, print
-    # it now...
-    if manifest.notice:
-      print(manifest.notice)
+    printed_notices = set()
+    # If there's a notice that's supposed to print at the end of the sync,
+    # print it now...  But avoid printing duplicate messages, and preserve
+    # order.
+    for m in sorted(self.ManifestList(opt), key=lambda x: x.path_prefix):
+      if m.notice and m.notice not in printed_notices:
+        print(m.notice)
+        printed_notices.add(m.notice)
 
     # If we saw an error, exit with code 1 so that other scripts can check.
     if err_event.is_set():

subcmds/upload.py

@@ -421,12 +421,6 @@ Gerrit Code Review:  https://www.gerritcodereview.com/
         labels = set(_ExpandCommaList(branch.project.config.GetString(key)))
         for label in opt.labels:
           labels.update(_ExpandCommaList(label))
-        # Basic sanity check on label syntax.
-        for label in labels:
-          if not re.match(r'^.+[+-][0-9]+$', label):
-            print('repo: error: invalid label syntax "%s": labels use forms '
-                  'like CodeReview+1 or Verified-1' % (label,), file=sys.stderr)
-            sys.exit(1)
 
         # Handle e-mail notifications.
         if opt.notify is False:

tests/test_git_superproject.py

@@ -24,7 +24,6 @@ from unittest import mock
 import git_superproject
 import git_trace2_event_log
 import manifest_xml
-import platform_utils
 from test_manifest_xml import sort_attributes
 
 
@@ -38,7 +37,8 @@ class SuperprojectTestCase(unittest.TestCase):
 
   def setUp(self):
     """Set up superproject every time."""
-    self.tempdir = tempfile.mkdtemp(prefix='repo_tests')
+    self.tempdirobj = tempfile.TemporaryDirectory(prefix='repo_tests')
+    self.tempdir = self.tempdirobj.name
     self.repodir = os.path.join(self.tempdir, '.repo')
     self.manifest_file = os.path.join(
         self.repodir, manifest_xml.MANIFEST_FILE_NAME)
@@ -75,7 +75,7 @@ class SuperprojectTestCase(unittest.TestCase):
 
   def tearDown(self):
     """Tear down superproject every time."""
-    platform_utils.rmtree(self.tempdir)
+    self.tempdirobj.cleanup()
 
   def getXmlManifest(self, data):
     """Helper to initialize a manifest for testing."""

tests/test_manifest_xml.py

@@ -17,7 +17,6 @@
 import os
 import platform
 import re
-import shutil
 import tempfile
 import unittest
 import xml.dom.minidom
@@ -92,7 +91,8 @@ class ManifestParseTestCase(unittest.TestCase):
   """TestCase for parsing manifests."""
 
   def setUp(self):
-    self.tempdir = tempfile.mkdtemp(prefix='repo_tests')
+    self.tempdirobj = tempfile.TemporaryDirectory(prefix='repo_tests')
+    self.tempdir = self.tempdirobj.name
     self.repodir = os.path.join(self.tempdir, '.repo')
     self.manifest_dir = os.path.join(self.repodir, 'manifests')
     self.manifest_file = os.path.join(
@@ -111,7 +111,7 @@ class ManifestParseTestCase(unittest.TestCase):
 """)
 
   def tearDown(self):
-    shutil.rmtree(self.tempdir, ignore_errors=True)
+    self.tempdirobj.cleanup()
 
   def getXmlManifest(self, data):
     """Helper to initialize a manifest for testing."""

tests/test_project.py

@@ -17,7 +17,6 @@
 import contextlib
 import os
 from pathlib import Path
-import shutil
 import subprocess
 import tempfile
 import unittest
@@ -32,11 +31,7 @@ import project
 @contextlib.contextmanager
 def TempGitTree():
   """Create a new empty git checkout for testing."""
-  # TODO(vapier): Convert this to tempfile.TemporaryDirectory once we drop
-  # Python 2 support entirely.
-  try:
-    tempdir = tempfile.mkdtemp(prefix='repo-tests')
-
+  with tempfile.TemporaryDirectory(prefix='repo-tests') as tempdir:
     # Tests need to assume, that main is default branch at init,
     # which is not supported in config until 2.28.
     cmd = ['git', 'init']
@@ -50,8 +45,6 @@ def TempGitTree():
       cmd += ['--template', templatedir]
     subprocess.check_call(cmd, cwd=tempdir)
     yield tempdir
-  finally:
-    platform_utils.rmtree(tempdir)
 
 
 class FakeProject(object):
@@ -124,14 +117,15 @@ class CopyLinkTestCase(unittest.TestCase):
   """
 
   def setUp(self):
-    self.tempdir = tempfile.mkdtemp(prefix='repo_tests')
+    self.tempdirobj = tempfile.TemporaryDirectory(prefix='repo_tests')
+    self.tempdir = self.tempdirobj.name
     self.topdir = os.path.join(self.tempdir, 'checkout')
     self.worktree = os.path.join(self.topdir, 'git-project')
     os.makedirs(self.topdir)
     os.makedirs(self.worktree)
 
   def tearDown(self):
-    shutil.rmtree(self.tempdir, ignore_errors=True)
+    self.tempdirobj.cleanup()
 
   @staticmethod
   def touch(path):

tests/test_wrapper.py

@@ -14,11 +14,9 @@
 
 """Unittests for the wrapper.py module."""
 
-import contextlib
 from io import StringIO
 import os
 import re
-import shutil
 import sys
 import tempfile
 import unittest
@@ -26,22 +24,9 @@ from unittest import mock
 
 import git_command
 import main
-import platform_utils
 import wrapper
 
 
-@contextlib.contextmanager
-def TemporaryDirectory():
-  """Create a new empty git checkout for testing."""
-  # TODO(vapier): Convert this to tempfile.TemporaryDirectory once we drop
-  # Python 2 support entirely.
-  try:
-    tempdir = tempfile.mkdtemp(prefix='repo-tests')
-    yield tempdir
-  finally:
-    platform_utils.rmtree(tempdir)
-
-
 def fixture(*paths):
   """Return a path relative to tests/fixtures.
   """
@@ -336,19 +321,19 @@ class NeedSetupGnuPG(RepoWrapperTestCase):
 
   def test_missing_dir(self):
     """The ~/.repoconfig tree doesn't exist yet."""
-    with TemporaryDirectory() as tempdir:
+    with tempfile.TemporaryDirectory(prefix='repo-tests') as tempdir:
       self.wrapper.home_dot_repo = os.path.join(tempdir, 'foo')
       self.assertTrue(self.wrapper.NeedSetupGnuPG())
 
   def test_missing_keyring(self):
     """The keyring-version file doesn't exist yet."""
-    with TemporaryDirectory() as tempdir:
+    with tempfile.TemporaryDirectory(prefix='repo-tests') as tempdir:
       self.wrapper.home_dot_repo = tempdir
       self.assertTrue(self.wrapper.NeedSetupGnuPG())
 
   def test_empty_keyring(self):
     """The keyring-version file exists, but is empty."""
-    with TemporaryDirectory() as tempdir:
+    with tempfile.TemporaryDirectory(prefix='repo-tests') as tempdir:
       self.wrapper.home_dot_repo = tempdir
       with open(os.path.join(tempdir, 'keyring-version'), 'w'):
         pass
@@ -356,7 +341,7 @@ class NeedSetupGnuPG(RepoWrapperTestCase):
 
   def test_old_keyring(self):
     """The keyring-version file exists, but it's old."""
-    with TemporaryDirectory() as tempdir:
+    with tempfile.TemporaryDirectory(prefix='repo-tests') as tempdir:
       self.wrapper.home_dot_repo = tempdir
       with open(os.path.join(tempdir, 'keyring-version'), 'w') as fp:
         fp.write('1.0\n')
@@ -364,7 +349,7 @@ class NeedSetupGnuPG(RepoWrapperTestCase):
 
   def test_new_keyring(self):
     """The keyring-version file exists, and is up-to-date."""
-    with TemporaryDirectory() as tempdir:
+    with tempfile.TemporaryDirectory(prefix='repo-tests') as tempdir:
       self.wrapper.home_dot_repo = tempdir
       with open(os.path.join(tempdir, 'keyring-version'), 'w') as fp:
         fp.write('1000.0\n')
@@ -376,7 +361,7 @@ class SetupGnuPG(RepoWrapperTestCase):
 
   def test_full(self):
     """Make sure it works completely."""
-    with TemporaryDirectory() as tempdir:
+    with tempfile.TemporaryDirectory(prefix='repo-tests') as tempdir:
       self.wrapper.home_dot_repo = tempdir
       self.wrapper.gpg_dir = os.path.join(self.wrapper.home_dot_repo, 'gnupg')
       self.assertTrue(self.wrapper.SetupGnuPG(True))
@@ -426,7 +411,8 @@ class GitCheckoutTestCase(RepoWrapperTestCase):
   @classmethod
   def setUpClass(cls):
     # Create a repo to operate on, but do it once per-class.
-    cls.GIT_DIR = tempfile.mkdtemp(prefix='repo-rev-tests')
+    cls.tempdirobj = tempfile.TemporaryDirectory(prefix='repo-rev-tests')
+    cls.GIT_DIR = cls.tempdirobj.name
     run_git = wrapper.Wrapper().run_git
 
     remote = os.path.join(cls.GIT_DIR, 'remote')
@@ -455,10 +441,10 @@ class GitCheckoutTestCase(RepoWrapperTestCase):
 
   @classmethod
   def tearDownClass(cls):
-    if not cls.GIT_DIR:
+    if not cls.tempdirobj:
       return
 
-    shutil.rmtree(cls.GIT_DIR)
+    cls.tempdirobj.cleanup()
 
 
 class ResolveRepoRev(GitCheckoutTestCase):