From b55769a5c9422e0aac532e901a4d7b5af834b34d Mon Sep 17 00:00:00 2001 From: Raman Tenneti Date: Fri, 13 Aug 2021 11:47:24 -0700 Subject: [PATCH] superproject: print messages if the manifest has superproject tag. 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 Tested-by: Raman Tenneti --- git_superproject.py | 32 +++++++++++++++++++------------- subcmds/sync.py | 14 +++++++++----- tests/fixtures/test.gitconfig | 4 ++-- tests/test_git_superproject.py | 2 +- 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()