diff --git a/git_cl.py b/git_cl.py index 9e1471996..1b375e856 100755 --- a/git_cl.py +++ b/git_cl.py @@ -18,6 +18,7 @@ import Queue import re import stat import sys +import tempfile import textwrap import threading import urllib2 @@ -113,6 +114,11 @@ def RunGitWithCode(args, suppress_stderr=False): return 1, '' +def RunGitSilent(args): + """Returns stdout, suppresses stderr and ingores the return code.""" + return RunGitWithCode(args, suppress_stderr=True)[1] + + def IsGitVersionAtLeast(min_version): prefix = 'git version ' version = RunGit(['--version']).strip() @@ -1633,7 +1639,7 @@ def GerritUpload(options, args, cl, change): """upload the current branch to gerrit.""" # We assume the remote called "origin" is the one we want. # It is probably not worthwhile to support different workflows. - remote = 'origin' + gerrit_remote = 'origin' branch = 'master' if options.target_branch: branch = options.target_branch @@ -1643,15 +1649,72 @@ def GerritUpload(options, args, cl, change): if not change_desc.description: print "Description is empty; aborting." return 1 - if CHANGE_ID not in change_desc.description: - AddChangeIdToCommitMessage(options, args) - commits = RunGit(['rev-list', '%s/%s..' % (remote, branch)]).splitlines() + if options.squash: + # Try to get the message from a previous upload. + shadow_branch = 'refs/heads/git_cl_uploads/' + cl.GetBranch() + message = RunGitSilent(['show', '--format=%s\n\n%b', '-s', shadow_branch]) + if not message: + if not options.force: + change_desc.prompt() + + if CHANGE_ID not in change_desc.description: + # Run the commit-msg hook without modifying the head commit by writing + # the commit message to a temporary file and running the hook over it, + # then reading the file back in. + commit_msg_hook = os.path.join(settings.GetRoot(), '.git', 'hooks', + 'commit-msg') + file_handle, msg_file = tempfile.mkstemp(text=True, + prefix='commit_msg') + try: + try: + with os.fdopen(file_handle, 'w') as fileobj: + fileobj.write(change_desc.description) + finally: + os.close(file_handle) + RunCommand([commit_msg_hook, msg_file]) + change_desc.set_description(gclient_utils.FileRead(msg_file)) + finally: + os.remove(msg_file) + + if not change_desc.description: + print "Description is empty; aborting." + return 1 + + message = change_desc.description + + remote, upstream_branch = cl.FetchUpstreamTuple(cl.GetBranch()) + if remote is '.': + # If our upstream branch is local, we base our squashed commit on its + # squashed version. + parent = ('refs/heads/git_cl_uploads/' + + scm.GIT.ShortBranchName(upstream_branch)) + + # Verify that the upstream branch has been uploaded too, otherwise Gerrit + # will create additional CLs when uploading. + if (RunGitSilent(['rev-parse', upstream_branch + ':']) != + RunGitSilent(['rev-parse', parent + ':'])): + print 'Upload upstream branch ' + upstream_branch + ' first.' + return 1 + else: + parent = cl.GetCommonAncestorWithUpstream() + + tree = RunGit(['rev-parse', 'HEAD:']).strip() + ref_to_push = RunGit(['commit-tree', tree, '-p', parent, + '-m', message]).strip() + else: + if CHANGE_ID not in change_desc.description: + AddChangeIdToCommitMessage(options, args) + ref_to_push = 'HEAD' + parent = '%s/%s' % (gerrit_remote, branch) + + commits = RunGitSilent(['rev-list', '%s..%s' % (parent, + ref_to_push)]).splitlines() if len(commits) > 1: print('WARNING: This will upload %d commits. Run the following command ' 'to see which commits will be uploaded: ' % len(commits)) - print('git log %s/%s..' % (remote, branch)) - print('You can also use `git squash-branch` to squash these into a single' + print('git log %s..%s' % (parent, ref_to_push)) + print('You can also use `git squash-branch` to squash these into a single ' 'commit.') ask_for_data('About to upload; enter to confirm.') @@ -1673,8 +1736,13 @@ def GerritUpload(options, args, cl, change): if receive_options: git_command.append('--receive-pack=git receive-pack %s' % ' '.join(receive_options)) - git_command += [remote, 'HEAD:refs/for/' + branch] + git_command += [gerrit_remote, ref_to_push + ':refs/for/' + branch] RunGit(git_command) + + if options.squash: + head = RunGit(['rev-parse', 'HEAD']).strip() + RunGit(['update-ref', '-m', 'Uploaded ' + head, shadow_branch, ref_to_push]) + # TODO(ukai): parse Change-Id: and set issue number? return 0 @@ -1690,7 +1758,7 @@ def GetTargetRef(remote, remote_branch, target_branch, pending_prefix): """ if not (remote and remote_branch): return None - + if target_branch: # Cannonicalize branch references to the equivalent local full symbolic # refs, which are then translated into the remote full symbolic refs @@ -1716,7 +1784,7 @@ def GetTargetRef(remote, remote_branch, target_branch, pending_prefix): not remote_branch.startswith('refs/remotes/%s/refs' % remote)): # Default to master for refs that are not branches. remote_branch = 'refs/remotes/%s/master' % remote - + # Create the true path to the remote branch. # Does the following translation: # * refs/remotes/origin/refs/diff/test -> refs/diff/test @@ -1896,6 +1964,8 @@ def CMDupload(parser, args): metavar='TARGET', help='Apply CL to remote ref TARGET. ' + 'Default: remote branch head, or master') + parser.add_option('--squash', action='store_true', + help='Squash multiple commits into one (Gerrit only)') parser.add_option('--email', default=None, help='email address to use to connect to Rietveld') parser.add_option('--tbr-owners', dest='tbr_owners', action='store_true', diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 42231e9df..94cd66263 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -571,7 +571,7 @@ class TestGitCl(TestCase): ] @staticmethod - def _gerrit_upload_calls(description, reviewers): + def _gerrit_upload_calls(description, reviewers, squash): calls = [ ((['git', 'config', 'gerrit.host'],), 'gerrit.example.com'), @@ -590,8 +590,29 @@ class TestGitCl(TestCase): 'fake_ancestor_sha..HEAD'],), description) ] + if squash: + ref_to_push = 'abcdef0123456789' + calls += [ + ((['git', 'show', '--format=%s\n\n%b', '-s', + 'refs/heads/git_cl_uploads/master'],), + (description, 0)), + ((['git', 'config', 'branch.master.merge'],), + 'refs/heads/master'), + ((['git', 'config', 'branch.master.remote'],), + 'origin'), + ((['get_or_create_merge_base', 'master', 'master'],), + 'origin/master'), + ((['git', 'rev-parse', 'HEAD:'],), + '0123456789abcdef'), + ((['git', 'commit-tree', '0123456789abcdef', '-p', + 'origin/master', '-m', 'd'],), + ref_to_push), + ] + else: + ref_to_push = 'HEAD' + calls += [ - ((['git', 'rev-list', 'origin/master..'],), ''), + ((['git', 'rev-list', 'origin/master..' + ref_to_push],), ''), ((['git', 'config', 'rietveld.cc'],), '') ] receive_pack = '--receive-pack=git receive-pack ' @@ -603,19 +624,28 @@ class TestGitCl(TestCase): receive_pack += '' calls += [ ((['git', - 'push', receive_pack, 'origin', 'HEAD:refs/for/master'],), + 'push', receive_pack, 'origin', ref_to_push + ':refs/for/master'],), '') ] + if squash: + calls += [ + ((['git', 'rev-parse', 'HEAD'],), 'abcdef0123456789'), + ((['git', 'update-ref', '-m', 'Uploaded abcdef0123456789', + 'refs/heads/git_cl_uploads/master', 'abcdef0123456789'],), + '') + ] + return calls def _run_gerrit_upload_test( self, upload_args, description, - reviewers): + reviewers, + squash=False): """Generic gerrit upload test framework.""" self.calls = self._gerrit_base_calls() - self.calls += self._gerrit_upload_calls(description, reviewers) + self.calls += self._gerrit_upload_calls(description, reviewers, squash) git_cl.main(['upload'] + upload_args) def test_gerrit_upload_without_change_id(self): @@ -643,6 +673,12 @@ class TestGitCl(TestCase): 'Change-Id:123456789\n', ['reviewer@example.com', 'another@example.com']) + def test_gerrit_upload_squash(self): + self._run_gerrit_upload_test( + ['--squash'], + 'desc\n\nBUG=\nChange-Id:123456789\n', + [], + squash=True) def test_config_gerrit_download_hook(self): self.mock(git_cl, 'FindCodereviewSettingsFile', CodereviewSettingsFileMock) @@ -762,7 +798,7 @@ class TestGitCl(TestCase): self.assertEqual(None, git_cl.GetTargetRef(None, 'refs/remotes/origin/master', 'master', None)) - + # Check default target refs for branches. self.assertEqual('refs/heads/master', git_cl.GetTargetRef('origin', 'refs/remotes/origin/master',