superproject: print messages if the manifest has superproject tag.v2.16.5

1) If the manifest has superproject tag (git_master, etc), then
   display error/warning messages (as it is doing today)
2) If the manifest doesn't have superproject tag (nest, chromeos
   manifests), then don't display any error/warning messages about
   superrproject (behave as though user has specified
   --no-use-superproject).
3) Print error/warning messages if --use-superproject passed as
   argument to repo sync.
4) No change in behavior for the repo init command.

git_superproject.py:
+ Fixed typo in _WriteManifestFile method name
+ Superproject accepts print_message  as an argument and it defaults
  to True. All messages that are printed to stderr are controlled by
  this flag. If it is True, then messages get printed.
+ Added PrintMessages function which return true if either
  --use-superproject is specified on the command line or if the
  manifest has a superproject tag.

sync.py:
+ Displays the warning message if PrintMessgages are enabled and
  passes that as argument to superproject object.
+ Added 'hassuperprojecttag' trace2 log entry for analysis. We can
  find users/branches that are using superproject, but the manifest is
  missing the superproject tag.

Tested:
$ ./run_tests

+ Verified printing of messages with and without superproject tag, with
  with --use-superproject option.

+ aosp-master
  $ repo_dev init --use-superproject -u https://android.googlesource.com/platform/manifest
  $ repo_dev sync

+ A manifest without superproject tag.
  $ repo_dev init -m $(pwd)/manifest_7482982.xml
  $ repo_dev sync -n -c -j32 -m $(pwd)/manifest_7482982.xml

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

4 files changed, 31 insertions, 21 deletions

diff --git a/git_superproject.py b/git_superproject.py
index c677924f..3613fb00 100644
--- a/git_superproject.py
+++ b/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
@@ -124,7 +126,8 @@ class Superproject(object):
 
   def _LogMessage(self, message):
     """Logs message to stderr and _git_event_log."""
-    print(message, file=sys.stderr)
+    if self._print_messages:
+      print(message, file=sys.stderr)
     self._git_event_log.ErrorEvent(message, '')
 
   def _LogError(self, message):
@@ -172,7 +175,7 @@ class Superproject(object):
       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:
@@ -223,14 +226,13 @@ 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._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:
@@ -258,8 +260,8 @@ class Superproject(object):
 
     data = self._LsTree()
     if not data:
-      print('warning: git ls-tree failed to return data for superproject',
-            file=sys.stderr)
+      self._LogWarning(f'warning: git ls-tree failed to return data for manifest: '
+                       f'{self._manifest.manifestFile}')
       return CommitIdsResult(None, True)
 
     # Parse lines like the following to select lines starting with '160000' and
@@ -278,7 +280,7 @@ 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:
@@ -329,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 = []
@@ -352,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)
 
 
@@ -360,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')
@@ -375,6 +375,7 @@ def _UseSuperprojectFromConfiguration():
     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
@@ -421,6 +422,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."""
 
diff --git a/subcmds/sync.py b/subcmds/sync.py
index ed656b8c..2a0de0a9 100644
--- a/subcmds/sync.py
+++ b/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,10 +319,11 @@ later is required to fix a server side protocol bug.
     if manifest_path:
       self._ReloadManifest(manifest_path, load_local_manifests)
     else:
-      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 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
@@ -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(
diff --git a/tests/fixtures/test.gitconfig b/tests/fixtures/test.gitconfig
index d69e162c..90afff04 100644
--- a/tests/fixtures/test.gitconfig
+++ b/tests/fixtures/test.gitconfig
@@ -12,10 +12,10 @@
         intm = 10m
         intg = 10g
 [repo "syncstate.main"]
-        synctime = 2021-08-11T17:54:14.530286Z
+        synctime = 2021-08-13T18:37:43.928600Z
         version = 1
 [repo "syncstate.sys"]
-        argv = ['/usr/bin/pytest-3', '-v']
+        argv = ['/usr/bin/pytest-3']
 [repo "syncstate.superproject"]
         test = false
 [repo "syncstate.options"]
diff --git a/tests/test_git_superproject.py b/tests/test_git_superproject.py
index 6ff81843..e9b824d6 100644
--- a/tests/test_git_superproject.py
+++ b/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()