superproject: Provide accurate feedback for user choice

Currently the code would give a message that would appear like the user
have enrolled the experiment regardless of the actual choice. For users
who choose to not enroll in the experiment, we should give them
instructions to override (enable) superproject once instead of how to
disable it, which is what the code already behave.

Bug: [google internal] b/199167992
Change-Id: Iba3314cb510aedf024375a26baa8bc1d5e2846cf
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/317382
Tested-by: Xin Li <delphij@google.com>
Reviewed-by: Raman Tenneti <rtenneti@google.com>

command.py

@@ -24,6 +24,10 @@ from error import InvalidProjectGroupsError
 import progress
 
 
+# Are we generating man-pages?
+GENERATE_MANPAGES = os.environ.get('_REPO_GENERATE_MANPAGES_') == ' indeed! '
+
+
 # Number of projects to submit to a single worker process at a time.
 # This number represents a tradeoff between the overhead of IPC and finer
 # grained opportunity for parallelism. This particular value was chosen by
@@ -122,10 +126,14 @@ class Command(object):
                  help='only show errors')
 
     if self.PARALLEL_JOBS is not None:
+      default = 'based on number of CPU cores'
+      if not GENERATE_MANPAGES:
+        # Only include active cpu count if we aren't generating man pages.
+        default = f'%default; {default}'
       p.add_option(
           '-j', '--jobs',
           type=int, default=self.PARALLEL_JOBS,
-          help='number of jobs to run in parallel (default: %s)' % self.PARALLEL_JOBS)
+          help=f'number of jobs to run in parallel (default: {default})')
 
   def _Options(self, p):
     """Initialize the option parser with subcommand-specific options."""

docs/release-process.md

@@ -277,6 +277,11 @@ Things in italics are things we used to care about but probably don't anymore.
 | Nov 2019 |              | 2.24.0       |
 | Jan 2020 |              | 2.25.0       |                 | **20.04 Focal** |
 | Apr 2020 | **Apr 2030** |              |                 | **20.04 Focal**      | 2.25.0   | 2.7.17 3.7.5 |
+| Oct 2020 | **Oct 2025** |              | 3.9.0           | 21.04 Hirsute / **Bullseye** |
+| Dec 2020 |              | 2.30.0       |                 | 21.04 Hirsute / **Bullseye** |
+| Apr 2021 | *Jan 2022*   |              |                 | 21.04 Hirsute        | 2.30.2   | 2.7.18 3.9.4 |
+| Aug 2021 | **Aug 2026** |              |                 | **Debian 11 Bullseye** | 2.30.2 | 2.7.18 3.9.2 |
+| **Date** |   **EOL**    | **[Git][rel-g]** | **[Python][rel-p]** | **[Ubuntu][rel-u] / [Debian][rel-d]** | **Git** | **Python** |
 
 
 [contact]: ../README.md#contact

git_superproject.py

@@ -59,7 +59,7 @@ class CommitIdsResult(NamedTuple):
 class UpdateProjectsResult(NamedTuple):
   """Return the overriding manifest file and whether caller should exit."""
 
-  # Path name of the overriding manfiest file if successful, otherwise None.
+  # Path name of the overriding manifest file if successful, otherwise None.
   manifest_path: str
   # Whether the caller should exit.
   fatal: bool
@@ -73,7 +73,7 @@ class Superproject(object):
   is a dictionary with project/commit id entries.
   """
   def __init__(self, manifest, repodir, git_event_log,
-               superproject_dir='exp-superproject', quiet=False):
+               superproject_dir='exp-superproject', quiet=False, print_messages=False):
     """Initializes superproject.
 
     Args:
@@ -83,11 +83,13 @@ class Superproject(object):
       git_event_log: A git trace2 event log to log events.
       superproject_dir: Relative path under |repodir| to checkout superproject.
       quiet:  If True then only print the progress messages.
