aiden
/
depot_tools
mirror of https://chromium.googlesource.com/chromium/tools/depot_tools
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[git_cl] Remove uses of RunGit which interact with git config.

Browse Source 
The previous interactions were incorrect w.r.t. scm.GIT's config
cache.

This moves depot_tools towards having one fewer git wrapper
(converging towards scm.GIT).

R=ayatane, yiwzhang@google.com

Change-Id: I507628b53f6a87a1eecbbe3e1e27c1eb6af3b878
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5648617
Auto-Submit: Robbie Iannucci <iannucci@chromium.org>
Reviewed-by: Yiwei Zhang <yiwzhang@google.com>
Commit-Queue: Yiwei Zhang <yiwzhang@google.com>
changes/17/5648617/6
Robert Iannucci 1 year ago committed by LUCI CQ
parent 1a616deaac
commit a6502426b5
3 changed files with 90 additions and 112 deletions
Show Stats Download Patch File Download Diff File

108
git_cl.py
Unescape Escape View File

@ -925,24 +925,24 @@ class Settings(object):
def GetFormatFullByDefault(self):
if self.format_full_by_default is None:
self._LazyUpdateIfNeeded()
result = (RunGit(
['config', '--bool', 'rietveld.format-full-by-default'],
error_ok=True).strip())
self.format_full_by_default = (result == 'true')
self.format_full_by_default = self._GetConfigBool(
'rietveld.format-full-by-default')
return self.format_full_by_default
def IsStatusCommitOrderByDate(self):
if self.is_status_commit_order_by_date is None:
result = (RunGit(['config', '--bool', 'cl.date-order'],
error_ok=True).strip())
self.is_status_commit_order_by_date = (result == 'true')
self.is_status_commit_order_by_date = self._GetConfigBool(
'cl.date-order')
return self.is_status_commit_order_by_date
def _GetConfig(self, key, default=''):
self._LazyUpdateIfNeeded()
return scm.GIT.GetConfig(self.GetRoot(), key, default)
def _GetConfigBool(self, key) -> bool:
self._LazyUpdateIfNeeded()
return scm.GIT.GetConfigBool(self.GetRoot(), key)
settings = Settings()
@ -2933,7 +2933,8 @@ class Changelist(object):
shutil.make_archive(traces_zip, 'zip', traces_dir)
# Collect and compress the git config and gitcookies.
git_config = RunGit(['config', '-l'])
git_config = '\n'.join(
f'{k}={v}' for k, v in scm.GIT.YieldConfigRegexp(settings.GetRoot()))
gclient_utils.FileWrite(os.path.join(git_info_dir, 'git-config'),
git_config)
@ -3575,54 +3576,45 @@ def FindCodereviewSettingsFile(filename='codereview.settings'):
def LoadCodereviewSettingsFromFile(fileobj):
"""Parses a codereview.settings file and updates hooks."""
keyvals = gclient_utils.ParseCodereviewSettingsContent(fileobj.read())
root = settings.GetRoot()
def SetProperty(name, setting, unset_error_ok=False):
def SetProperty(name, setting):
fullname = 'rietveld.' + name
if setting in keyvals:
RunGit(['config', fullname, keyvals[setting]])
scm.GIT.SetConfig(root, fullname, keyvals[setting])
else:
RunGit(['config', '--unset-all', fullname], error_ok=unset_error_ok)
scm.GIT.SetConfig(root, fullname, None, modify_all=True)
if not keyvals.get('GERRIT_HOST', False):
SetProperty('server', 'CODE_REVIEW_SERVER')
# Only server setting is required. Other settings can be absent.
# In that case, we ignore errors raised during option deletion attempt.
SetProperty('cc', 'CC_LIST', unset_error_ok=True)
SetProperty('tree-status-url', 'STATUS', unset_error_ok=True)
SetProperty('viewvc-url', 'VIEW_VC', unset_error_ok=True)
SetProperty('bug-prefix', 'BUG_PREFIX', unset_error_ok=True)
SetProperty('cpplint-regex', 'LINT_REGEX', unset_error_ok=True)
SetProperty('cpplint-ignore-regex',
'LINT_IGNORE_REGEX',
unset_error_ok=True)
SetProperty('run-post-upload-hook',
'RUN_POST_UPLOAD_HOOK',
unset_error_ok=True)
SetProperty('format-full-by-default',
'FORMAT_FULL_BY_DEFAULT',
unset_error_ok=True)
SetProperty('cc', 'CC_LIST')
SetProperty('tree-status-url', 'STATUS')
SetProperty('viewvc-url', 'VIEW_VC')
SetProperty('bug-prefix', 'BUG_PREFIX')
SetProperty('cpplint-regex', 'LINT_REGEX')
SetProperty('cpplint-ignore-regex', 'LINT_IGNORE_REGEX')
SetProperty('run-post-upload-hook', 'RUN_POST_UPLOAD_HOOK')
SetProperty('format-full-by-default', 'FORMAT_FULL_BY_DEFAULT')
if 'GERRIT_HOST' in keyvals:
RunGit(['config', 'gerrit.host', keyvals['GERRIT_HOST']])
scm.GIT.SetConfig(root, 'gerrit.host', keyvals['GERRIT_HOST'])
if 'GERRIT_SQUASH_UPLOADS' in keyvals:
RunGit([
'config', 'gerrit.squash-uploads', keyvals['GERRIT_SQUASH_UPLOADS']
])
scm.GIT.SetConfig(root, 'gerrit.squash-uploads',
keyvals['GERRIT_SQUASH_UPLOADS'])
if 'GERRIT_SKIP_ENSURE_AUTHENTICATED' in keyvals:
RunGit([
'config', 'gerrit.skip-ensure-authenticated',
keyvals['GERRIT_SKIP_ENSURE_AUTHENTICATED']
])
scm.GIT.SetConfig('gerrit.skip-ensure-authenticated',
keyvals['GERRIT_SKIP_ENSURE_AUTHENTICATED'])
if 'PUSH_URL_CONFIG' in keyvals and 'ORIGIN_URL_CONFIG' in keyvals:
# should be of the form
# PUSH_URL_CONFIG: url.ssh://gitrw.chromium.org.pushinsteadof
# ORIGIN_URL_CONFIG: http://src.chromium.org/git
RunGit([
'config', keyvals['PUSH_URL_CONFIG'], keyvals['ORIGIN_URL_CONFIG']
])
scm.GIT.SetConfig(keyvals['PUSH_URL_CONFIG'],
keyvals['ORIGIN_URL_CONFIG'])
def urlretrieve(source, destination):
@ -3890,8 +3882,8 @@ class _GitCookiesChecker(object):
"""Runs checks and suggests fixes to make git use .gitcookies from default
path."""
default = gerrit_util.CookiesAuthenticator.get_gitcookies_path()
configured_path = RunGitSilent(
['config', '--global', 'http.cookiefile']).strip()
configured_path = scm.GIT.GetConfig(settings.GetRoot(),
'http.cookiefile', '')
configured_path = os.path.expanduser(configured_path)
if configured_path:
self._ensure_default_gitcookies_path(configured_path, default)
@ -3914,7 +3906,10 @@ class _GitCookiesChecker(object):
print('However, your configured .gitcookies file is missing.')
confirm_or_exit('Reconfigure git to use default .gitcookies?',
action='reconfigure')
RunGit(['config', '--global', 'http.cookiefile', default_path])
scm.GIT.SetConfig(settings.GetRoot(),
'http.cookiefile',
default_path,
scope='global')
return
if os.path.exists(default_path):
@ -3927,7 +3922,10 @@ class _GitCookiesChecker(object):
confirm_or_exit('Move existing .gitcookies to default location?',
action='move')
shutil.move(configured_path, default_path)
RunGit(['config', '--global', 'http.cookiefile', default_path])
scm.GIT.SetConfig(settings.GetRoot(),
'http.cookiefile',
default_path,
scope='global')
print('Moved and reconfigured git to use .gitcookies from %s' %
default_path)
@ -3941,7 +3939,10 @@ class _GitCookiesChecker(object):
' git config --global --unset http.cookiefile\n'
' mv %s %s.backup\n\n' % (default_path, default_path))
confirm_or_exit(action='setup .gitcookies')
RunGit(['config', '--global', 'http.cookiefile', default_path])
scm.GIT.SetConfig(settings.GetRoot(),
'http.cookiefile',
default_path,
scope='global')
print('Configured git to use .gitcookies from %s' % default_path)
def get_hosts_with_creds(self):
@ -4143,12 +4144,14 @@ def CMDbaseurl(parser, args):
branch = scm.GIT.ShortBranchName(branchref)
if not args:
print('Current base-url:')
return RunGit(['config', 'branch.%s.base-url' % branch],
error_ok=False).strip()
url = scm.GIT.GetConfig(settings.GetRoot(), f'branch.{branch}.base-url')
if url is None:
raise ValueError('Missing base URL.')
return url
print('Setting base-url to %s' % args[0])
return RunGit(['config', 'branch.%s.base-url' % branch, args[0]],
error_ok=False).strip()
scm.GIT.SetConfig(settings.GetRoot(), f'branch.{branch}.base-url', args[0])
return 0
def color_for_status(status):
@ -4674,9 +4677,8 @@ def CMDissue(parser, args):
issue_branch_map = {}
git_config = {}
for config in RunGit(['config', '--get-regexp',
r'branch\..*issue']).splitlines():
name, _space, val = config.partition(' ')
for name, val in scm.GIT.YieldConfigRegexp(
settings.GetRoot(), re.compile(r'branch\..*issue')):
git_config[name] = val
for branch in branches:
@ -7047,13 +7049,11 @@ def CMDcheckout(parser, args):
target_issue = str(issue_arg.issue)
output = RunGit([
'config', '--local', '--get-regexp', r'branch\..*\.' + ISSUE_CONFIG_KEY
],
error_ok=True)
output = scm.GIT.YieldConfigRegexp(
settings.GetRoot(), re.compile(r'branch\..*\.' + ISSUE_CONFIG_KEY))
branches = []
for key, issue in [x.split() for x in output.splitlines()]:
for key, issue in output:
if issue == target_issue:
branches.append(
re.sub(r'branch\.(.*)\.' + ISSUE_CONFIG_KEY, r'\1', key))

20
scm.py
Unescape Escape View File

@ -172,14 +172,19 @@ class CachedGitConfigState(object):
"""Returns all values of `key` as a list of strings."""
return self._maybe_load_config().get(key, [])
def YieldConfigRegexp(self, pattern: str) -> Iterable[Tuple[str, str]]:
def YieldConfigRegexp(self, pattern: Optional[str]) -> Iterable[Tuple[str, str]]:
"""Yields (key, value) pairs for any config keys matching `pattern`.
This use re.match, so `pattern` needs to be for the entire config key.
If pattern is None, this returns all config items.
"""
p = re.compile(pattern)
if pattern is None:
pred = lambda _: True
else:
pred = re.compile(pattern).match
for name, values in sorted(self._maybe_load_config().items()):
if p.match(name):
if pred(name):
for value in values:
yield name, value
@ -515,8 +520,13 @@ class GIT(object):
return GIT._get_config_state(cwd).GetConfigList(key)
@staticmethod
def YieldConfigRegexp(cwd: str, pattern: str) -> Iterable[Tuple[str, str]]:
"""Yields (key, value) pairs for any config keys matching `pattern`."""
def YieldConfigRegexp(cwd: str, pattern: Optional[str] = None) -> Iterable[Tuple[str, str]]:
"""Yields (key, value) pairs for any config keys matching `pattern`.
This use re.match, so `pattern` needs to be for the entire config key.
If pattern is None, this returns all config items.
"""
yield from GIT._get_config_state(cwd).YieldConfigRegexp(pattern)
@staticmethod

