opendev
/
git-review
Code Issues Proposed changes

Don't keep incomplete rebase state by default 

Dumping the user into a dirtied working tree after a failed rebase
attempt can be confusing, no matter how much contextual explanation
we provide when doing so. By default, run `git rebase --abort`
automatically so as to clean up from a failed test rebase, and then
let the user rebase again on their own if that's the state they want
to be in. Add a -K/--keep-rebase option to get the old behavior, and
mention it when we automatically abort in case the user wants to
just have git-review redo the rebase for them and leave things in
that incomplete state.

Change-Id: I7d7bfca1623a71a9b4fe445360d94fd6b039f040
This commit is contained in:
Jeremy Stanley 2022-07-15 18:21:39 +00:00
parent 7009d7d03d
commit 6d61b1c2cf
2 changed files with 51 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
git_review/cmd.py
View File

@ -900,7 +900,7 @@ def check_remote(branch, remote, scheme, hostname, port, project,
raise
def rebase_changes(branch, remote, interactive=True):
def rebase_changes(branch, remote, interactive=True, keep=False):
global _orig_head
@ -959,17 +959,35 @@ def rebase_changes(branch, remote, interactive=True):
print("Errors running %s" % cmd)
if interactive:
print(output)
printwrap("It is likely that your change has a merge conflict. "
"You may resolve it in the working tree now as "
"described above and then run 'git review' again, or "
"if you do not want to resolve it yet (note that the "
"change can not merge until the conflict is resolved) "
"you may run 'git rebase --abort' then 'git review -R' "
"to upload the change without rebasing.")
if keep:
printwrap("It is likely that your change has a merge "
"conflict. You may resolve it in the working tree "
"now as described above and then run 'git review' "
"again, or if you do not want to resolve it yet "
"(note that the change can not merge until the "
"conflict is resolved) you may run 'git rebase "
"--abort' then 'git review -R' to upload the change "
"without rebasing.")
else:
printwrap("It is likely that your change has a merge "
"conflict, but the result of the test rebase has "
"been discarded. You may rebase it yourself and "
"review again, or use 'git review -R' to upload the "
"change without resolving the conflict, or use the "
"--keep-rebase option to leave the incomplete "
"rebase result in your working tree.")
return False
return True
def abort_rebase():
cmd = "git rebase --abort"
(status, output) = run_command_status(cmd)
if status != 0:
print("Errors running %s" % cmd)
print(output)
def undo_rebase():
global _orig_head
if not _orig_head:
@ -1546,6 +1564,10 @@ additional information:
action="store_true",
help="Force and push a rebase even when not"
" needed.")
rebase_group.add_argument("-K", "--keep-rebase", dest="keep_rebase",
action="store_true",
help="Keep the unfinished test rebase if a "
" merge conflict is detected.")
track_group = parser.add_mutually_exclusive_group()
track_group.add_argument("--track", dest="track",
@ -1768,7 +1790,9 @@ additional information:
return
if options.rebase or options.force_rebase:
if not rebase_changes(branch, remote):
if not rebase_changes(branch, remote, keep=options.keep_rebase):
if not options.keep_rebase:
abort_rebase()
sys.exit(1)
if not options.force_rebase and not undo_rebase():
sys.exit(1)

19
git_review/tests/test_git_review.py
View File

@ -328,8 +328,25 @@ class GitReviewTestCase(tests.BaseGitReviewTestCase):
"Errors running git rebase --preserve-merges "
"-i remotes/%s/master" % self._remote in exc.args[0])
self.assertTrue(rebased)
self.assertIn("It is likely that your change has a merge conflict.",
self.assertIn("It is likely that your change has a merge conflict",
exc.args[0])
output = self._run_git('rebase', '--abort')
self.assertIn("fatal: No rebase in progress?", output)
def test_keep_aborted_rebase_state(self):
"""Test git-review -K behavior when rebase is needed."""
self._run_git_review('-s')
head_1 = self._run_git('rev-parse', 'HEAD^1')
self._run_git('checkout', '-b', 'test_branch', head_1)
self._simple_change('some other message',
'create conflict with master')
self._run_git_review('-K')
output = self._run_git('status')
self.assertIn(
"You are currently editing a commit while rebasing", output)
def test_upload_without_rebase(self):
"""Test change not needing a rebase can upload without rebasing."""