Skip to content

C
Chromium_Depot_tools
Commit e7f0b4c6 authored by Michael Moss's avatar Michael Moss Committed by Commit Bot
Browse files
Options

Revert "git-cl: Keep git push traces" 

This reverts commit dc8e23d3.

Reason for revert: Breaking chrome releases crbug.com/960638

Original change's description:
> git-cl: Keep git push traces
> 
> Keep the last N git push traces.
> Name them after the time when they were collected, and add a
> README file to each one to provide some context to developers.
> 
> Bug: 955206
> Change-Id: Ib5fcf2f78fb65f6ddd80a93619c14e1ef70c5564
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1595108
> Commit-Queue: Edward Lesmes <ehmaldonado@chromium.org>
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>

TBR=dpranke@chromium.org,ehmaldonado@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 955206
Change-Id: Id56dfb1b32a54070d761ef75bc26bbb9081f86fc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/1600535Reviewed-by: 's avatarMichael Moss <mmoss@chromium.org>
Commit-Queue: Michael Moss <mmoss@chromium.org>
parent e5e44404
Hide whitespace changes
Inline Side-by-side
Showing with 58 additions and 227 deletions
git_cl.py
View file @ e7f0b4c6
......@@ -79,30 +79,12 @@ GIT_HASH_RE = re.compile(r'\b([a-f0-9]{6})[a-f0-9]{34}\b', flags=re.I)
# Used to redact the cookies from the gitcookies file.
GITCOOKIES_REDACT_RE = re.compile(r'1/.*')
# The maximum number of traces we will keep. Multiplied by 3 since we store
# 3 files per trace.
MAX_TRACES = 3 * 10
# Message to display to the user after git-cl has run, to inform them of the
# traces we just collected.
TRACES_MESSAGE = (
'\n'
'When filing a bug for this push, be sure to include the traces found at:\n'
' %(trace_name)s-traces.zip\n'
'Consider including the git config and gitcookies, which we have packed for \n'
'you at:\n'
' %(trace_name)s-git-info.zip\n')
# Format of the message to be stored as part of the traces to give developers a
# better context when they go through traces.
TRACES_README_FORMAT = (
'Date: %(now)s\n'
'\n'
'Change: https://%(gerrit_host)s/q/%(change_id)s\n'
'Title: %(title)s\n'
'\n'
'%(description)s\n'
'\n'
'Execution time: %(execution_time)s\n'
'Exit code: %(exit_code)s\n') + TRACES_MESSAGE
'When filing a bug, be sure to include the traces found at:\n'
' %s.zip\n'
'Consider including the git config and gitcookies,\n'
'which we have packed for you at:\n'
' %s.zip\n')
COMMIT_BOT_EMAIL = 'commit-bot@chromium.org'
POSTUPSTREAM_HOOK = '.git/hooks/post-cl-land'
......@@ -223,12 +205,6 @@ def time_time():
return time.time()
def datetime_now():
# Use this so that it can be mocked in tests without interfering with python
# system machinery.
return datetime.datetime.now()
def ask_for_data(prompt):
try:
return raw_input(prompt)
......@@ -2520,73 +2496,18 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
else:
print('OK, will keep Gerrit commit-msg hook in place.')
def _CleanUpOldTraces(self):
"""Keep only the last |MAX_TRACES| traces."""
try:
traces = sorted([
os.path.join(TRACES_DIR, f)
for f in os.listdir(TRACES_DIR)
if (os.path.isfile(os.path.join(TRACES_DIR, f))
and not f.startswith('tmp'))
])
traces_to_delete = traces[:-MAX_TRACES]
for trace in traces_to_delete:
os.remove(trace)
except OSError:
print('WARNING: Failed to remove old git traces from\n'
' %s'
'Consider removing them manually.' % TRACES_DIR)
def _WriteGitPushTraces(self, traces_dir, git_push_metadata):
"""Zip and write the git push traces stored in traces_dir."""
def _RunGitPushWithTraces(self, change_desc, refspec, refspec_opts):
gclient_utils.safe_makedirs(TRACES_DIR)
now = datetime_now()
trace_name = os.path.join(TRACES_DIR, now.strftime('%Y%m%dT%H%M%S.%f'))
traces_zip = trace_name + '-traces'
traces_readme = trace_name + '-README'
# Create a temporary dir to store git config and gitcookies in. It will be
# compressed and stored next to the traces.
git_info_dir = tempfile.mkdtemp()
git_info_zip = trace_name + '-git-info'
git_push_metadata['now'] = now.strftime('%c')
git_push_metadata['trace_name'] = trace_name
gclient_utils.FileWrite(
traces_readme, TRACES_README_FORMAT % git_push_metadata)
# Keep only the first 6 characters of the git hashes on the packet
# trace. This greatly decreases size after compression.
packet_traces = os.path.join(traces_dir, 'trace-packet')
contents = gclient_utils.FileRead(packet_traces)
gclient_utils.FileWrite(
packet_traces, GIT_HASH_RE.sub(r'\1', contents))
shutil.make_archive(traces_zip, 'zip', traces_dir)
# Collect and compress the git config and gitcookies.
git_config = RunGit(['config', '-l'])
gclient_utils.FileWrite(
os.path.join(git_info_dir, 'git-config'),
git_config)
cookie_auth = gerrit_util.Authenticator.get()
if isinstance(cookie_auth, gerrit_util.CookiesAuthenticator):
gitcookies_path = cookie_auth.get_gitcookies_path()
gitcookies = gclient_utils.FileRead(gitcookies_path)
gclient_utils.FileWrite(
os.path.join(git_info_dir, 'gitcookies'),
GITCOOKIES_REDACT_RE.sub('REDACTED', gitcookies))
shutil.make_archive(git_info_zip, 'zip', git_info_dir)
print(TRACES_MESSAGE % {'trace_name': trace_name})
gclient_utils.rmtree(git_info_dir)
def _RunGitPushWithTraces(
self, change_desc, refspec, refspec_opts, git_push_metadata):
"""Run git push and collect the traces resulting from the execution."""
# Create a temporary directory to store traces in. Traces will be compressed
# and stored in a 'traces' dir inside depot_tools.
traces_dir = tempfile.mkdtemp()
trace_name = os.path.basename(traces_dir)
traces_zip = os.path.join(TRACES_DIR, trace_name + '-traces')
# Create a temporary dir to store git config and gitcookies in. It will be
# compressed and stored next to the traces.
git_info_dir = tempfile.mkdtemp()
git_info_zip = os.path.join(TRACES_DIR, trace_name + '-git-info')
env = os.environ.copy()
env['GIT_REDACT_COOKIES'] = 'o,SSO,GSSO_Uberproxy'
......@@ -2597,10 +2518,9 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
try:
push_returncode = 0
remote_url = self.GetRemoteUrl()
before_push = time_time()
push_stdout = gclient_utils.CheckCallAndFilter(
['git', 'push', remote_url, refspec],
['git', 'push', self.GetRemoteUrl(), refspec],
env=env,
print_stdout=True,
# Flush after every line: useful for seeing progress when running as
......@@ -2612,7 +2532,8 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
'for the reason of the failure.\n'
'Hint: run command below to diagnose common Git/Gerrit '
'credential problems:\n'
' git cl creds-check',
' git cl creds-check\n' +
TRACES_MESSAGE % (traces_zip, git_info_zip),
change_desc)
finally:
execution_time = time_time() - before_push
......@@ -2623,11 +2544,31 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
'arguments': metrics_utils.extract_known_subcommand_args(refspec_opts),
})
git_push_metadata['execution_time'] = execution_time
git_push_metadata['exit_code'] = push_returncode
self._WriteGitPushTraces(traces_dir, git_push_metadata)
self._CleanUpOldTraces()
if push_returncode != 0:
# Keep only the first 6 characters of the git hashes on the packet
# trace. This greatly decreases size after compression.
packet_traces = os.path.join(traces_dir, 'trace-packet')
contents = gclient_utils.FileRead(packet_traces)
gclient_utils.FileWrite(
packet_traces, GIT_HASH_RE.sub(r'\1', contents))
shutil.make_archive(traces_zip, 'zip', traces_dir)
# Collect and compress the git config and gitcookies.
git_config = RunGit(['config', '-l'])
gclient_utils.FileWrite(
os.path.join(git_info_dir, 'git-config'),
git_config)
cookie_auth = gerrit_util.Authenticator.get()
if isinstance(cookie_auth, gerrit_util.CookiesAuthenticator):
gitcookies_path = cookie_auth.get_gitcookies_path()
gitcookies = gclient_utils.FileRead(gitcookies_path)
gclient_utils.FileWrite(
os.path.join(git_info_dir, 'gitcookies'),
GITCOOKIES_REDACT_RE.sub('REDACTED', gitcookies))
shutil.make_archive(git_info_zip, 'zip', git_info_dir)
gclient_utils.rmtree(git_info_dir)
gclient_utils.rmtree(traces_dir)
return push_stdout
......@@ -2881,14 +2822,7 @@ class _GerritChangelistImpl(_ChangelistCodereviewBase):
'spaces not allowed in refspec: "%s"' % refspec_suffix)
refspec = '%s:refs/for/%s%s' % (ref_to_push, branch, refspec_suffix)
git_push_metadata = {
'gerrit_host': self._GetGerritHost(),
'title': title or '<untitled>',
'change_id': change_id,
'description': change_desc.description,
}
push_stdout = self._RunGitPushWithTraces(
change_desc, refspec, refspec_opts, git_push_metadata)
push_stdout = self._RunGitPushWithTraces(change_desc, refspec, refspec_opts)
if options.squash:
regex = re.compile(r'remote:\s+https?://[\w\-\.\+\/#]*/(\d+)\s.*')
......
tests/git_cl_test.py
View file @ e7f0b4c6
......@@ -909,8 +909,7 @@ class TestGitCl(TestCase):
post_amend_description=None, issue=None, cc=None,
custom_cl_base=None, tbr=None,
short_hostname='chromium',
labels=None, change_id=None, original_title=None,
final_description=None):
labels=None):
if post_amend_description is None:
post_amend_description = description
cc = cc or []
......@@ -1108,67 +1107,6 @@ class TestGitCl(TestCase):
None,),
]
final_description = final_description or post_amend_description.strip()
original_title = original_title or title or '<untitled>'
# Trace-related calls
calls += [
# Write a description with context for the current trace.
((['FileWrite', 'TRACES_DIR/20170316T200041.000000-README',
'Date: Thu Mar 16 20:00:41 2017\n\n'
'Change: https://%(short_hostname)s-review.googlesource.com/'
'q/%(change_id)s\n'
'Title: %(title)s\n\n'
'%(description)s\n\n'
'Execution time: 1000\n'
'Exit code: 0\n\n'
'When filing a bug for this push, be sure to include the traces '
'found at:\n'
' TRACES_DIR/20170316T200041.000000-traces.zip\n'
'Consider including the git config and gitcookies, which we have '
'packed for \nyou at:\n'
' TRACES_DIR/20170316T200041.000000-git-info.zip\n' % {
'short_hostname': short_hostname,
'change_id': change_id,
'description': final_description,
'title': original_title,
}],),
None,
),
# Read traces and shorten git hashes.
((['FileRead', 'TEMP_DIR/trace-packet'],),
('git-hash: 0123456789012345678901234567890123456789\n'
'git-hash: abcdeabcdeabcdeabcdeabcdeabcdeabcdeabcde\n'),
),
((['FileWrite', 'TEMP_DIR/trace-packet',
'git-hash: 012345\n'
'git-hash: abcdea\n'],),
None,
),
# Make zip file for the git traces.
((['make_archive', 'TRACES_DIR/20170316T200041.000000-traces', 'zip',
'TEMP_DIR'],),
None,
),
# Collect git config and gitcookies.
((['git', 'config', '-l'],),
'git-config-output',
),
((['FileWrite', 'TEMP_DIR/git-config', 'git-config-output'],),
None,
),
((['FileRead', '~/.gitcookies'],),
'gitcookies 1/SECRET',
),
((['FileWrite', 'TEMP_DIR/gitcookies', 'gitcookies REDACTED'],),
None,
),
# Make zip file for the git config and gitcookies.
((['make_archive', 'TRACES_DIR/20170316T200041.000000-git-info', 'zip',
'TEMP_DIR'],),
None,
),
]
if squash:
calls += [
((['git', 'config', 'branch.master.gerritissue', '123456'],),
......@@ -1209,10 +1147,7 @@ class TestGitCl(TestCase):
custom_cl_base=None,
tbr=None,
short_hostname='chromium',
labels=None,
change_id=None,
original_title=None,
final_description=None):
labels=None):
"""Generic gerrit upload test framework."""
if squash_mode is None:
if '--no-squash' in upload_args:
......@@ -1234,17 +1169,6 @@ class TestGitCl(TestCase):
lambda *_, **__: self._mocked_call(['RunEditor']))
self.mock(git_cl, 'DownloadGerritHook', lambda force: self._mocked_call(
'DownloadGerritHook', force))
self.mock(git_cl.gclient_utils, 'FileRead',
lambda path: self._mocked_call(['FileRead', path]))
self.mock(git_cl.gclient_utils, 'FileWrite',
lambda path, contents: self._mocked_call(
['FileWrite', path, contents]))
self.mock(git_cl, 'datetime_now',
lambda: datetime.datetime(2017, 3, 16, 20, 0, 41, 0))
self.mock(git_cl.tempfile, 'mkdtemp', lambda: 'TEMP_DIR')
self.mock(git_cl, 'TRACES_DIR', 'TRACES_DIR')
self.mock(git_cl.shutil, 'make_archive',
lambda *args: self._mocked_call(['make_archive'] + list(args)))
self.calls = self._gerrit_base_calls(
issue=issue,
......@@ -1266,10 +1190,7 @@ class TestGitCl(TestCase):
issue=issue, cc=cc,
custom_cl_base=custom_cl_base, tbr=tbr,
short_hostname=short_hostname,
labels=labels,
change_id=change_id,
original_title=original_title,
final_description=final_description)
labels=labels)
# Uncomment when debugging.
# print('\n'.join(map(lambda x: '%2i: %s' % x, enumerate(self.calls))))
git_cl.main(['upload'] + upload_args)
......@@ -1280,8 +1201,7 @@ class TestGitCl(TestCase):
'desc\n\nBUG=\n',
[],
squash=False,
post_amend_description='desc\n\nBUG=\n\nChange-Id: Ixxx',
change_id='Ixxx')
post_amend_description='desc\n\nBUG=\n\nChange-Id: Ixxx')
def test_gerrit_upload_without_change_id_override_nosquash(self):
self._run_gerrit_upload_test(
......@@ -1290,8 +1210,7 @@ class TestGitCl(TestCase):
[],
squash=False,
squash_mode='override_nosquash',
post_amend_description='desc\n\nBUG=\n\nChange-Id: Ixxx',
change_id='Ixxx')
post_amend_description='desc\n\nBUG=\n\nChange-Id: Ixxx')
def test_gerrit_no_reviewer(self):
self._run_gerrit_upload_test(
......@@ -1299,8 +1218,7 @@ class TestGitCl(TestCase):
'desc\n\nBUG=\n\nChange-Id: I123456789\n',
[],
squash=False,
squash_mode='override_nosquash',
change_id='I123456789')
squash_mode='override_nosquash')
def test_gerrit_no_reviewer_non_chromium_host(self):
# TODO(crbug/877717): remove this test case.
......@@ -1310,8 +1228,7 @@ class TestGitCl(TestCase):
[],
squash=False,
squash_mode='override_nosquash',
short_hostname='other',
change_id='I123456789')
short_hostname='other')
def test_gerrit_patchset_title_special_chars(self):
self.mock(git_cl.sys, 'stdout', StringIO.StringIO())
......@@ -1320,9 +1237,7 @@ class TestGitCl(TestCase):
'desc\n\nBUG=\n\nChange-Id: I123456789',
squash=False,
squash_mode='override_nosquash',
title='We%27ll_escape_%5E%5F_%5E_special_chars%2E%2E%2E%40%7Bu%7D',
change_id='I123456789',
original_title='We\'ll escape ^_ ^ special chars...@{u}')
title='We%27ll_escape_%5E%5F_%5E_special_chars%2E%2E%2E%40%7Bu%7D')
def test_gerrit_reviewers_cmd_line(self):
self._run_gerrit_upload_test(
......@@ -1331,10 +1246,7 @@ class TestGitCl(TestCase):
['foo@example.com'],
squash=False,
squash_mode='override_nosquash',
notify=True,
change_id='I123456789',
final_description=
'desc\n\nBUG=\nR=foo@example.com\n\nChange-Id: I123456789')
notify=True)
def test_gerrit_reviewer_multiple(self):
self.mock(git_cl.gerrit_util, 'GetCodeReviewTbrScore',
......@@ -1348,18 +1260,14 @@ class TestGitCl(TestCase):
expected_upstream_ref='origin/master',
cc=['more@example.com', 'people@example.com'],
tbr='reviewer@example.com',
labels={'Code-Review': 2},
change_id='123456789',
original_title='Initial upload')
labels={'Code-Review': 2})
def test_gerrit_upload_squash_first_is_default(self):
self._run_gerrit_upload_test(
[],
'desc\nBUG=\n\nChange-Id: 123456789',
[],
expected_upstream_ref='origin/master',
change_id='123456789',
original_title='Initial upload')
expected_upstream_ref='origin/master')
def test_gerrit_upload_squash_first(self):
self._run_gerrit_upload_test(
......@@ -1367,9 +1275,7 @@ class TestGitCl(TestCase):
'desc\nBUG=\n\nChange-Id: 123456789',
[],
squash=True,
expected_upstream_ref='origin/master',
change_id='123456789',
original_title='Initial upload')
expected_upstream_ref='origin/master')
def test_gerrit_upload_squash_first_with_labels(self):
self._run_gerrit_upload_test(
......@@ -1378,9 +1284,7 @@ class TestGitCl(TestCase):
[],
squash=True,
expected_upstream_ref='origin/master',
labels={'Commit-Queue': 1, 'Auto-Submit': 1},
change_id='123456789',
original_title='Initial upload')
labels={'Commit-Queue': 1, 'Auto-Submit': 1})
def test_gerrit_upload_squash_first_against_rev(self):
custom_cl_base = 'custom_cl_base_rev_or_branch'
......@@ -1390,9 +1294,7 @@ class TestGitCl(TestCase):
[],
squash=True,
expected_upstream_ref='origin/master',
custom_cl_base=custom_cl_base,
change_id='123456789',
original_title='Initial upload')
custom_cl_base=custom_cl_base)
self.assertIn(
'If you proceed with upload, more than 1 CL may be created by Gerrit',
sys.stdout.getvalue())
......@@ -1405,9 +1307,7 @@ class TestGitCl(TestCase):
[],
squash=True,
expected_upstream_ref='origin/master',
issue=123456,
change_id='123456789',
original_title='User input')
issue=123456)
def test_gerrit_upload_squash_reupload_to_abandoned(self):
self.mock(git_cl, 'DieWithError',
......@@ -1421,8 +1321,7 @@ class TestGitCl(TestCase):
squash=True,
expected_upstream_ref='origin/master',
issue=123456,
fetched_status='ABANDONED',
change_id='123456789')
fetched_status='ABANDONED')
def test_gerrit_upload_squash_reupload_to_not_owned(self):
self.mock(git_cl.gerrit_util, 'GetAccountDetails',
......@@ -1435,9 +1334,7 @@ class TestGitCl(TestCase):
squash=True,
expected_upstream_ref='origin/master',
issue=123456,
other_cl_owner='other@example.com',
change_id='123456789',
original_title='User input')
other_cl_owner='other@example.com')
self.assertIn(
'WARNING: Change 123456 is owned by other@example.com, but you '
'authenticate to Gerrit as yet-another@example.com.\n'
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment