git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: "SZEDER Gábor" <szeder.dev@gmail.com>,
	"Brian Norris" <briannorris@chromium.org>
Cc: git@vger.kernel.org, Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: git 2.24: git revert <commit1> <commit2> requires extra '--continue'?
Date: Sat, 23 Nov 2019 09:53:51 +0000	[thread overview]
Message-ID: <e7c01e0f-8466-c2c5-b53a-a93f941dfb1c@gmail.com> (raw)
In-Reply-To: <20191123003454.GA23183@szeder.dev>

Hi Brian & Gábor

Thanks to Brian for reporting this and Gábor for bisecting and creating 
the tests.

On 23/11/2019 00:34, SZEDER Gábor wrote:
> On Fri, Nov 22, 2019 at 03:10:20PM -0800, Brian Norris wrote:
>> Hi! I'm using git 2.24 (or, a variant of that packaged my distro -- I
>> can try to build my own if this is deemed not reproducible), and I
>> feel like I've been seeing a regression here:
>>
>> Previously, when reverting multiple commits (with the default --edit
>> behavior), as soon as I'm done editing the first revert commit
>> message, the second revert commit pops up an editor, and I'm on my
>> way. Now, it seems to require an extra 'git revert --continue' prompt
>> in between, yet it doesn't actually recommend that. This is highly
>> confusing, and seemingly unnecessary.
> 
> Thanks for the report, this is indeed a regression in v2.24.0: it
> bisects down to a47ba3c777 (rebase -i: check for updated todo after
> squash and reword, 2019-08-19) [Cc'ing Phillip].  It's not specific to
> 'git revert', but with a slight variation affects 'git cherry-pick' as
> well.

Thanks, I suspect it's missing an 'if (is_rebase_i(opts))' I'll take a 
look later and come up with a fix

Best Wishes

Phillip

>> An annotated transcript provided below.
> 
> I transcribed your transcript into tests that can be run in our test
> framework and demonstrate this regression:
> 
>    --- >8 ---
> 
> #!/bin/sh
> 
> test_description='test'
> 
> . ./test-lib.sh
> 
> test_expect_success "Brian's revert regression" '
> 	test_create_repo revert &&
> 	(
> 		cd revert &&
> 
> 		echo 1 >file &&
> 		git add file &&
> 		git commit -m first &&
> 		echo 2 >file &&
> 		git commit -am second &&
> 		echo 3 >file &&
> 		git commit -am third &&
> 
> 		git checkout -b branch &&
> 
> 		git revert --edit HEAD HEAD^ &&
> 
> 		echo 1 >expect &&
> 		test_cmp expect file
> 	)
> '
> 
> test_expect_success "a variant of Brian's regression for cherry-pick" '
> 	test_create_repo cherry-pick &&
> 	(
> 		cd cherry-pick &&
> 
> 		echo 1 >file &&
> 		git add file &&
> 		git commit -m first &&
> 		echo 2 >file &&
> 		git commit -am second &&
> 		echo 3 >file &&
> 		git commit -am third &&
> 
> 		git checkout -b branch HEAD^^ &&
> 
> 		git cherry-pick --edit master^ master &&
> 
> 		echo 3 >expect &&
> 		test_cmp expect file
> 	)
> '
> 
> test_done
> 
>    ---  >8  ---
> 
> They both succeed on a47ba3c777's parent, but fail on a47ba3c777 when
> the 'git revert' or 'git cherry-pick' commands return with exit code 1
> after reverting/cherry-picking the first commit instead of processing
> the second commit:
> 
>    + git revert --edit HEAD HEAD^
>    [branch 88ea48c] Revert "third"
>     Author: A U Thor <author@example.com>
>     1 file changed, 1 insertion(+), 1 deletion(-)
>    On branch branch
>    Revert currently in progress.
>    
>    nothing to commit, working tree clean
>    error: last command exited with $?=1
>    not ok 1 - Brian's revert regression
> 
> 
> 
>    + git cherry-pick --edit master^ master
>    [branch 2cb3f74] second
>     Author: A U Thor <author@example.com>
>     Date: Sat Nov 23 00:17:32 2019 +0000
>     1 file changed, 1 insertion(+), 1 deletion(-)
>    On branch branch
>    Cherry-pick currently in progress.
>    
>    nothing to commit, working tree clean
>    The previous cherry-pick is now empty, possibly due to conflict
>    resolution.
>    If you wish to commit it anyway, use:
>    
>        git commit --allow-empty
>    
>    If you wish to skip this commit, use:
>    
>        git reset
>    
>    Then "git cherry-pick --continue" will resume cherry-picking
>    the remaining commits.
>    error: last command exited with $?=1
>    not ok 2 - a variant of Brian's regression for cherry-pick
> 
> 
> These test should probably be squeezed into
> 't3508-cherry-pick-many-commits.sh', but Friday has just turned into
> Saturday around here, so that's enough from me for now.
> 
> 
>> Note that none of this happens with --no-edit; my reverts happen
>> smoothly, with no extra need for --continue.
>>
>> Regards,
>> Brian
>>
>> $ mkdir tmp
>> $ cd tmp
>> /tmp$ git init
>> Initialized empty Git repository in [...]/tmp/.git/
>> /tmp$ touch foo
>> /tmp$ git add foo
>> /tmp$ echo bar >> foo
>> /tmp$ git commit -am baz
>> [master (root-commit) a388f78d9013] baz
>>   1 file changed, 1 insertion(+)
>>   create mode 100644 foo
>> /tmp$ echo pow >> foo
>> /tmp$ git commit -am whizzbang
>> [master 51753222dd9a] whizzbang
>>   1 file changed, 1 insertion(+)
>> /tmp$ echo pop >> foo
>> /tmp$ git commit -am nothing
>> [master 7153607b11e0] nothing
>>   1 file changed, 1 insertion(+)
>> /tmp$ git revert HEAD HEAD^
>> ## EDITOR pops up, as expected
>> [master 586469974ec2] Revert "nothing"
>>   1 file changed, 1 deletion(-)
>> On branch master
>> Revert currently in progress.
>>
>> nothing to commit, working tree clean
>> ## Unexpected, confusing pause? No prompt to '--continue'
>> /tmp$ git log --oneline
>> 586469974ec2 (HEAD -> master) Revert "nothing"
>> 7153607b11e0 nothing
>> 51753222dd9a whizzbang
>> a388f78d9013 baz
>> /tmp$ git status
>> On branch master
>> Revert currently in progress.
>>    (run "git revert --continue" to continue)
>>    (use "git revert --skip" to skip this patch)
>>    (use "git revert --abort" to cancel the revert operation)
>>
>> nothing to commit, working tree clean
>> /tmp$ git revert --continue
>> ## EDITOR pops up, as expected
>> [master b8dd23f61d07] Revert "whizzbang"
>>   1 file changed, 1 deletion(-)
>> On branch master
>> Revert currently in progress.
>>
>> nothing to commit, working tree clean
>> ## Unexpected state; both reverts happened, but revert is still in progress?
>> /tmp$ git log --oneline
>> b8dd23f61d07 (HEAD -> master) Revert "whizzbang"
>> 586469974ec2 Revert "nothing"
>> 7153607b11e0 nothing
>> 51753222dd9a whizzbang
>> a388f78d9013 baz
>> /tmp$ git status
>> On branch master
>> Revert currently in progress.
>>    (run "git revert --continue" to continue)
>>    (use "git revert --skip" to skip this patch)
>>    (use "git revert --abort" to cancel the revert operation)
>>
>> nothing to commit, working tree clean
>> ## OK...I'll run it one more time.
>> /tmp$ git revert --continue
>> /tmp$ git status
>> On branch master
>> nothing to commit, working tree clean
>> ## *Now* I'm done
>> /tmp$ git log --oneline
>> b8dd23f61d07 (HEAD -> master) Revert "whizzbang"
>> 586469974ec2 Revert "nothing"
>> 7153607b11e0 nothing
>> 51753222dd9a whizzbang
>> a388f78d9013 baz

  reply	other threads:[~2019-11-23  9:54 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-22 23:10 git 2.24: git revert <commit1> <commit2> requires extra '--continue'? Brian Norris
2019-11-23  0:34 ` SZEDER Gábor
2019-11-23  9:53   ` Phillip Wood [this message]
2019-11-23 17:20     ` [PATCH] sequencer: don't re-read todo for revert and cherry-pick SZEDER Gábor
2019-11-23 21:14       ` Johannes Schindelin
2019-11-24  4:49       ` Junio C Hamano
2019-11-24 10:44         ` Phillip Wood
2019-11-24 21:10           ` [PATCH] t3429: try to protect against a potential racy todo file problem SZEDER Gábor
2019-11-25  1:28             ` Junio C Hamano
2019-11-25  3:10             ` SZEDER Gábor
2019-11-25 13:18             ` SZEDER Gábor
2019-11-25 14:43               ` Phillip Wood
2019-11-25 15:15                 ` SZEDER Gábor
2019-11-25 16:40                   ` Phillip Wood
2019-11-25  1:10           ` [PATCH] sequencer: don't re-read todo for revert and cherry-pick Junio C Hamano
2019-11-25 10:47             ` Phillip Wood

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e7c01e0f-8466-c2c5-b53a-a93f941dfb1c@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=briannorris@chromium.org \
    --cc=git@vger.kernel.org \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).