+      print_messages: if True then print error/warning messages.
     """
     self._project_commit_ids = None
     self._manifest = manifest
     self._git_event_log = git_event_log
     self._quiet = quiet
+    self._print_messages = print_messages
     self._branch = self._GetBranch()
     self._repodir = os.path.abspath(repodir)
     self._superproject_dir = superproject_dir
@@ -122,10 +124,19 @@ class Superproject(object):
       branch = branch[len(R_HEADS):]
     return branch
 
-  def _LogError(self, message):
+  def _LogMessage(self, message):
     """Logs message to stderr and _git_event_log."""
+    if self._print_messages:
       print(message, file=sys.stderr)
-    self._git_event_log.ErrorEvent(message, '')
+    self._git_event_log.ErrorEvent(message, f'{message}')
+
+  def _LogError(self, message):
+    """Logs error message to stderr and _git_event_log."""
+    self._LogMessage(f'repo superproject error: {message}')
+
+  def _LogWarning(self, message):
+    """Logs warning message to stderr and _git_event_log."""
+    self._LogMessage(f'repo superproject warning: {message}')
 
   def _Init(self):
     """Sets up a local Git repository to get a copy of a superproject.
@@ -146,7 +157,7 @@ class Superproject(object):
                    capture_stderr=True)
     retval = p.Wait()
     if retval:
