threads / patch / 59923

v2fix cherry-pick/revert status when doing multiple commits

Subject: [PATCH v2] fix cherry-pick/revert status when doing multiple commits

## tl;dr

3 messages between Jun 27, 2023 and Jun 28, 2023. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

Jacob Keller· Jun 27, 2023, 22:41 UTC · lore
From: Jacob Keller <jacob.keller@gmail.com>

The status report for an in-progress cherry-pick does not show the current commit if the cherry-pick happens as part of a series of multiple commits:

 $ git cherry-pick <commit1> <commit2>
 < one of the cherry-picks fails to merge clean >
 Cherry-pick currently in progress.
  (run "git cherry-pick --continue" to continue)
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)
 $ git status
 On branch <branch>
 Your branch is ahead of '<upstream>' by 1 commit.
   (use "git push" to publish your local commits)
 Cherry-pick currently in progress.
   (run "git cherry-pick --continue" to continue)
   (use "git cherry-pick --skip" to skip this patch)
   (use "git cherry-pick --abort" to cancel the cherry-pick operation)

The show_cherry_pick_in_progress() function prints "Cherry-pick currently in progress". That function does have a more verbose print based on whether the cherry_pick_head_oid is null or not. If it is not null, then a more helpful message including which commit is actually being picked is displayed.

The introduction of the "Cherry-pick currently in progress" message comes from 4a72486de97b ("fix cherry-pick/revert status after commit", 2019-04-17). This commit modified wt_status_get_state() in order to detect that a cherry-pick was in progress even if the user has used `git commit` in the middle of the sequence.

The check used to detect this is the call to sequencer_get_last_command. If the sequencer indicates that the lass command was a REPLAY_PICK, then the state->cherry_pick_in_progress is set to 1 and the cherry_pick_head_oid is initialized to the null_oid. Similar behavior is done for the case of REPLAY_REVERT.

It happens that this call of sequencer_get_last_command will always report the action even if the user hasn't interrupted anything. Thus, during a range of cherry-picks or reverts, the cherry_pick_head_oid and revert_head_oid will always be overwritten and initialized to the null oid.

This results in status always displaying the terse message which does not include commit information.

Fix this by adding an additional check so that we do not re-initialize the cherry_pick_head_oid or revert_head_oid if we have already set the cherry_pick_in_progress or revert_in_progress bits. This ensures that git status will display the more helpful information when its available. Add a test case covering this behavior.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
Changes since v1:
* add the missing test case that I had locally but forgot to squash in
* use else if as suggested by Phillip
 t/t7512-status-help.sh | 22 ++++++++++++++++++++++
 wt-status.c            |  4 ++--
 2 files changed, 24 insertions(+), 2 deletions(-)
Show changes to 2 files +24 −2

t/t7512-status-help.sh, wt-status.c

diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index 2f16d5787edf..c2ab8a444a83 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -774,6 +774,28 @@ EOF
 	test_cmp expected actual
 '
 
+test_expect_success 'status when cherry-picking multiple commits' '
+	git reset --hard cherry_branch &&
+	test_when_finished "git cherry-pick --abort" &&
+	test_must_fail git cherry-pick cherry_branch_second one_cherry &&
+	TO_CHERRY_PICK=$(git rev-parse --short CHERRY_PICK_HEAD) &&
+	cat >expected <<EOF &&
+On branch cherry_branch
+You are currently cherry-picking commit $TO_CHERRY_PICK.
+  (fix conflicts and run "git cherry-pick --continue")
+  (use "git cherry-pick --skip" to skip this patch)
+  (use "git cherry-pick --abort" to cancel the cherry-pick operation)
+
+Unmerged paths:
+  (use "git add <file>..." to mark resolution)
+	both modified:   main.txt
+
+no changes added to commit (use "git add" and/or "git commit -a")
+EOF
+	git status --untracked-files=no >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'status when cherry-picking after committing conflict resolution' '
 	git reset --hard cherry_branch &&
 	test_when_finished "git cherry-pick --abort" &&
