diff --git a/git_cl.py b/git_cl.py index cf784bb4e..09d908471 100755 --- a/git_cl.py +++ b/git_cl.py @@ -2377,26 +2377,28 @@ class Changelist(object): # Change-Id. Thus, just create a new footer, but let user verify the # new description. message = '%s\n\nChange-Id: %s' % (message, change_id) - print( - 'WARNING: change %s has Change-Id footer(s):\n' - ' %s\n' - 'but change has Change-Id %s, according to Gerrit.\n' - 'Please, check the proposed correction to the description, ' - 'and edit it if necessary but keep the "Change-Id: %s" footer\n' - % (self.GetIssue(), '\n '.join(footer_change_ids), change_id, - change_id)) - confirm_or_exit(action='edit') + change_desc = ChangeDescription(message, bug=bug) if not options.force: - change_desc = ChangeDescription(message) - change_desc.prompt(bug=bug) - message = change_desc.description - if not message: - DieWithError("Description is empty. Aborting...") + print( + 'WARNING: change %s has Change-Id footer(s):\n' + ' %s\n' + 'but change has Change-Id %s, according to Gerrit.\n' + 'Please, check the proposed correction to the description, ' + 'and edit it if necessary but keep the "Change-Id: %s" footer\n' + % (self.GetIssue(), '\n '.join(footer_change_ids), change_id, + change_id)) + confirm_or_exit(action='edit') + change_desc.prompt() + + message = change_desc.description + if not message: + DieWithError("Description is empty. Aborting...") + # Continue the while loop. # Sanity check of this code - we should end up with proper message # footer. assert [change_id] == git_footers.get_footer_change_id(message) - change_desc = ChangeDescription(message) + change_desc = ChangeDescription(message, bug=bug) else: # if not self.GetIssue() if options.message: message = options.message @@ -2404,10 +2406,10 @@ class Changelist(object): message = _create_description_from_log(git_diff_args) if options.title: message = options.title + '\n\n' + message - change_desc = ChangeDescription(message) - + change_desc = ChangeDescription(message, bug=bug) if not options.force: - change_desc.prompt(bug=bug) + change_desc.prompt() + # On first upload, patchset title is always this string, while # --title flag gets converted to first line of message. title = 'Initial upload' @@ -2441,7 +2443,7 @@ class Changelist(object): ref_to_push = RunGit(['commit-tree', tree, '-p', parent, '-F', desc_tempfile.name]).strip() os.remove(desc_tempfile.name) - else: + else: # if not options.squash change_desc = ChangeDescription( options.message or _create_description_from_log(git_diff_args)) if not change_desc.description: @@ -2792,8 +2794,14 @@ class ChangeDescription(object): COLON_SEPARATED_HASH_TAG = r'^([a-zA-Z0-9_\- ]+):' BAD_HASH_TAG_CHUNK = r'[^a-zA-Z0-9]+' - def __init__(self, description): + def __init__(self, description, bug=None): self._description_lines = (description or '').strip().splitlines() + if bug: + regexp = re.compile(self.BUG_LINE) + prefix = settings.GetBugPrefix() + if not any((regexp.match(line) for line in self._description_lines)): + values = list(_get_bug_line_values(prefix, bug)) + self.append_footer('Bug: %s' % ', '.join(values)) @property # www.logilab.org/ticket/89786 def description(self): # pylint: disable=method-hidden @@ -2888,7 +2896,7 @@ class ChangeDescription(object): return self.append_footer('Cq-Do-Not-Cancel-Tryjobs: true') - def prompt(self, bug=None, git_footer=True): + def prompt(self): """Asks the user to update the description.""" self.set_description([ '# Enter a description of the change.', @@ -2897,19 +2905,13 @@ class ChangeDescription(object): '#--------------------This line is 72 characters long' '--------------------', ] + self._description_lines) - regexp = re.compile(self.BUG_LINE) prefix = settings.GetBugPrefix() if not any((regexp.match(line) for line in self._description_lines)): - values = list(_get_bug_line_values(prefix, bug or '')) or [prefix] - if git_footer: - self.append_footer('Bug: %s' % ', '.join(values)) - else: - for value in values: - self.append_footer('BUG=%s' % value) + self.append_footer('Bug: %s' % prefix) content = gclient_utils.RunEditor(self.description, True, - git_editor=settings.GetGitEditor()) + git_editor=settings.GetGitEditor()) if not content: DieWithError('Running editor failed') lines = content.splitlines() diff --git a/tests/git_cl_test.py b/tests/git_cl_test.py index 82a06fc69..f1cdf4d62 100755 --- a/tests/git_cl_test.py +++ b/tests/git_cl_test.py @@ -822,7 +822,8 @@ class TestGitCl(TestCase): @classmethod def _gerrit_base_calls(cls, issue=None, fetched_description=None, fetched_status=None, other_cl_owner=None, - custom_cl_base=None, short_hostname='chromium'): + custom_cl_base=None, short_hostname='chromium', + change_id=None): calls = cls._is_gerrit_calls(True) if not custom_cl_base: calls += [ @@ -854,7 +855,7 @@ class TestGitCl(TestCase): ), { 'owner': {'email': (other_cl_owner or 'owner@example.com')}, - 'change_id': '123456789', + 'change_id': (change_id or '123456789'), 'current_revision': 'sha1_of_current_revision', 'revisions': {'sha1_of_current_revision': { 'commit': {'message': fetched_description}, @@ -918,7 +919,8 @@ class TestGitCl(TestCase): custom_cl_base=None, tbr=None, short_hostname='chromium', labels=None, change_id=None, original_title=None, - final_description=None, gitcookies_exists=True): + final_description=None, gitcookies_exists=True, + force=False): if post_amend_description is None: post_amend_description = description cc = cc or [] @@ -970,13 +972,20 @@ class TestGitCl(TestCase): post_amend_description) ] if squash: - if not issue: + if force or not issue: + if issue: + calls += [ + ((['git', 'config', 'rietveld.bug-prefix'],), ''), + ] # Prompting to edit description on first upload. calls += [ ((['git', 'config', 'rietveld.bug-prefix'],), ''), - ((['git', 'config', 'core.editor'],), ''), - ((['RunEditor'],), description), ] + if not force: + calls += [ + ((['git', 'config', 'core.editor'],), ''), + ((['RunEditor'],), description), + ] ref_to_push = 'abcdef0123456789' calls += [ ((['git', 'config', 'branch.master.merge'],), 'refs/heads/master'), @@ -1228,7 +1237,9 @@ class TestGitCl(TestCase): change_id=None, original_title=None, final_description=None, - gitcookies_exists=True): + gitcookies_exists=True, + force=False, + fetched_description=None): """Generic gerrit upload test framework.""" if squash_mode is None: if '--no-squash' in upload_args: @@ -1275,11 +1286,12 @@ class TestGitCl(TestCase): self.calls = self._gerrit_base_calls( issue=issue, - fetched_description=description, + fetched_description=fetched_description or description, fetched_status=fetched_status, other_cl_owner=other_cl_owner, custom_cl_base=custom_cl_base, - short_hostname=short_hostname) + short_hostname=short_hostname, + change_id=change_id) if fetched_status != 'ABANDONED': self.mock(tempfile, 'NamedTemporaryFile', MakeNamedTemporaryFileMock( expected_content=description)) @@ -1297,7 +1309,8 @@ class TestGitCl(TestCase): change_id=change_id, original_title=original_title, final_description=final_description, - gitcookies_exists=gitcookies_exists) + gitcookies_exists=gitcookies_exists, + force=force) # Uncomment when debugging. # print('\n'.join(map(lambda x: '%2i: %s' % x, enumerate(self.calls)))) git_cl.main(['upload'] + upload_args) @@ -1374,6 +1387,30 @@ class TestGitCl(TestCase): final_description=( 'desc\n\nBUG=\nR=foo@example.com\n\nChange-Id: I123456789')) + def test_gerrit_upload_force_sets_bug(self): + self._run_gerrit_upload_test( + ['-b', '10000', '-f'], + u'desc=\n\nBug: 10000\nChange-Id: Ixxx', + [], + force=True, + expected_upstream_ref='origin/master', + fetched_description='desc=\n\nChange-Id: Ixxx', + original_title='Initial upload', + change_id='Ixxx') + + def test_gerrit_upload_force_sets_bug_if_wrong_changeid(self): + self._run_gerrit_upload_test( + ['-b', '10000', '-f', '-m', 'Title'], + u'desc=\n\nChange-Id: Ixxxx\n\nChange-Id: Izzzz\nBug: 10000', + [], + force=True, + issue='123456', + expected_upstream_ref='origin/master', + fetched_description='desc=\n\nChange-Id: Ixxxx', + original_title='Title', + title='Title', + change_id='Izzzz') + def test_gerrit_reviewer_multiple(self): self.mock(git_cl.gerrit_util, 'GetCodeReviewTbrScore', lambda *a: self._mocked_call('GetCodeReviewTbrScore', *a))