From 0cb6e92ac561878cc7f6d711648e732b498f943f Mon Sep 17 00:00:00 2001 From: Xin Li Date: Wed, 16 Jun 2021 10:19:00 -0700 Subject: [PATCH] Add the ability to administratively enroll repo into using superproject. Repo will remember a choice and an expiration time of the choice, per user, about whether to use superproject by default. When not specified from command line and the choice is not expired, repo would use the user default value. When a user default value is not present and when the system wide enable default is provided in git's system configuration, repo would ask the user for a confirmation which will be valid for two weeks. git_config.py: Add support for system config. When reading system config, we would use --system to avoid hardcoding a path as the value may be different on some other distributions. git_superproject.py: Add a new subroutine, _UseSuperproject(), which returns whether superproject should be used and whether it is from a user configuration. The value is determined in the following order: 1. If the user specifies either --use-superproject or --no-use-superproject, then that choice is being used. 2. If neither is specified, we would then check the saved value (upon repo init) and use that choice when there was a choice. 3. We then check if there is a saved and unexpired value for user's choice in their ~/.gitconfig, and use the unexpired choice, if available. 4. Finally, if all the above didn't give us a decision, and if the git system configuration is providing a rollout hint, present a prompt to user for their decision and save it in ~/.gitconfig. subcmds/sync.py: Make use of the new UseSuperproject() provided by git_superproject.py. While there also silent stderr from git describe when determining the version of repo. Bug: [google internal] b/190688390 Change-Id: Iad3ee03026342ee500e5d65e2f0fa600d7637613 Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/309762 Reviewed-by: Mike Frysinger Tested-by: Xin Li --- git_command.py | 3 +- git_config.py | 14 +++++++- git_superproject.py | 81 +++++++++++++++++++++++++++++++++++++++++++++ subcmds/sync.py | 11 ++---- 4 files changed, 98 insertions(+), 11 deletions(-) diff --git a/git_command.py b/git_command.py index 04953f38..95db91f2 100644 --- a/git_command.py +++ b/git_command.py @@ -75,7 +75,8 @@ def RepoSourceVersion(): proj = os.path.dirname(os.path.abspath(__file__)) env[GIT_DIR] = os.path.join(proj, '.git') result = subprocess.run([GIT, 'describe', HEAD], stdout=subprocess.PIPE, - encoding='utf-8', env=env, check=False) + stderr=subprocess.DEVNULL, encoding='utf-8', + env=env, check=False) if result.returncode == 0: ver = result.stdout.strip() if ver.startswith('v'): diff --git a/git_config.py b/git_config.py index 978f6a59..3eaf201c 100644 --- a/git_config.py +++ b/git_config.py @@ -65,6 +65,15 @@ class GitConfig(object): _USER_CONFIG = '~/.gitconfig' + _ForSystem = None + _SYSTEM_CONFIG = '/etc/gitconfig' + + @classmethod + def ForSystem(cls): + if cls._ForSystem is None: + cls._ForSystem = cls(configfile=cls._SYSTEM_CONFIG) + return cls._ForSystem + @classmethod def ForUser(cls): if cls._ForUser is None: @@ -356,7 +365,10 @@ class GitConfig(object): return c def _do(self, *args): - command = ['config', '--file', self.file, '--includes'] + if self.file == self._SYSTEM_CONFIG: + command = ['config', '--system', '--includes'] + else: + command = ['config', '--file', self.file, '--includes'] command.extend(args) p = GitCommand(None, diff --git a/git_superproject.py b/git_superproject.py index 0c477060..2a47847a 100644 --- a/git_superproject.py +++ b/git_superproject.py @@ -23,11 +23,14 @@ Examples: """ import hashlib +import functools import os import sys +import time from typing import NamedTuple from git_command import git_require, GitCommand +from git_config import RepoConfig from git_refs import R_HEADS from manifest_xml import LOCAL_MANIFEST_GROUP_PREFIX @@ -343,3 +346,81 @@ class Superproject(object): manifest_path = self._WriteManfiestFile() return UpdateProjectsResult(manifest_path, False) + + +@functools.lru_cache(maxsize=None) +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') + if user_value is not None: + user_expiration = user_cfg.GetInt('repo.superprojectChoiceExpire') + 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. + print(('You are currently enrolled in Git submodules experiment ' + '(go/android-submodules-quickstart). Use --no-use-superproject ' + 'to override.\n'), file=sys.stderr) + return user_value + + # We don't have an unexpired choice, ask for one. + system_value = system_cfg.GetBoolean('repo.superprojectChoice') + if system_value: + # The system configuration is proposing that we should enable the + # use of superproject. Present this to user for confirmation if we + # are on a TTY, or, when we are not on a TTY, accept the system + # default for this time only. + # + # TODO(b/190688390) - Remove prompt when we are comfortable with the new + # default value. + prompt = ('Repo can now use Git submodules (go/android-submodules-quickstart) ' + 'instead of manifests to represent the state of the Android ' + 'superproject, which results in faster syncs and better atomicity.\n\n') + if sys.stdout.isatty(): + prompt += 'Would you like to opt in for two weeks (y/N)? ' + response = input(prompt).lower() + time_choiceexpire = time_now + (86400 * 14) + if response in ('y', 'yes'): + userchoice = True + elif response in ('a', 'always'): + userchoice = True + time_choiceexpire = 0 + elif response == 'never': + userchoice = False + time_choiceexpire = 0 + elif response in ('n', 'no'): + userchoice = False + else: + # Unrecognized user response, assume the intention was no, but + # only for 2 hours instead of 2 weeks to balance between not + # being overly pushy while still retain the opportunity to + # enroll. + userchoice = False + time_choiceexpire = time_now + 7200 + + user_cfg.SetString('repo.superprojectChoiceExpire', str(time_choiceexpire)) + user_cfg.SetBoolean('repo.superprojectChoice', userchoice) + + return userchoice + else: + print('Accepting once since we are not on a TTY', file=sys.stderr) + return True + + # For all other cases, we would not use superproject by default. + return False + + +def UseSuperproject(opt, manifest): + """Returns a boolean if use-superproject option is enabled.""" + + if opt.use_superproject is not None: + return opt.use_superproject + else: + client_value = manifest.manifestProject.config.GetBoolean('repo.superproject') + if client_value is not None: + return client_value + else: + return _UseSuperprojectFromConfiguration() diff --git a/subcmds/sync.py b/subcmds/sync.py index 8d89cf72..8defc932 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -278,16 +278,9 @@ later is required to fix a server side protocol bug. branch = branch[len(R_HEADS):] return branch - def _UseSuperproject(self, opt): - """Returns True if use-superproject option is enabled""" - if opt.use_superproject is not None: - return opt.use_superproject - else: - return self.manifest.manifestProject.config.GetBoolean('repo.superproject') - def _GetCurrentBranchOnly(self, opt): """Returns True if current-branch or use-superproject options are enabled.""" - return opt.current_branch_only or self._UseSuperproject(opt) + return opt.current_branch_only or git_superproject.UseSuperproject(opt, self.manifest) def _UpdateProjectsRevisionId(self, opt, args, load_local_manifests): """Update revisionId of every project with the SHA from superproject. @@ -964,7 +957,7 @@ later is required to fix a server side protocol bug. self._UpdateManifestProject(opt, mp, manifest_name) load_local_manifests = not self.manifest.HasLocalManifests - if self._UseSuperproject(opt): + if git_superproject.UseSuperproject(opt, self.manifest): new_manifest_name = self._UpdateProjectsRevisionId(opt, args, load_local_manifests) if not new_manifest_name: manifest_name = new_manifest_name