diff --git a/wt-status.c b/wt-status.c
index 068b76ef6d96..8d23ff8ced23 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1790,10 +1790,10 @@ void wt_status_get_state(struct repository *r,
 		oidcpy(&state->revert_head_oid, &oid);
 	}
 	if (!sequencer_get_last_command(r, &action)) {
-		if (action == REPLAY_PICK) {
+		if (action == REPLAY_PICK && !state->cherry_pick_in_progress) {
 			state->cherry_pick_in_progress = 1;
 			oidcpy(&state->cherry_pick_head_oid, null_oid());
-		} else {
+		} else if (action == REPLAY_REVERT && !state->revert_in_progress) {
 			state->revert_in_progress = 1;
 			oidcpy(&state->revert_head_oid, null_oid());
 		}
-- 
2.41.0.1.g9857a21e0017.dirty
Phillip Wood· Jun 28, 2023, 11:11 UTC · re: Jacob Keller · lore

Re: [PATCH v2] fix cherry-pick/revert status when doing multiple commits

Hi Jacob
This version looks good to me
Thanks for re-rolling
Phillip
On 27/06/2023 23:41, Jacob Keller wrote:
Show 117 quoted lines
> From: Jacob Keller <jacob.keller@gmail.com>
> 
> The status report for an in-progress cherry-pick does not show the
> current commit if the cherry-pick happens as part of a series of
> multiple commits:
> 
>   $ git cherry-pick <commit1> <commit2>
>   < one of the cherry-picks fails to merge clean >
>   Cherry-pick currently in progress.
>    (run "git cherry-pick --continue" to continue)
>    (use "git cherry-pick --skip" to skip this patch)
>    (use "git cherry-pick --abort" to cancel the cherry-pick operation)
> 
>   $ git status
>   On branch <branch>
>   Your branch is ahead of '<upstream>' by 1 commit.
>     (use "git push" to publish your local commits)
> 
>   Cherry-pick currently in progress.
>     (run "git cherry-pick --continue" to continue)
>     (use "git cherry-pick --skip" to skip this patch)
>     (use "git cherry-pick --abort" to cancel the cherry-pick operation)
> 
> The show_cherry_pick_in_progress() function prints "Cherry-pick
> currently in progress". That function does have a more verbose print
> based on whether the cherry_pick_head_oid is null or not. If it is not
> null, then a more helpful message including which commit is actually
> being picked is displayed.
> 
> The introduction of the "Cherry-pick currently in progress" message
> comes from 4a72486de97b ("fix cherry-pick/revert status after commit",
> 2019-04-17). This commit modified wt_status_get_state() in order to
> detect that a cherry-pick was in progress even if the user has used `git
> commit` in the middle of the sequence.
> 
> The check used to detect this is the call to sequencer_get_last_command.
> If the sequencer indicates that the lass command was a REPLAY_PICK, then
> the state->cherry_pick_in_progress is set to 1 and the
> cherry_pick_head_oid is initialized to the null_oid. Similar behavior is
> done for the case of REPLAY_REVERT.
> 
> It happens that this call of sequencer_get_last_command will always
> report the action even if the user hasn't interrupted anything. Thus,
> during a range of cherry-picks or reverts, the cherry_pick_head_oid and
> revert_head_oid will always be overwritten and initialized to the null
> oid.
> 
> This results in status always displaying the terse message which does
> not include commit information.
> 
> Fix this by adding an additional check so that we do not re-initialize
> the cherry_pick_head_oid or revert_head_oid if we have already set the
> cherry_pick_in_progress or revert_in_progress bits. This ensures that
> git status will display the more helpful information when its available.
> Add a test case covering this behavior.
> 
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
> 
> Changes since v1:
> * add the missing test case that I had locally but forgot to squash in
> * use else if as suggested by Phillip
> 
>   t/t7512-status-help.sh | 22 ++++++++++++++++++++++
>   wt-status.c            |  4 ++--
>   2 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
> index 2f16d5787edf..c2ab8a444a83 100755
> --- a/t/t7512-status-help.sh
> +++ b/t/t7512-status-help.sh
> @@ -774,6 +774,28 @@ EOF
>   	test_cmp expected actual
>   '
>   
> +test_expect_success 'status when cherry-picking multiple commits' '
> +	git reset --hard cherry_branch &&
> +	test_when_finished "git cherry-pick --abort" &&
> +	test_must_fail git cherry-pick cherry_branch_second one_cherry &&
> +	TO_CHERRY_PICK=$(git rev-parse --short CHERRY_PICK_HEAD) &&
> +	cat >expected <<EOF &&
> +On branch cherry_branch
> +You are currently cherry-picking commit $TO_CHERRY_PICK.
> +  (fix conflicts and run "git cherry-pick --continue")
> +  (use "git cherry-pick --skip" to skip this patch)
> +  (use "git cherry-pick --abort" to cancel the cherry-pick operation)
> +
> +Unmerged paths:
> +  (use "git add <file>..." to mark resolution)
> +	both modified:   main.txt
> +
> +no changes added to commit (use "git add" and/or "git commit -a")
> +EOF
> +	git status --untracked-files=no >actual &&
> +	test_cmp expected actual
> +'
> +
>   test_expect_success 'status when cherry-picking after committing conflict resolution' '
>   	git reset --hard cherry_branch &&
>   	test_when_finished "git cherry-pick --abort" &&
> diff --git a/wt-status.c b/wt-status.c
> index 068b76ef6d96..8d23ff8ced23 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1790,10 +1790,10 @@ void wt_status_get_state(struct repository *r,
>   		oidcpy(&state->revert_head_oid, &oid);
>   	}
>   	if (!sequencer_get_last_command(r, &action)) {
> -		if (action == REPLAY_PICK) {
> +		if (action == REPLAY_PICK && !state->cherry_pick_in_progress) {
>   			state->cherry_pick_in_progress = 1;
>   			oidcpy(&state->cherry_pick_head_oid, null_oid());
> -		} else {
> +		} else if (action == REPLAY_REVERT && !state->revert_in_progress) {
>   			state->revert_in_progress = 1;
>   			oidcpy(&state->revert_head_oid, null_oid());
>   		}
Junio C Hamano· Jun 28, 2023, 16:26 UTC · re: Phillip Wood · lore

Re: [PATCH v2] fix cherry-pick/revert status when doing multiple commits

Phillip Wood <phillip.wood123@gmail.com> writes:
Show 7 quoted lines
> Hi Jacob
>
> This version looks good to me
>
> Thanks for re-rolling
>
> Phillip
Thanks Jacob for writing and Phillip for reviewing.  Queued.

← back to recent threads