74
tests/git_cl_test.py
Unescape Escape View File

@ -583,6 +583,12 @@ class TestGitCl(unittest.TestCase):
super(TestGitCl, self).setUp()
self.calls = []
self._calls_done = []
oldEnv = dict(os.environ)
def _resetEnv():
os.environ = oldEnv
self.addCleanup(_resetEnv)
self.failed = False
mock.patch('sys.stdout', io.StringIO()).start()
mock.patch('git_cl.time_time',
@ -709,24 +715,6 @@ class TestGitCl(unittest.TestCase):
def test_LoadCodereviewSettingsFromFile_gerrit(self):
codereview_file = io.StringIO('GERRIT_HOST: true')
self.calls = [
((['git', 'config', '--unset-all', 'rietveld.cc'], ), CERR1),
((['git', 'config', '--unset-all',
'rietveld.tree-status-url'], ), CERR1),
((['git', 'config', '--unset-all',
'rietveld.viewvc-url'], ), CERR1),
((['git', 'config', '--unset-all',
'rietveld.bug-prefix'], ), CERR1),
((['git', 'config', '--unset-all',
'rietveld.cpplint-regex'], ), CERR1),
((['git', 'config', '--unset-all',
'rietveld.cpplint-ignore-regex'], ), CERR1),
((['git', 'config', '--unset-all',
'rietveld.run-post-upload-hook'], ), CERR1),
(([
'git', 'config', '--unset-all',
'rietveld.format-full-by-default'
], ), CERR1),
((['git', 'config', 'gerrit.host', 'true'], ), ''),
]
self.assertIsNone(
git_cl.LoadCodereviewSettingsFromFile(codereview_file))
@ -1063,14 +1051,13 @@ class TestGitCl(unittest.TestCase):
None,
),
# Collect git config and gitcookies.
(
(['git', 'config', '-l'], ),
'git-config-output',
),
#
# We accept ANY for the git-config file because it's just reflecting
# our mocked git config in scm.GIT anyway.
(
([
'FileWrite',
os.path.join('TEMP_DIR', 'git-config'), 'git-config-output'
os.path.join('TEMP_DIR', 'git-config'), mock.ANY,
], ),
None,
),
@ -1223,7 +1210,6 @@ class TestGitCl(unittest.TestCase):
f'https://{short_hostname}.googlesource.com/my/repo')
scm.GIT.SetConfig('', 'user.email', 'owner@example.com')
self.calls = []
if squash_mode == "override_nosquash":
if issue:
mock.patch('gerrit_util.GetChange',
@ -2485,34 +2471,23 @@ class TestGitCl(unittest.TestCase):
'change 123456 at https://chromium-review.googlesource.com does not '
'exist or you have no access to it\n', sys.stderr.getvalue())
def _checkout_calls(self):
return [
(([
'git', 'config', '--local', '--get-regexp',
'branch\\..*\\.gerritissue'
], ), ('branch.ger-branch.gerritissue 123456\n'
'branch.gbranch654.gerritissue 654321\n')),
]
def _checkout_config(self):
scm.GIT.SetConfig('', 'branch.ger-branch.gerritissue', '123456')
scm.GIT.SetConfig('', 'branch.gbranch654.gerritissue', '654321')
def test_checkout_gerrit(self):
"""Tests git cl checkout <issue>."""
self.calls = self._checkout_calls()
self._checkout_config()
self.calls += [((['git', 'checkout', 'ger-branch'], ), '')]
self.assertEqual(0, git_cl.main(['checkout', '123456']))
def test_checkout_not_found(self):
"""Tests git cl checkout <issue>."""
self.calls = self._checkout_calls()
self._checkout_config()
self.assertEqual(1, git_cl.main(['checkout', '99999']))
def test_checkout_no_branch_issues(self):
"""Tests git cl checkout <issue>."""
self.calls = [
(([
'git', 'config', '--local', '--get-regexp',
'branch\\..*\\.gerritissue'
], ), CERR1),
]
self.assertEqual(1, git_cl.main(['checkout', '99999']))
def _test_gerrit_ensure_authenticated_common(self, auth):
@ -3157,13 +3132,8 @@ class TestGitCl(unittest.TestCase):
mock.patch('git_cl._GitCookiesChecker.get_hosts_with_creds',
lambda _: []).start()
self.calls = [
((['git', 'config', '--global', 'http.cookiefile'], ), CERR1),
(('ask_for_data', 'Press Enter to setup .gitcookies, '
'or Ctrl+C to abort'), ''),
(([
'git', 'config', '--global', 'http.cookiefile',
os.path.expanduser(os.path.join('~', '.gitcookies'))
], ), ''),
]
self.assertEqual(0, git_cl.main(['creds-check']))
self.assertIn('\nConfigured git to use .gitcookies from',
@ -3171,20 +3141,18 @@ class TestGitCl(unittest.TestCase):
def test_creds_check_gitcookies_configured_custom_broken(self):
self._common_creds_check_mocks()
mock.patch('git_cl._GitCookiesChecker.get_hosts_with_creds',
lambda _: []).start()
custom_cookie_path = ('C:\\.gitcookies' if sys.platform == 'win32' else
'/custom/.gitcookies')
scm.GIT.SetConfig('', 'http.cookiefile', custom_cookie_path)
os.environ['GIT_COOKIES_PATH'] = '/official/.gitcookies'
mock.patch('git_cl._GitCookiesChecker.get_hosts_with_creds',
lambda _: []).start()
self.calls = [
((['git', 'config', '--global',
'http.cookiefile'], ), custom_cookie_path),
(('os.path.exists', custom_cookie_path), False),
(('ask_for_data', 'Reconfigure git to use default .gitcookies? '
'Press Enter to reconfigure, or Ctrl+C to abort'), ''),
(([
'git', 'config', '--global', 'http.cookiefile',
os.path.expanduser(os.path.join('~', '.gitcookies'))
], ), ''),
]
self.assertEqual(0, git_cl.main(['creds-check']))
self.assertIn(

Write Preview
Loading…
Cancel
Save