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

Cleanup: Merge a bunch of redundent env['GIT_PAGER'] = 'cat' statements.

Browse Source 
Additionally:
- replace some RunCommand(['git', ...]) calls with RunGit().
- replace Settings._GetConfig('rietveld.foo') _GetRietveldConfig('foo').
- merge and cache calls to git rev-parse --show-cdup.
- merge some calls to git merge-base.

Review URL: https://codereview.chromium.org/157913005

git-svn-id: svn://svn.chromium.org/chrome/trunk/tools/depot_tools@250248 0039d316-1c4b-4281-b951-d872f2087c98
experimental/szager/collated-output
thestig@chromium.org 12 years ago
parent 2504f2ed7c
commit 8b0553c1f9
2 changed files with 58 additions and 61 deletions
Show Stats Download Patch File Download Diff File

100
git_cl.py
Unescape Escape View File

@ -63,6 +63,12 @@ def DieWithError(message):
sys.exit(1) sys.exit(1)
def GetNoGitPagerEnv():
env = os.environ.copy()
# 'cat' is a magical git string that disables pagers on all platforms.
env['GIT_PAGER'] = 'cat'
return env
def RunCommand(args, error_ok=False, error_message=None, **kwargs): def RunCommand(args, error_ok=False, error_message=None, **kwargs):
try: try:
return subprocess2.check_output(args, shell=False, **kwargs) return subprocess2.check_output(args, shell=False, **kwargs)
@ -83,15 +89,12 @@ def RunGit(args, **kwargs):
def RunGitWithCode(args, suppress_stderr=False): def RunGitWithCode(args, suppress_stderr=False):
"""Returns return code and stdout.""" """Returns return code and stdout."""
try: try:
env = os.environ.copy()
# 'cat' is a magical git string that disables pagers on all platforms.
env['GIT_PAGER'] = 'cat'
if suppress_stderr: if suppress_stderr:
stderr = subprocess2.VOID stderr = subprocess2.VOID
else: else:
stderr = sys.stderr stderr = sys.stderr
out, code = subprocess2.communicate(['git'] + args, out, code = subprocess2.communicate(['git'] + args,
env=env, env=GetNoGitPagerEnv(),
stdout=subprocess2.PIPE, stdout=subprocess2.PIPE,
stderr=stderr) stderr=stderr)
return code, out[0] return code, out[0]
@ -239,11 +242,9 @@ def print_stats(similarity, find_copies, args):
# --no-ext-diff is broken in some versions of Git, so try to work around # --no-ext-diff is broken in some versions of Git, so try to work around
# this by overriding the environment (but there is still a problem if the # this by overriding the environment (but there is still a problem if the
# git config key "diff.external" is used). # git config key "diff.external" is used).
env = os.environ.copy() env = GetNoGitPagerEnv()
if 'GIT_EXTERNAL_DIFF' in env: if 'GIT_EXTERNAL_DIFF' in env:
del env['GIT_EXTERNAL_DIFF'] del env['GIT_EXTERNAL_DIFF']
# 'cat' is a magical git string that disables pagers on all platforms.
env['GIT_PAGER'] = 'cat'
if find_copies: if find_copies:
similarity_options = ['--find-copies-harder', '-l100000', similarity_options = ['--find-copies-harder', '-l100000',
@ -261,7 +262,7 @@ class Settings(object):
def __init__(self): def __init__(self):
self.default_server = None self.default_server = None
self.cc = None self.cc = None
self.root = None self.relative_root = None
self.is_git_svn = None self.is_git_svn = None
self.svn_branch = None self.svn_branch = None
self.tree_status_url = None self.tree_status_url = None
@ -292,20 +293,23 @@ class Settings(object):
if not self.default_server: if not self.default_server:
self.LazyUpdateIfNeeded() self.LazyUpdateIfNeeded()
self.default_server = gclient_utils.UpgradeToHttps( self.default_server = gclient_utils.UpgradeToHttps(
self._GetConfig('rietveld.server', error_ok=True)) self._GetRietveldConfig('server', error_ok=True))
if error_ok: if error_ok:
return self.default_server return self.default_server
if not self.default_server: if not self.default_server:
error_message = ('Could not find settings file. You must configure ' error_message = ('Could not find settings file. You must configure '
'your review setup by running "git cl config".') 'your review setup by running "git cl config".')
self.default_server = gclient_utils.UpgradeToHttps( self.default_server = gclient_utils.UpgradeToHttps(
self._GetConfig('rietveld.server', error_message=error_message)) self._GetRietveldConfig('server', error_message=error_message))
return self.default_server return self.default_server
def GetRelativeRoot(self):
if self.relative_root is None:
self.relative_root = RunGit(['rev-parse', '--show-cdup']).strip()
return self.relative_root
def GetRoot(self): def GetRoot(self):
if not self.root: return os.path.abspath(self.GetRelativeRoot())
self.root = os.path.abspath(RunGit(['rev-parse', '--show-cdup']).strip())
return self.root
def GetIsGitSvn(self): def GetIsGitSvn(self):
"""Return true if this repo looks like it's using git-svn.""" """Return true if this repo looks like it's using git-svn."""
@ -328,15 +332,12 @@ class Settings(object):
# regexp matching the git-svn line that contains the URL. # regexp matching the git-svn line that contains the URL.
git_svn_re = re.compile(r'^\s*git-svn-id: (\S+)@', re.MULTILINE) git_svn_re = re.compile(r'^\s*git-svn-id: (\S+)@', re.MULTILINE)
env = os.environ.copy()
# 'cat' is a magical git string that disables pagers on all platforms.
env['GIT_PAGER'] = 'cat'
# We don't want to go through all of history, so read a line from the # We don't want to go through all of history, so read a line from the
# pipe at a time. # pipe at a time.
# The -100 is an arbitrary limit so we don't search forever. # The -100 is an arbitrary limit so we don't search forever.
cmd = ['git', 'log', '-100', '--pretty=medium'] cmd = ['git', 'log', '-100', '--pretty=medium']
proc = subprocess2.Popen(cmd, stdout=subprocess2.PIPE, env=env) proc = subprocess2.Popen(cmd, stdout=subprocess2.PIPE,
env=GetNoGitPagerEnv())
url = None url = None
for line in proc.stdout: for line in proc.stdout:
match = git_svn_re.match(line) match = git_svn_re.match(line)
@ -391,24 +392,23 @@ class Settings(object):
if not self.tree_status_url: if not self.tree_status_url:
error_message = ('You must configure your tree status URL by running ' error_message = ('You must configure your tree status URL by running '
'"git cl config".') '"git cl config".')
self.tree_status_url = self._GetConfig('rietveld.tree-status-url', self.tree_status_url = self._GetRietveldConfig(
error_ok=error_ok, 'tree-status-url', error_ok=error_ok, error_message=error_message)
error_message=error_message)
return self.tree_status_url return self.tree_status_url
def GetViewVCUrl(self): def GetViewVCUrl(self):
if not self.viewvc_url: if not self.viewvc_url:
self.viewvc_url = self._GetConfig('rietveld.viewvc-url', error_ok=True) self.viewvc_url = self._GetRietveldConfig('viewvc-url', error_ok=True)
return self.viewvc_url return self.viewvc_url
def GetBugPrefix(self): def GetBugPrefix(self):
return self._GetConfig('rietveld.bug-prefix', error_ok=True) return self._GetRietveldConfig('bug-prefix', error_ok=True)
def GetDefaultCCList(self): def GetDefaultCCList(self):
return self._GetConfig('rietveld.cc', error_ok=True) return self._GetRietveldConfig('cc', error_ok=True)
def GetDefaultPrivateFlag(self): def GetDefaultPrivateFlag(self):
return self._GetConfig('rietveld.private', error_ok=True) return self._GetRietveldConfig('private', error_ok=True)
def GetIsGerrit(self): def GetIsGerrit(self):
"""Return true if this repo is assosiated with gerrit code review system.""" """Return true if this repo is assosiated with gerrit code review system."""
@ -422,6 +422,9 @@ class Settings(object):
self.git_editor = self._GetConfig('core.editor', error_ok=True) self.git_editor = self._GetConfig('core.editor', error_ok=True)
return self.git_editor or None return self.git_editor or None
def _GetRietveldConfig(self, param, **kwargs):
return self._GetConfig('rietveld.' + param, **kwargs)
def _GetConfig(self, param, **kwargs): def _GetConfig(self, param, **kwargs):
self.LazyUpdateIfNeeded() self.LazyUpdateIfNeeded()
return RunGit(['config', param], **kwargs).strip() return RunGit(['config', param], **kwargs).strip()
@ -534,6 +537,9 @@ or verify this branch is set up to track another (via the --track argument to
return remote, upstream_branch return remote, upstream_branch
def GetCommonAncestorWithUpstream(self):
return RunGit(['merge-base', self.GetUpstreamBranch(), 'HEAD']).strip()
def GetUpstreamBranch(self): def GetUpstreamBranch(self):
if self.upstream_branch is None: if self.upstream_branch is None:
remote, upstream_branch = self.FetchUpstreamTuple(self.GetBranch()) remote, upstream_branch = self.FetchUpstreamTuple(self.GetBranch())
@ -735,17 +741,13 @@ or verify this branch is set up to track another (via the --track argument to
if not self.GitSanityChecks(upstream_branch): if not self.GitSanityChecks(upstream_branch):
DieWithError('\nGit sanity check failure') DieWithError('\nGit sanity check failure')
env = os.environ.copy() root = settings.GetRelativeRoot()
# 'cat' is a magical git string that disables pagers on all platforms.
env['GIT_PAGER'] = 'cat'
root = RunCommand(['git', 'rev-parse', '--show-cdup'], env=env).strip()
if not root: if not root:
root = '.' root = '.'
absroot = os.path.abspath(root) absroot = os.path.abspath(root)
# We use the sha1 of HEAD as a name of this change. # We use the sha1 of HEAD as a name of this change.
name = RunCommand(['git', 'rev-parse', 'HEAD'], env=env).strip() name = RunGitWithCode(['rev-parse', 'HEAD'])[1].strip()
# Need to pass a relative path for msysgit. # Need to pass a relative path for msysgit.
try: try:
files = scm.GIT.CaptureStatus([root], '.', upstream_branch) files = scm.GIT.CaptureStatus([root], '.', upstream_branch)
@ -766,10 +768,8 @@ or verify this branch is set up to track another (via the --track argument to
# If the change was never uploaded, use the log messages of all commits # If the change was never uploaded, use the log messages of all commits
# up to the branch point, as git cl upload will prefill the description # up to the branch point, as git cl upload will prefill the description
# with these log messages. # with these log messages.
description = RunCommand(['git', args = ['log', '--pretty=format:%s%n%n%b', '%s...' % (upstream_branch)]
'log', '--pretty=format:%s%n%n%b', description = RunGitWithCode(args)[1].strip()
'%s...' % (upstream_branch)],
env=env).strip()
if not author: if not author:
author = RunGit(['config', 'user.email']).strip() or None author = RunGit(['config', 'user.email']).strip() or None
@ -1012,7 +1012,7 @@ def FindCodereviewSettingsFile(filename='codereview.settings'):
""" """
inherit_ok_file = 'inherit-review-settings-ok' inherit_ok_file = 'inherit-review-settings-ok'
cwd = os.getcwd() cwd = os.getcwd()
root = os.path.abspath(RunGit(['rev-parse', '--show-cdup']).strip()) root = settings.GetRoot()
if os.path.isfile(os.path.join(root, inherit_ok_file)): if os.path.isfile(os.path.join(root, inherit_ok_file)):
root = '/' root = '/'
while True: while True:
@ -1382,7 +1382,7 @@ def CMDpresubmit(parser, args):
base_branch = args[0] base_branch = args[0]
else: else:
# Default to diffing against the common ancestor of the upstream branch. # Default to diffing against the common ancestor of the upstream branch.
base_branch = RunGit(['merge-base', cl.GetUpstreamBranch(), 'HEAD']).strip() base_branch = cl.GetCommonAncestorWithUpstream()
cl.RunHook( cl.RunHook(
committing=not options.upload, committing=not options.upload,
@ -1618,7 +1618,7 @@ def CMDupload(parser, args):
base_branch = args[0] base_branch = args[0]
else: else:
# Default to diffing against common ancestor of upstream branch # Default to diffing against common ancestor of upstream branch
base_branch = RunGit(['merge-base', cl.GetUpstreamBranch(), 'HEAD']).strip() base_branch = cl.GetCommonAncestorWithUpstream()
args = [base_branch, 'HEAD'] args = [base_branch, 'HEAD']
# Apply watchlists on upload. # Apply watchlists on upload.
@ -1815,7 +1815,7 @@ def SendUpstream(parser, args, cmd):
# We might be in a directory that's present in this branch but not in the # We might be in a directory that's present in this branch but not in the
# trunk. Move up to the top of the tree so that git commands that expect a # trunk. Move up to the top of the tree so that git commands that expect a
# valid CWD won't fail after we check out the merge branch. # valid CWD won't fail after we check out the merge branch.
rel_base_path = RunGit(['rev-parse', '--show-cdup']).strip() rel_base_path = settings.GetRelativeRoot()
if rel_base_path: if rel_base_path:
os.chdir(rel_base_path) os.chdir(rel_base_path)
@ -1978,7 +1978,7 @@ def PatchIssue(issue_arg, reject, nocommit, directory):
# Switch up to the top-level directory, if necessary, in preparation for # Switch up to the top-level directory, if necessary, in preparation for
# applying the patch. # applying the patch.
top = RunGit(['rev-parse', '--show-cdup']).strip() top = settings.GetRelativeRoot()
if top: if top:
os.chdir(top) os.chdir(top)
@ -1992,9 +1992,6 @@ def PatchIssue(issue_arg, reject, nocommit, directory):
except subprocess2.CalledProcessError: except subprocess2.CalledProcessError:
DieWithError('Git patch mungling failed.') DieWithError('Git patch mungling failed.')
logging.info(patch_data) logging.info(patch_data)
env = os.environ.copy()
# 'cat' is a magical git string that disables pagers on all platforms.
env['GIT_PAGER'] = 'cat'
# We use "git apply" to apply the patch instead of "patch" so that we can # We use "git apply" to apply the patch instead of "patch" so that we can
# pick up file adds. # pick up file adds.
@ -2007,7 +2004,7 @@ def PatchIssue(issue_arg, reject, nocommit, directory):
elif IsGitVersionAtLeast('1.7.12'): elif IsGitVersionAtLeast('1.7.12'):
cmd.append('--3way') cmd.append('--3way')
try: try:
subprocess2.check_call(cmd, env=env, subprocess2.check_call(cmd, env=GetNoGitPagerEnv(),
stdin=patch_data, stdout=subprocess2.VOID) stdin=patch_data, stdout=subprocess2.VOID)
except subprocess2.CalledProcessError: except subprocess2.CalledProcessError:
DieWithError('Failed to apply the patch') DieWithError('Failed to apply the patch')
@ -2030,11 +2027,8 @@ def CMDrebase(parser, args):
# git svn dcommit. # git svn dcommit.
# It's the only command that doesn't use parser at all since we just defer # It's the only command that doesn't use parser at all since we just defer
# execution to git-svn. # execution to git-svn.
env = os.environ.copy()
# 'cat' is a magical git string that disables pagers on all platforms.
env['GIT_PAGER'] = 'cat'
return subprocess2.call(['git', 'svn', 'rebase'] + args, env=env) return RunGitWithCode(['svn', 'rebase'] + args)[1]
def GetTreeStatus(url=None): def GetTreeStatus(url=None):
@ -2125,9 +2119,7 @@ def CMDtry(parser, args):
# Process --bot and --testfilter. # Process --bot and --testfilter.
if not options.bot: if not options.bot:
# Get try slaves from PRESUBMIT.py files if not specified. # Get try slaves from PRESUBMIT.py files if not specified.
change = cl.GetChange( change = cl.GetChange(cl.GetCommonAncestorWithUpstream(), None)
RunGit(['merge-base', cl.GetUpstreamBranch(), 'HEAD']).strip(),
None)
options.bot = presubmit_support.DoGetTrySlaves( options.bot = presubmit_support.DoGetTrySlaves(
change, change,
change.LocalPaths(), change.LocalPaths(),
@ -2254,7 +2246,7 @@ def CMDdiff(parser, args):
if not issue: if not issue:
DieWithError('No issue found for current branch (%s)' % branch) DieWithError('No issue found for current branch (%s)' % branch)
TMP_BRANCH = 'git-cl-diff' TMP_BRANCH = 'git-cl-diff'
base_branch = RunGit(['merge-base', cl.GetUpstreamBranch(), 'HEAD']).strip() base_branch = cl.GetCommonAncestorWithUpstream()
# Create a new branch based on the merge-base # Create a new branch based on the merge-base
RunGit(['checkout', '-q', '-b', TMP_BRANCH, base_branch]) RunGit(['checkout', '-q', '-b', TMP_BRANCH, base_branch])
@ -2292,7 +2284,7 @@ def CMDowners(parser, args):
base_branch = args[0] base_branch = args[0]
else: else:
# Default to diffing against the common ancestor of the upstream branch. # Default to diffing against the common ancestor of the upstream branch.
base_branch = RunGit(['merge-base', cl.GetUpstreamBranch(), 'HEAD']).strip() base_branch = cl.GetCommonAncestorWithUpstream()
change = cl.GetChange(base_branch, None) change = cl.GetChange(base_branch, None)
return owners_finder.OwnersFinder( return owners_finder.OwnersFinder(
@ -2315,7 +2307,7 @@ def CMDformat(parser, args):
# git diff generates paths against the root of the repository. Change # git diff generates paths against the root of the repository. Change
# to that directory so clang-format can find files even within subdirs. # to that directory so clang-format can find files even within subdirs.
rel_base_path = RunGit(['rev-parse', '--show-cdup']).strip() rel_base_path = settings.GetRelativeRoot()
if rel_base_path: if rel_base_path:
os.chdir(rel_base_path) os.chdir(rel_base_path)

19
tests/git_cl_test.py
Unescape Escape View File

@ -204,7 +204,6 @@ class TestGitCl(TestCase):
((['git', ((['git',
'config', '--local', '--get-regexp', '^svn-remote\\.'],), 'config', '--local', '--get-regexp', '^svn-remote\\.'],),
(('', None), 0)), (('', None), 0)),
((['git', 'rev-parse', '--show-cdup'],), ''),
((['git', 'svn', 'info'],), ''), ((['git', 'svn', 'info'],), ''),
((['git', ((['git',
'config', 'branch.master.rietveldissue', '1'],), ''), 'config', 'branch.master.rietveldissue', '1'],), ''),
@ -319,8 +318,8 @@ class TestGitCl(TestCase):
] ]
@classmethod @classmethod
def _dcommit_calls_3(cls): def _dcommit_calls_3(cls, is_first_call):
return [ calls = [
((['git', ((['git',
'diff', '--no-ext-diff', '--stat', '--find-copies-harder', 'diff', '--no-ext-diff', '--stat', '--find-copies-harder',
'-l100000', '-C50', 'fake_ancestor_sha', '-l100000', '-C50', 'fake_ancestor_sha',
@ -334,7 +333,12 @@ class TestGitCl(TestCase):
((['git', 'branch', '-D', 'git-cl-commit'],), ''), ((['git', 'branch', '-D', 'git-cl-commit'],), ''),
((['git', 'show-ref', '--quiet', '--verify', ((['git', 'show-ref', '--quiet', '--verify',
'refs/heads/git-cl-cherry-pick'],), ''), 'refs/heads/git-cl-cherry-pick'],), ''),
((['git', 'rev-parse', '--show-cdup'],), '\n'), ]
if is_first_call:
calls += [
((['git', 'rev-parse', '--show-cdup'],), '\n'),
]
calls += [
((['git', 'checkout', '-q', '-b', 'git-cl-commit'],), ''), ((['git', 'checkout', '-q', '-b', 'git-cl-commit'],), ''),
((['git', 'reset', '--soft', 'fake_ancestor_sha'],), ''), ((['git', 'reset', '--soft', 'fake_ancestor_sha'],), ''),
((['git', 'commit', '-m', ((['git', 'commit', '-m',
@ -346,7 +350,8 @@ class TestGitCl(TestCase):
(('', None), 0)), (('', None), 0)),
((['git', 'checkout', '-q', 'working'],), ''), ((['git', 'checkout', '-q', 'working'],), ''),
((['git', 'branch', '-D', 'git-cl-commit'],), ''), ((['git', 'branch', '-D', 'git-cl-commit'],), ''),
] ]
return calls
@staticmethod @staticmethod
def _cmd_line(description, args, similarity, find_copies, private): def _cmd_line(description, args, similarity, find_copies, private):
@ -509,14 +514,14 @@ class TestGitCl(TestCase):
self._dcommit_calls_1() + self._dcommit_calls_1() +
self._git_sanity_checks('fake_ancestor_sha', 'working') + self._git_sanity_checks('fake_ancestor_sha', 'working') +
self._dcommit_calls_normal() + self._dcommit_calls_normal() +
self._dcommit_calls_3()) self._dcommit_calls_3(False))
git_cl.main(['dcommit']) git_cl.main(['dcommit'])
def test_dcommit_bypass_hooks(self): def test_dcommit_bypass_hooks(self):
self.calls = ( self.calls = (
self._dcommit_calls_1() + self._dcommit_calls_1() +
self._dcommit_calls_bypassed() + self._dcommit_calls_bypassed() +
self._dcommit_calls_3()) self._dcommit_calls_3(True))
git_cl.main(['dcommit', '--bypass-hooks']) git_cl.main(['dcommit', '--bypass-hooks'])

Write Preview
Loading…
Cancel
Save