-      self._LogError(f'repo: error: git init call failed, command: git {cmd}, '
+      self._LogWarning(f'git init call failed, command: git {cmd}, '
                        f'return code: {retval}, stderr: {p.stderr}')
       return False
     return True
@@ -161,10 +172,10 @@ class Superproject(object):
       True if fetch is successful, or False.
     """
     if not os.path.exists(self._work_git):
-      self._LogError(f'git fetch missing directory: {self._work_git}')
+      self._LogWarning(f'git fetch missing directory: {self._work_git}')
       return False
     if not git_require((2, 28, 0)):
-      print('superproject requires a git version 2.28 or later', file=sys.stderr)
+      self._LogWarning('superproject requires a git version 2.28 or later')
       return False
     cmd = ['fetch', url, '--depth', '1', '--force', '--no-tags', '--filter', 'blob:none']
     if self._branch:
@@ -176,7 +187,7 @@ class Superproject(object):
                    capture_stderr=True)
     retval = p.Wait()
     if retval:
-      self._LogError(f'repo: error: git fetch call failed, command: git {cmd}, '
+      self._LogWarning(f'git fetch call failed, command: git {cmd}, '
                        f'return code: {retval}, stderr: {p.stderr}')
       return False
     return True
@@ -190,7 +201,7 @@ class Superproject(object):
       data: data returned from 'git ls-tree ...' instead of None.
     """
     if not os.path.exists(self._work_git):
-      self._LogError(f'git ls-tree missing directory: {self._work_git}')
+      self._LogWarning(f'git ls-tree missing directory: {self._work_git}')
       return None
     data = None
     branch = 'HEAD' if not self._branch else self._branch
@@ -205,7 +216,7 @@ class Superproject(object):
     if retval == 0:
       data = p.stdout
     else:
-      self._LogError(f'repo: error: git ls-tree call failed, command: git {cmd}, '
+      self._LogWarning(f'git ls-tree call failed, command: git {cmd}, '
                        f'return code: {retval}, stderr: {p.stderr}')
     return data
 
@@ -215,18 +226,17 @@ class Superproject(object):
     Returns:
       SyncResult
     """
-    print('NOTICE: --use-superproject is in beta; report any issues to the '
-          'address described in `repo version`', file=sys.stderr)
-
     if not self._manifest.superproject:
-      self._LogError(f'repo error: superproject tag is not defined in manifest: '
+      self._LogWarning(f'superproject tag is not defined in manifest: '
                        f'{self._manifest.manifestFile}')
       return SyncResult(False, False)
 
+    print('NOTICE: --use-superproject is in beta; report any issues to the '
+          'address described in `repo version`', file=sys.stderr)
     should_exit = True
     url = self._manifest.superproject['remote'].url
     if not url:
-      self._LogError(f'repo error: superproject URL is not defined in manifest: '
+      self._LogWarning(f'superproject URL is not defined in manifest: '
                        f'{self._manifest.manifestFile}')
       return SyncResult(False, should_exit)
 
@@ -250,8 +260,8 @@ class Superproject(object):
 
     data = self._LsTree()
     if not data:
-      print('warning: git ls-tree failed to return data for superproject',
+      self._LogWarning(f'warning: git ls-tree failed to return data for manifest: '
-            file=sys.stderr)
+                       f'{self._manifest.manifestFile}')
       return CommitIdsResult(None, True)
 
     # Parse lines like the following to select lines starting with '160000' and
@@ -270,14 +280,14 @@ class Superproject(object):
     self._project_commit_ids = commit_ids
     return CommitIdsResult(commit_ids, False)
 
-  def _WriteManfiestFile(self):
+  def _WriteManifestFile(self):
     """Writes manifest to a file.
 
     Returns:
       manifest_path: Path name of the file into which manifest is written instead of None.
     """
     if not os.path.exists(self._superproject_path):
-      self._LogError(f'error: missing superproject directory: {self._superproject_path}')
+      self._LogWarning(f'missing superproject directory: {self._superproject_path}')
       return None
     manifest_str = self._manifest.ToXml(groups=self._manifest.GetGroupsStr()).toxml()
     manifest_path = self._manifest_path
@@ -285,7 +295,7 @@ class Superproject(object):
       with open(manifest_path, 'w', encoding='utf-8') as fp:
         fp.write(manifest_str)
     except IOError as e:
-      self._LogError(f'error: cannot write manifest to : {manifest_path} {e}')
+      self._LogError(f'cannot write manifest to : {manifest_path} {e}')
       return None
     return manifest_path
 
@@ -321,7 +331,6 @@ class Superproject(object):
     commit_ids_result = self._GetAllProjectsCommitIds()
     commit_ids = commit_ids_result.commit_ids
     if not commit_ids:
-      print('warning: Cannot get project commit ids from manifest', file=sys.stderr)
       return UpdateProjectsResult(None, commit_ids_result.fatal)
 
     projects_missing_commit_ids = []
@@ -336,7 +345,7 @@ class Superproject(object):
     # If superproject doesn't have a commit id for a project, then report an
     # error event and continue as if do not use superproject is specified.
     if projects_missing_commit_ids:
-      self._LogError(f'error: please file a bug using {self._manifest.contactinfo.bugurl} '
+      self._LogWarning(f'please file a bug using {self._manifest.contactinfo.bugurl} '
                        f'to report missing commit_ids for: {projects_missing_commit_ids}')
       return UpdateProjectsResult(None, False)
 
@@ -344,7 +353,7 @@ class Superproject(object):
       if not self._SkipUpdatingProjectRevisionId(project):
         project.SetRevisionId(commit_ids.get(project.relpath))
 
-    manifest_path = self._WriteManfiestFile()
+    manifest_path = self._WriteManifestFile()
     return UpdateProjectsResult(manifest_path, False)
 
 
@@ -352,7 +361,6 @@ class Superproject(object):
 def _UseSuperprojectFromConfiguration():
   """Returns the user choice of whether to use superproject."""
   user_cfg = RepoConfig.ForUser()
-  system_cfg = RepoConfig.ForSystem()
   time_now = int(time.time())
 
   user_value = user_cfg.GetBoolean('repo.superprojectChoice')
@@ -361,12 +369,18 @@ def _UseSuperprojectFromConfiguration():
     if user_expiration is not None and (user_expiration <= 0 or user_expiration >= time_now):
       # TODO(b/190688390) - Remove prompt when we are comfortable with the new
       # default value.
+      if user_value:
         print(('You are currently enrolled in Git submodules experiment '
                '(go/android-submodules-quickstart).  Use --no-use-superproject '
                'to override.\n'), file=sys.stderr)
+      else:
+        print(('You are not currently enrolled in Git submodules experiment '
+               '(go/android-submodules-quickstart).  Use --use-superproject '
+               'to override.\n'), file=sys.stderr)
     return user_value
 
   # We don't have an unexpired choice, ask for one.
+  system_cfg = RepoConfig.ForSystem()
   system_value = system_cfg.GetBoolean('repo.superprojectChoice')
   if system_value:
     # The system configuration is proposing that we should enable the
@@ -413,6 +427,11 @@ def _UseSuperprojectFromConfiguration():
   return False
 
 
+def PrintMessages(opt, manifest):
+  """Returns a boolean if error/warning messages are to be printed."""
+  return opt.use_superproject is not None or manifest.superproject
+
+
 def UseSuperproject(opt, manifest):
   """Returns a boolean if use-superproject option is enabled."""
 

man/repo-abandon.1

@@ -20,7 +20,8 @@ It is equivalent to "git branch \fB\-D\fR <branchname>".
 show this help message and exit
 .TP
 \fB\-j\fR JOBS, \fB\-\-jobs\fR=\fI\,JOBS\/\fR
-number of jobs to run in parallel (default: 4)
+number of jobs to run in parallel (default: based on
+number of CPU cores)
 .TP
 \fB\-\-all\fR
 delete all branches in all projects

man/repo-branches.1

@@ -46,7 +46,8 @@ is shown, then the branch appears in all projects.
 show this help message and exit
 .TP
 \fB\-j\fR JOBS, \fB\-\-jobs\fR=\fI\,JOBS\/\fR
-number of jobs to run in parallel (default: 4)
+number of jobs to run in parallel (default: based on
+number of CPU cores)
 .SS Logging options:
 .TP
 \fB\-v\fR, \fB\-\-verbose\fR

man/repo-checkout.1

@@ -15,7 +15,8 @@ Checkout a branch for development
 show this help message and exit
 .TP
 \fB\-j\fR JOBS, \fB\-\-jobs\fR=\fI\,JOBS\/\fR
-number of jobs to run in parallel (default: 4)
+number of jobs to run in parallel (default: based on
+number of CPU cores)
 .SS Logging options:
 .TP
 \fB\-v\fR, \fB\-\-verbose\fR

man/repo-diff.1

@@ -19,7 +19,8 @@ to the Unix 'patch' command.
 show this help message and exit
 .TP
 \fB\-j\fR JOBS, \fB\-\-jobs\fR=\fI\,JOBS\/\fR
-number of jobs to run in parallel (default: 4)
+number of jobs to run in parallel (default: based on
+number of CPU cores)
 .TP
 \fB\-u\fR, \fB\-\-absolute\fR
 paths are relative to the repository root

man/repo-forall.1

@@ -17,7 +17,8 @@ repo forall \fB\-r\fR str1 [str2] ... \fB\-c\fR <command> [<arg>...]
 show this help message and exit
 .TP
 \fB\-j\fR JOBS, \fB\-\-jobs\fR=\fI\,JOBS\/\fR
-number of jobs to run in parallel (default: 4)
+number of jobs to run in parallel (default: based on
+number of CPU cores)
 .TP
 \fB\-r\fR, \fB\-\-regex\fR
 execute the command only on projects matching regex or

man/repo-grep.1

@@ -15,7 +15,8 @@ Print lines matching a pattern
 show this help message and exit
 .TP
 \fB\-j\fR JOBS, \fB\-\-jobs\fR=\fI\,JOBS\/\fR
-number of jobs to run in parallel (default: 4)
+number of jobs to run in parallel (default: based on
+number of CPU cores)
 .SS Logging options:
 .TP
 \fB\-\-verbose\fR

man/repo-prune.1

@@ -15,7 +15,8 @@ Prune (delete) already merged topics
 show this help message and exit
 .TP
 \fB\-j\fR JOBS, \fB\-\-jobs\fR=\fI\,JOBS\/\fR
-number of jobs to run in parallel (default: 4)
+number of jobs to run in parallel (default: based on
+number of CPU cores)
 .SS Logging options:
 .TP
 \fB\-v\fR, \fB\-\-verbose\fR

man/repo-smartsync.1

@@ -15,7 +15,8 @@ Update working tree to the latest known good revision
 show this help message and exit
 .TP
 \fB\-j\fR JOBS, \fB\-\-jobs\fR=\fI\,JOBS\/\fR
-number of jobs to run in parallel (default: 1)
+number of jobs to run in parallel (default: based on
+number of CPU cores)
 .TP
 \fB\-\-jobs\-network\fR=\fI\,JOBS\/\fR
 number of network jobs to run in parallel (defaults to

man/repo-start.1

@@ -15,7 +15,8 @@ Start a new branch for development
 show this help message and exit
 .TP
 \fB\-j\fR JOBS, \fB\-\-jobs\fR=\fI\,JOBS\/\fR
-number of jobs to run in parallel (default: 4)
+number of jobs to run in parallel (default: based on
+number of CPU cores)
 .TP
 \fB\-\-all\fR
 begin branch in all projects

man/repo-status.1

@@ -15,7 +15,8 @@ Show the working tree status
 show this help message and exit
 .TP
 \fB\-j\fR JOBS, \fB\-\-jobs\fR=\fI\,JOBS\/\fR
-number of jobs to run in parallel (default: 4)
+number of jobs to run in parallel (default: based on
+number of CPU cores)
 .TP
 \fB\-o\fR, \fB\-\-orphans\fR
 include objects in working directory outside of repo

man/repo-sync.1

@@ -15,7 +15,8 @@ Update working tree to the latest revision
 show this help message and exit
 .TP
 \fB\-j\fR JOBS, \fB\-\-jobs\fR=\fI\,JOBS\/\fR
-number of jobs to run in parallel (default: 1)
+number of jobs to run in parallel (default: based on
+number of CPU cores)
 .TP
 \fB\-\-jobs\-network\fR=\fI\,JOBS\/\fR
 number of network jobs to run in parallel (defaults to

man/repo-upload.1

@@ -15,7 +15,8 @@ Upload changes for code review
 show this help message and exit
 .TP
 \fB\-j\fR JOBS, \fB\-\-jobs\fR=\fI\,JOBS\/\fR
-number of jobs to run in parallel (default: 4)
+number of jobs to run in parallel (default: based on
+number of CPU cores)
 .TP
 \fB\-t\fR
 send local branch name to Gerrit Code Review

release/update-manpages

@@ -47,6 +47,11 @@ def main(argv):
   if not shutil.which('help2man'):
     sys.exit('Please install help2man to continue.')
 
+  # Let repo know we're generating man pages so it can avoid some dynamic
+  # behavior (like probing active number of CPUs).  We use a weird name &
+  # value to make it less likely for users to set this var themselves.
+  os.environ['_REPO_GENERATE_MANPAGES_'] = ' indeed! '
+
   # "repo branch" is an alias for "repo branches".
   del subcmds.all_commands['branch']
   (MANDIR / 'repo-branch.1').write_text('.so man1/repo-branches.1')

subcmds/sync.py

@@ -298,10 +298,12 @@ later is required to fix a server side protocol bug.
     Returns:
       Returns path to the overriding manifest file instead of None.
     """
+    print_messages = git_superproject.PrintMessages(opt, self.manifest)
     superproject = git_superproject.Superproject(self.manifest,
                                                  self.repodir,
                                                  self.git_event_log,
-                                                 quiet=opt.quiet)
+                                                 quiet=opt.quiet,
+                                                 print_messages=print_messages)
     if opt.local_only:
       manifest_path = superproject.manifest_path
       if manifest_path:
@@ -317,6 +319,7 @@ later is required to fix a server side protocol bug.
     if manifest_path:
       self._ReloadManifest(manifest_path, load_local_manifests)
     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.',
@@ -970,6 +973,7 @@ later is required to fix a server side protocol bug.
     superproject_logging_data = {
         'superproject': use_superproject,
         'haslocalmanifests': bool(self.manifest.HasLocalManifests),
+        'hassuperprojecttag': bool(self.manifest.superproject),
     }
     if use_superproject:
       manifest_name = self._UpdateProjectsRevisionId(

tests/fixtures/test.gitconfig

@@ -12,7 +12,7 @@
 	intm = 10m
 	intg = 10g
 [repo "syncstate.main"]
-	synctime = 2021-07-29T22:07:43.463365Z
+	synctime = 2021-08-13T18:37:43.928600Z
 	version = 1
 [repo "syncstate.sys"]
 	argv = ['/usr/bin/pytest-3']

tests/test_git_superproject.py

@@ -203,7 +203,7 @@ class SuperprojectTestCase(unittest.TestCase):
     project.SetRevisionId('ABCDEF')
     # Create temporary directory so that it can write the file.
     os.mkdir(self._superproject._superproject_path)
-    manifest_path = self._superproject._WriteManfiestFile()
+    manifest_path = self._superproject._WriteManifestFile()
     self.assertIsNotNone(manifest_path)
     with open(manifest_path, 'r') as fp:
       manifest_xml_data = fp.read()