From: Phillip Wood Date: Wed, 28 Jun 2023 11:11:03 GMT Subject: Re: [PATCH v2] fix cherry-pick/revert status when doing multiple commits Message-ID: <743b17ee-2c5b-be7e-70f3-76d0f9d0ff5e@gmail.com> In-Reply-To: <20230627224230.1951135-1-jacob.e.keller@intel.com> Hi Jacob This version looks good to me Thanks for re-rolling Phillip On 27/06/2023 23:41, Jacob Keller wrote: > From: Jacob Keller > > 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 > < 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 > Your branch is ahead of '' 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 > --- > > 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 < +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 ..." 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()); > }