[PATCH] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[PATCH] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[ZheNing Hu via GitGitGadget]

From: ZheNing Hu <adlternative@gmail.com>

If we set the value of the environment variable GIT_CHERRY_PICK_HELP
when using `git cherry-pick`, CHERRY_PICK_HEAD will be deleted, then
we will get an error when we try to use `git cherry-pick --continue`
or other cherr-pick command.

So add option action check in print_advice(), we will not remove
CHERRY_PICK_HEAD if we are indeed cherry-picking instead of rebasing.

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
---
    [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP
    
    E.g. We are now in the branch main, we want to cherry-pick commit
    ac346df, then we want to use GIT_CHERRY_PICK_HELP to use customized
    advice:
    
    GIT_CHERRY_PICK_HELP="you should use git cherry-pick --continue after
    resloving the conflict!" git cherry-pick ac346df
    
    then we can see the correct advice message, but after that, if we want
    to use "git cherry-pick --continue" or other cherry-pick commands, an
    error will appear.
    
    So this patch fixes this bug when git cherry-pick is used with
    environment variable GIT_CHERRY_PICK_HELP.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1001%2Fadlternative%2Fcherry-pick-help-fix-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1001/adlternative/cherry-pick-help-fix-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1001

 sequencer.c                     |  5 +++--
 t/t3507-cherry-pick-conflict.sh | 25 ++++++++++++++++++++-----
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 0bec01cf38e..c01b0b9e9c9 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -409,8 +409,9 @@ static void print_advice(struct repository *r, int show_hint,
 		 * (typically rebase --interactive) wants to take care
 		 * of the commit itself so remove CHERRY_PICK_HEAD
 		 */
-		refs_delete_ref(get_main_ref_store(r), "", "CHERRY_PICK_HEAD",
-				NULL, 0);
+		if (opts->action != REPLAY_PICK)
+			refs_delete_ref(get_main_ref_store(r), "", "CHERRY_PICK_HEAD",
+					NULL, 0);
 		return;
 	}
 
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 014001b8f32..70becaf27aa 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -109,14 +109,29 @@ test_expect_success \
 	test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
 '
 
-test_expect_success 'GIT_CHERRY_PICK_HELP suppresses CHERRY_PICK_HEAD' '
-	pristine_detach initial &&
+test_expect_success 'GIT_CHERRY_PICK_HELP respects CHERRY_PICK_HEAD' '
+	git init repo &&
 	(
+		cd repo &&
+		git branch -M main &&
+		echo 1 >file &&
+		git add file &&
+		git commit -m 1 &&
+		echo 2 >file &&
+		git add file &&
+		git commit -m 2 &&
+		git checkout HEAD~ &&
+		echo 3 >file &&
+		git add file &&
+		git commit -m 3 &&
 		GIT_CHERRY_PICK_HELP="and then do something else" &&
 		export GIT_CHERRY_PICK_HELP &&
-		test_must_fail git cherry-pick picked
-	) &&
-	test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
+		test_must_fail git cherry-pick main &&
+		git rev-parse --verify CHERRY_PICK_HEAD >actual &&
+		git rev-parse --verify main >expect &&
+		test_cmp expect actual &&
+		git cherry-pick --abort
+	)
 '
 
 test_expect_success 'git reset clears CHERRY_PICK_HEAD' '

base-commit: daab8a564f8bbac55f70f8bf86c070e001a9b006
-- 
gitgitgadget

Re: [PATCH] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[Junio C Hamano]

"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: ZheNing Hu <adlternative@gmail.com>
>
> If we set the value of the environment variable GIT_CHERRY_PICK_HELP
> when using `git cherry-pick`, CHERRY_PICK_HEAD will be deleted, then
> we will get an error when we try to use `git cherry-pick --continue`
> or other cherr-pick command.

I think that the GIT_CHERRY_PICK_HELP is an implemention detail for
various forms of rebase to use cherry-pick as a backend and not for
use by end users.  

I strongly suspect that the right solution for the breakage is to
unsetenv(GIT_CHERRY_PICK_HELP) in cmd_cherry_pick() without touching
sequencer.c at all.

It _is_ ugly that a helper that is responsible for emitting an
advise message also makes a decision whether the pseudo-ref gets
deleted or not, but a fix to that problem should be done byy making
the logic for the decision less complex, not more.

So, I am not enthused to see this change.

Re: [PATCH] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[ZheNing Hu]

Junio C Hamano <gitster@pobox.com> 于2021年7月23日周五 上午5:25写道：
>
> "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: ZheNing Hu <adlternative@gmail.com>
> >
> > If we set the value of the environment variable GIT_CHERRY_PICK_HELP
> > when using `git cherry-pick`, CHERRY_PICK_HEAD will be deleted, then
> > we will get an error when we try to use `git cherry-pick --continue`
> > or other cherr-pick command.
>
> I think that the GIT_CHERRY_PICK_HELP is an implemention detail for
> various forms of rebase to use cherry-pick as a backend and not for
> use by end users.
>

But someone complain to me that the cherry-pick advice is not good enough.
Think about a git newbie is cherry-picking a patch series containing
several commits,
E.g.

git cherry-pick dev~3..dev

And then he (she) will see these advice info:
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
hint: and commit the result with 'git commit'

After he resolving git conflict, execute 'git commit' according to the
prompt, A terrible thing happened: CHERRY_PICK_HEAD is deleted
by git and no errors are output. But in fact .git/sequencer still exists,
Wait until he uses the cherry-pick command next time, the error appears:

error: cherry-pick is already in progress
hint: try "git cherry-pick (--continue | --abort | --quit)"
fatal: cherry-pick failed

So we should not encourage users to use git commit when git cherry-pick.
It would be great if it could provide advice similar to rebase, like this:

Once you are satisfied with your changes, run

  git cherry-pick --continue

> I strongly suspect that the right solution for the breakage is to
> unsetenv(GIT_CHERRY_PICK_HELP) in cmd_cherry_pick() without touching
> sequencer.c at all.
>
> It _is_ ugly that a helper that is responsible for emitting an
> advise message also makes a decision whether the pseudo-ref gets
> deleted or not, but a fix to that problem should be done byy making
> the logic for the decision less complex, not more.
>

May be you are right :)

> So, I am not enthused to see this change.

Thanks.
--
ZheNing Hu

Re: [PATCH] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[Felipe Contreras]

ZheNing Hu wrote:
> Junio C Hamano <gitster@pobox.com> 于2021年7月23日周五 上午5:25写道：
> >
> > "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
> >
> > > From: ZheNing Hu <adlternative@gmail.com>
> > >
> > > If we set the value of the environment variable GIT_CHERRY_PICK_HELP
> > > when using `git cherry-pick`, CHERRY_PICK_HEAD will be deleted, then
> > > we will get an error when we try to use `git cherry-pick --continue`
> > > or other cherr-pick command.
> >
> > I think that the GIT_CHERRY_PICK_HELP is an implemention detail for
> > various forms of rebase to use cherry-pick as a backend and not for
> > use by end users.
> >
> 
> But someone complain to me that the cherry-pick advice is not good enough.

I agree it's not good enough.

In my opinion `git cherry-pick` should be a more prominent command, and
a lot functionality is missing.

> Think about a git newbie is cherry-picking a patch series containing
> several commits,
> E.g.
> 
> git cherry-pick dev~3..dev
> 
> And then he (she) will see these advice info:
> hint: after resolving the conflicts, mark the corrected paths
> hint: with 'git add <paths>' or 'git rm <paths>'
> hint: and commit the result with 'git commit'
> 
> After he resolving git conflict, execute 'git commit' according to the
> prompt, A terrible thing happened: CHERRY_PICK_HEAD is deleted
> by git and no errors are output. But in fact .git/sequencer still exists,
> Wait until he uses the cherry-pick command next time, the error appears:
> 
> error: cherry-pick is already in progress
> hint: try "git cherry-pick (--continue | --abort | --quit)"
> fatal: cherry-pick failed
> 
> So we should not encourage users to use git commit when git cherry-pick.
> It would be great if it could provide advice similar to rebase, like this:
> 
> Once you are satisfied with your changes, run
> 
>   git cherry-pick --continue

Agreed.

-- 
Felipe Contreras

[PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[ZheNing Hu via GitGitGadget]

From: ZheNing Hu <adlternative@gmail.com>

If we set the value of the environment variable GIT_CHERRY_PICK_HELP
when using `git cherry-pick`, CHERRY_PICK_HEAD will be deleted, then
we will get an error when we try to use `git cherry-pick --continue`
or other cherr-pick command.

So unsetenv(GIT_CHERRY_PICK_HELP) in cmd_cherry_pick(), to avoid
deleting CHERRY_PICK_HEAD when we are truly cherry-picking, which can
fix this breakage.

Signed-off-by: ZheNing Hu <adlternative@gmail.com>
Mentored-by: Christian Couder <christian.couder@gmail.com>
Mentored-by Hariom Verma <hariom18599@gmail.com>:
---
    [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP
    
    This patch fixes the bug when git cherry-pick is used with environment
    variable GIT_CHERRY_PICK_HELP.
    
    Change from last version:
    
     1. Only unsetenv(GIT_CHERRY_PICK_HELP) without touching anything in
        sequencer.c. Now git cherry-pick will ignore GIT_CHERRY_PICK_HELP,
    
    $ GIT_CHERRY_PICK_HELP="213" git cherry-pick dev~3..dev
    
    will only output default advice:
    
    hint: after resolving the conflicts, mark the corrected paths hint: with
    'git add ' or 'git rm ' hint: and commit the result with 'git commit'
    
    This may still not be good enough, hope that cherry-pick will not advice
    anything related to "git commit". Maybe we should make --no-commit as
    cherry-pick default behavior?

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1001%2Fadlternative%2Fcherry-pick-help-fix-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1001/adlternative/cherry-pick-help-fix-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1001

Range-diff vs v1:

 1:  c2a6a625ac8 ! 1:  fbb9c166502 [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP
     @@ Commit message
          we will get an error when we try to use `git cherry-pick --continue`
          or other cherr-pick command.
      
     -    So add option action check in print_advice(), we will not remove
     -    CHERRY_PICK_HEAD if we are indeed cherry-picking instead of rebasing.
     +    So unsetenv(GIT_CHERRY_PICK_HELP) in cmd_cherry_pick(), to avoid
     +    deleting CHERRY_PICK_HEAD when we are truly cherry-picking, which can
     +    fix this breakage.
      
          Signed-off-by: ZheNing Hu <adlternative@gmail.com>
     +    Mentored-by: Christian Couder <christian.couder@gmail.com>
     +    Mentored-by Hariom Verma <hariom18599@gmail.com>:
      
     - ## sequencer.c ##
     -@@ sequencer.c: static void print_advice(struct repository *r, int show_hint,
     - 		 * (typically rebase --interactive) wants to take care
     - 		 * of the commit itself so remove CHERRY_PICK_HEAD
     - 		 */
     --		refs_delete_ref(get_main_ref_store(r), "", "CHERRY_PICK_HEAD",
     --				NULL, 0);
     -+		if (opts->action != REPLAY_PICK)
     -+			refs_delete_ref(get_main_ref_store(r), "", "CHERRY_PICK_HEAD",
     -+					NULL, 0);
     - 		return;
     - 	}
     + ## builtin/revert.c ##
     +@@ builtin/revert.c: int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
       
     + 	opts.action = REPLAY_PICK;
     + 	sequencer_init_config(&opts);
     ++	unsetenv("GIT_CHERRY_PICK_HELP");
     + 	res = run_sequencer(argc, argv, &opts);
     + 	if (res < 0)
     + 		die(_("cherry-pick failed"));
      
       ## t/t3507-cherry-pick-conflict.sh ##
     +@@ t/t3507-cherry-pick-conflict.sh: test_expect_success 'advice from failed cherry-pick --no-commit' "
     + 	test_cmp expected actual
     + "
     + 
     ++test_expect_success 'advice from failed cherry-pick with GIT_CHERRY_PICK_HELP' "
     ++	pristine_detach initial &&
     ++	(
     ++		picked=\$(git rev-parse --short picked) &&
     ++		cat <<-EOF >expected &&
     ++		error: could not apply \$picked... picked
     ++		hint: after resolving the conflicts, mark the corrected paths
     ++		hint: with 'git add <paths>' or 'git rm <paths>'
     ++		hint: and commit the result with 'git commit'
     ++		EOF
     ++		GIT_CHERRY_PICK_HELP='and then do something else' &&
     ++		export GIT_CHERRY_PICK_HELP &&
     ++		test_must_fail git cherry-pick picked 2>actual &&
     ++		test_cmp expected actual
     ++	)
     ++"
     ++
     + test_expect_success 'failed cherry-pick sets CHERRY_PICK_HEAD' '
     + 	pristine_detach initial &&
     + 	test_must_fail git cherry-pick picked &&
      @@ t/t3507-cherry-pick-conflict.sh: test_expect_success \
       	test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
       '
       
      -test_expect_success 'GIT_CHERRY_PICK_HELP suppresses CHERRY_PICK_HEAD' '
      -	pristine_detach initial &&
     -+test_expect_success 'GIT_CHERRY_PICK_HELP respects CHERRY_PICK_HEAD' '
     -+	git init repo &&
     - 	(
     -+		cd repo &&
     -+		git branch -M main &&
     -+		echo 1 >file &&
     -+		git add file &&
     -+		git commit -m 1 &&
     -+		echo 2 >file &&
     -+		git add file &&
     -+		git commit -m 2 &&
     -+		git checkout HEAD~ &&
     -+		echo 3 >file &&
     -+		git add file &&
     -+		git commit -m 3 &&
     - 		GIT_CHERRY_PICK_HELP="and then do something else" &&
     - 		export GIT_CHERRY_PICK_HELP &&
     +-	(
     +-		GIT_CHERRY_PICK_HELP="and then do something else" &&
     +-		export GIT_CHERRY_PICK_HELP &&
      -		test_must_fail git cherry-pick picked
      -	) &&
      -	test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
     -+		test_must_fail git cherry-pick main &&
     -+		git rev-parse --verify CHERRY_PICK_HEAD >actual &&
     -+		git rev-parse --verify main >expect &&
     -+		test_cmp expect actual &&
     -+		git cherry-pick --abort
     -+	)
     - '
     - 
     +-'
     +-
       test_expect_success 'git reset clears CHERRY_PICK_HEAD' '
     + 	pristine_detach initial &&
     + 


 builtin/revert.c                |  1 +
 t/t3507-cherry-pick-conflict.sh | 27 +++++++++++++++++----------
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/builtin/revert.c b/builtin/revert.c
index 237f2f18d4c..ec0abe7db73 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -245,6 +245,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
 
 	opts.action = REPLAY_PICK;
 	sequencer_init_config(&opts);
+	unsetenv("GIT_CHERRY_PICK_HELP");
 	res = run_sequencer(argc, argv, &opts);
 	if (res < 0)
 		die(_("cherry-pick failed"));
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index 014001b8f32..6f8035399d9 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -76,6 +76,23 @@ test_expect_success 'advice from failed cherry-pick --no-commit' "
 	test_cmp expected actual
 "
 
+test_expect_success 'advice from failed cherry-pick with GIT_CHERRY_PICK_HELP' "
+	pristine_detach initial &&
+	(
+		picked=\$(git rev-parse --short picked) &&
+		cat <<-EOF >expected &&
+		error: could not apply \$picked... picked
+		hint: after resolving the conflicts, mark the corrected paths
+		hint: with 'git add <paths>' or 'git rm <paths>'
+		hint: and commit the result with 'git commit'
+		EOF
+		GIT_CHERRY_PICK_HELP='and then do something else' &&
+		export GIT_CHERRY_PICK_HELP &&
+		test_must_fail git cherry-pick picked 2>actual &&
+		test_cmp expected actual
+	)
+"
+
 test_expect_success 'failed cherry-pick sets CHERRY_PICK_HEAD' '
 	pristine_detach initial &&
 	test_must_fail git cherry-pick picked &&
@@ -109,16 +126,6 @@ test_expect_success \
 	test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
 '
 
-test_expect_success 'GIT_CHERRY_PICK_HELP suppresses CHERRY_PICK_HEAD' '
-	pristine_detach initial &&
-	(
-		GIT_CHERRY_PICK_HELP="and then do something else" &&
-		export GIT_CHERRY_PICK_HELP &&
-		test_must_fail git cherry-pick picked
-	) &&
-	test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
-'
-
 test_expect_success 'git reset clears CHERRY_PICK_HEAD' '
 	pristine_detach initial &&
 

base-commit: daab8a564f8bbac55f70f8bf86c070e001a9b006
-- 
gitgitgadget

Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[Phillip Wood]

On 24/07/2021 15:01, ZheNing Hu via GitGitGadget wrote:
> From: ZheNing Hu <adlternative@gmail.com>
> 
> If we set the value of the environment variable GIT_CHERRY_PICK_HELP
> when using `git cherry-pick`, CHERRY_PICK_HEAD will be deleted, then
> we will get an error when we try to use `git cherry-pick --continue`
> or other cherr-pick command.
> 
> So unsetenv(GIT_CHERRY_PICK_HELP) in cmd_cherry_pick(), to avoid
> deleting CHERRY_PICK_HEAD when we are truly cherry-picking, which can
> fix this breakage.
> 
> Signed-off-by: ZheNing Hu <adlternative@gmail.com>
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by Hariom Verma <hariom18599@gmail.com>:
> ---
>      [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP
>      
>      This patch fixes the bug when git cherry-pick is used with environment
>      variable GIT_CHERRY_PICK_HELP.
>      
>      Change from last version:
>      
>       1. Only unsetenv(GIT_CHERRY_PICK_HELP) without touching anything in
>          sequencer.c. Now git cherry-pick will ignore GIT_CHERRY_PICK_HELP,
>      
>      $ GIT_CHERRY_PICK_HELP="213" git cherry-pick dev~3..dev
>      
>      will only output default advice:
>      
>      hint: after resolving the conflicts, mark the corrected paths hint: with
>      'git add ' or 'git rm ' hint: and commit the result with 'git commit'
>      
>      This may still not be good enough, hope that cherry-pick will not advice
>      anything related to "git commit". Maybe we should make --no-commit as
>      cherry-pick default behavior?
> 
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1001%2Fadlternative%2Fcherry-pick-help-fix-v2
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1001/adlternative/cherry-pick-help-fix-v2
> Pull-Request: https://github.com/gitgitgadget/git/pull/1001
> 
> Range-diff vs v1:
> 
>   1:  c2a6a625ac8 ! 1:  fbb9c166502 [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP
>       @@ Commit message
>            we will get an error when we try to use `git cherry-pick --continue`
>            or other cherr-pick command.
>        
>       -    So add option action check in print_advice(), we will not remove
>       -    CHERRY_PICK_HEAD if we are indeed cherry-picking instead of rebasing.
>       +    So unsetenv(GIT_CHERRY_PICK_HELP) in cmd_cherry_pick(), to avoid
>       +    deleting CHERRY_PICK_HEAD when we are truly cherry-picking, which can
>       +    fix this breakage.
>        
>            Signed-off-by: ZheNing Hu <adlternative@gmail.com>
>       +    Mentored-by: Christian Couder <christian.couder@gmail.com>
>       +    Mentored-by Hariom Verma <hariom18599@gmail.com>:
>        
>       - ## sequencer.c ##
>       -@@ sequencer.c: static void print_advice(struct repository *r, int show_hint,
>       - 		 * (typically rebase --interactive) wants to take care
>       - 		 * of the commit itself so remove CHERRY_PICK_HEAD
>       - 		 */
>       --		refs_delete_ref(get_main_ref_store(r), "", "CHERRY_PICK_HEAD",
>       --				NULL, 0);
>       -+		if (opts->action != REPLAY_PICK)
>       -+			refs_delete_ref(get_main_ref_store(r), "", "CHERRY_PICK_HEAD",
>       -+					NULL, 0);
>       - 		return;
>       - 	}
>       + ## builtin/revert.c ##
>       +@@ builtin/revert.c: int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
>         
>       + 	opts.action = REPLAY_PICK;
>       + 	sequencer_init_config(&opts);
>       ++	unsetenv("GIT_CHERRY_PICK_HELP");
>       + 	res = run_sequencer(argc, argv, &opts);
>       + 	if (res < 0)
>       + 		die(_("cherry-pick failed"));
>        
>         ## t/t3507-cherry-pick-conflict.sh ##
>       +@@ t/t3507-cherry-pick-conflict.sh: test_expect_success 'advice from failed cherry-pick --no-commit' "
>       + 	test_cmp expected actual
>       + "
>       +
>       ++test_expect_success 'advice from failed cherry-pick with GIT_CHERRY_PICK_HELP' "
>       ++	pristine_detach initial &&
>       ++	(
>       ++		picked=\$(git rev-parse --short picked) &&
>       ++		cat <<-EOF >expected &&
>       ++		error: could not apply \$picked... picked
>       ++		hint: after resolving the conflicts, mark the corrected paths
>       ++		hint: with 'git add <paths>' or 'git rm <paths>'
>       ++		hint: and commit the result with 'git commit'
>       ++		EOF
>       ++		GIT_CHERRY_PICK_HELP='and then do something else' &&
>       ++		export GIT_CHERRY_PICK_HELP &&
>       ++		test_must_fail git cherry-pick picked 2>actual &&
>       ++		test_cmp expected actual
>       ++	)
>       ++"
>       ++
>       + test_expect_success 'failed cherry-pick sets CHERRY_PICK_HEAD' '
>       + 	pristine_detach initial &&
>       + 	test_must_fail git cherry-pick picked &&
>        @@ t/t3507-cherry-pick-conflict.sh: test_expect_success \
>         	test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
>         '
>         
>        -test_expect_success 'GIT_CHERRY_PICK_HELP suppresses CHERRY_PICK_HEAD' '
>        -	pristine_detach initial &&
>       -+test_expect_success 'GIT_CHERRY_PICK_HELP respects CHERRY_PICK_HEAD' '
>       -+	git init repo &&
>       - 	(
>       -+		cd repo &&
>       -+		git branch -M main &&
>       -+		echo 1 >file &&
>       -+		git add file &&
>       -+		git commit -m 1 &&
>       -+		echo 2 >file &&
>       -+		git add file &&
>       -+		git commit -m 2 &&
>       -+		git checkout HEAD~ &&
>       -+		echo 3 >file &&
>       -+		git add file &&
>       -+		git commit -m 3 &&
>       - 		GIT_CHERRY_PICK_HELP="and then do something else" &&
>       - 		export GIT_CHERRY_PICK_HELP &&
>       +-	(
>       +-		GIT_CHERRY_PICK_HELP="and then do something else" &&
>       +-		export GIT_CHERRY_PICK_HELP &&
>        -		test_must_fail git cherry-pick picked
>        -	) &&
>        -	test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
>       -+		test_must_fail git cherry-pick main &&
>       -+		git rev-parse --verify CHERRY_PICK_HEAD >actual &&
>       -+		git rev-parse --verify main >expect &&
>       -+		test_cmp expect actual &&
>       -+		git cherry-pick --abort
>       -+	)
>       - '
>       -
>       +-'
>       +-
>         test_expect_success 'git reset clears CHERRY_PICK_HEAD' '
>       + 	pristine_detach initial &&
>       +
> 
> 
>   builtin/revert.c                |  1 +
>   t/t3507-cherry-pick-conflict.sh | 27 +++++++++++++++++----------
>   2 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/builtin/revert.c b/builtin/revert.c
> index 237f2f18d4c..ec0abe7db73 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -245,6 +245,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
>   
>   	opts.action = REPLAY_PICK;
>   	sequencer_init_config(&opts);
> +	unsetenv("GIT_CHERRY_PICK_HELP");

This will break git-rebase--preserve-merges.sh which uses 
GIT_CHERRY_PICK_HELP to set the help and ensure CHERRY_PICK_HEAD is 
removed when picking commits. I'm a bit confused as to what the problem 
is - how is 'git cherry-pick' being run with GIT_CHERRY_PICK_HELP set in 
the environment outside of a rebase (your explanation in [1] does not 
mention how GIT_CHERRY_PICK_HELP is set)? As far as I can see 'git 
rebase -i' does not set it so the only case I can think of is 
cherry-picking from an exec command  while running 'git rebase -p'

Best Wishes

Phillip

[1] 
https://lore.kernel.org/git/CAOLTT8Ty47fyY7T3d68CYPKh9k+HAHsnCLJ=F0KaLm+0gp3+EQ@mail.gmail.com/

>   	res = run_sequencer(argc, argv, &opts);
>   	if (res < 0)
>   		die(_("cherry-pick failed"));
> diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
> index 014001b8f32..6f8035399d9 100755
> --- a/t/t3507-cherry-pick-conflict.sh
> +++ b/t/t3507-cherry-pick-conflict.sh
> @@ -76,6 +76,23 @@ test_expect_success 'advice from failed cherry-pick --no-commit' "
>   	test_cmp expected actual
>   "
>   
> +test_expect_success 'advice from failed cherry-pick with GIT_CHERRY_PICK_HELP' "
> +	pristine_detach initial &&
> +	(
> +		picked=\$(git rev-parse --short picked) &&
> +		cat <<-EOF >expected &&
> +		error: could not apply \$picked... picked
> +		hint: after resolving the conflicts, mark the corrected paths
> +		hint: with 'git add <paths>' or 'git rm <paths>'
> +		hint: and commit the result with 'git commit'
> +		EOF
> +		GIT_CHERRY_PICK_HELP='and then do something else' &&
> +		export GIT_CHERRY_PICK_HELP &&
> +		test_must_fail git cherry-pick picked 2>actual &&
> +		test_cmp expected actual
> +	)
> +"
> +
>   test_expect_success 'failed cherry-pick sets CHERRY_PICK_HEAD' '
>   	pristine_detach initial &&
>   	test_must_fail git cherry-pick picked &&
> @@ -109,16 +126,6 @@ test_expect_success \
>   	test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
>   '
>   
> -test_expect_success 'GIT_CHERRY_PICK_HELP suppresses CHERRY_PICK_HEAD' '
> -	pristine_detach initial &&
> -	(
> -		GIT_CHERRY_PICK_HELP="and then do something else" &&
> -		export GIT_CHERRY_PICK_HELP &&
> -		test_must_fail git cherry-pick picked
> -	) &&
> -	test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
> -'
> -
>   test_expect_success 'git reset clears CHERRY_PICK_HEAD' '
>   	pristine_detach initial &&
>   
> 
> base-commit: daab8a564f8bbac55f70f8bf86c070e001a9b006
> 

Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> This will break git-rebase--preserve-merges.sh which uses
> GIT_CHERRY_PICK_HELP to set the help and ensure CHERRY_PICK_HEAD is 
> removed when picking commits.

Ahh, I didn't realize we still had scripted rebase backends that
called cherry-pick as an executable.  I was hoping that all rebase
backends by now would be calling into the cherry-pick machinery
directly, bypassing cmd_cherry_pick(), and that was why I suggested
to catch stray one the end-users set manually in the environment
and clear it there.

> I'm a bit confused as to what the
> problem is - how is 'git cherry-pick' being run with
> GIT_CHERRY_PICK_HELP set in the environment outside of a rebase (your
> explanation in [1] does not mention how GIT_CHERRY_PICK_HELP is set)?

I didn't press for the information too hard, but I guessed that it
was perhaps because somebody like stackoverflow suggested to set a
message in their environment to get a "better message."

Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Phillip Wood <phillip.wood123@gmail.com> writes:
>
>> This will break git-rebase--preserve-merges.sh which uses
>> GIT_CHERRY_PICK_HELP to set the help and ensure CHERRY_PICK_HEAD is 
>> removed when picking commits.
>
> Ahh, I didn't realize we still had scripted rebase backends that
> called cherry-pick as an executable.  I was hoping that all rebase
> backends by now would be calling into the cherry-pick machinery
> directly, bypassing cmd_cherry_pick(), and that was why I suggested
> to catch stray one the end-users set manually in the environment
> and clear it there.
>
>> I'm a bit confused as to what the
>> problem is - how is 'git cherry-pick' being run with
>> GIT_CHERRY_PICK_HELP set in the environment outside of a rebase (your
>> explanation in [1] does not mention how GIT_CHERRY_PICK_HELP is set)?
>
> I didn't press for the information too hard, but I guessed that it
> was perhaps because somebody like stackoverflow suggested to set a
> message in their environment to get a "better message."

A good way forward may be to relieve sequencer.c::print_advice() of
the responsibility of optinally removing CHERRY_PICK_HEAD; make it a
separate function that bases its decision on a more direct cue, not
on the presense of a custom message in GIT_CHERRY_PICK_HELP, make
do_pick_commit(), which is the sole caller of print_advice(), call
it after calling print_advice().

I do not offhand know what that "direct cue" should be, but we may
already have an appropriate field in the replay_opts structure;
"replay.action is neither REVERT nor PICK" could be a good enough
approximation, I dunno.

Otherwise we can allocate a new bit in the structure, have relevant
callers set it, and teach cherry-pick an unadvertised command line
option that sets the bit, and use that option only from
git-rebase--preserve-merges when it makes a call to cherry-pick.
When "rebase -p" is either retired or rewritten in C, we can retire
the option from cherry-pick.

Workable?

Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[Phillip Wood]

On 27/07/2021 22:00, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>>
>>> This will break git-rebase--preserve-merges.sh which uses
>>> GIT_CHERRY_PICK_HELP to set the help and ensure CHERRY_PICK_HEAD is
>>> removed when picking commits.
>>
>> Ahh, I didn't realize we still had scripted rebase backends that
>> called cherry-pick as an executable.  I was hoping that all rebase
>> backends by now would be calling into the cherry-pick machinery
>> directly, bypassing cmd_cherry_pick(), and that was why I suggested
>> to catch stray one the end-users set manually in the environment
>> and clear it there.
>>
>>> I'm a bit confused as to what the
>>> problem is - how is 'git cherry-pick' being run with
>>> GIT_CHERRY_PICK_HELP set in the environment outside of a rebase (your
>>> explanation in [1] does not mention how GIT_CHERRY_PICK_HELP is set)?
>>
>> I didn't press for the information too hard, but I guessed that it
>> was perhaps because somebody like stackoverflow suggested to set a
>> message in their environment to get a "better message."
> 
> A good way forward may be to relieve sequencer.c::print_advice() of
> the responsibility of optinally removing CHERRY_PICK_HEAD; make it a
> separate function that bases its decision on a more direct cue, not
> on the presense of a custom message in GIT_CHERRY_PICK_HELP, make
> do_pick_commit(), which is the sole caller of print_advice(), call
> it after calling print_advice().
> 
> I do not offhand know what that "direct cue" should be, but we may
> already have an appropriate field in the replay_opts structure;
> "replay.action is neither REVERT nor PICK" could be a good enough
> approximation, I dunno.
> 
> Otherwise we can allocate a new bit in the structure, have relevant
> callers set it, and teach cherry-pick an unadvertised command line
> option that sets the bit, and use that option only from
> git-rebase--preserve-merges when it makes a call to cherry-pick.
> When "rebase -p" is either retired or rewritten in C, we can retire
> the option from cherry-pick.
>
> Workable?

Most of the time the builtin rebase should not be writing 
CHERRY_PICK_HEAD in the first place (it needs it when a commit becomes 
empty but not otherwise). For 'rebase -p' adding a command line option 
to cherry-pick as you suggest is probably the cleanest solution - in the 
short term 'rebase -i' could set it until we refactor the code that 
creates CHERRY_PICK_HEAD. One thing to note is that I think 
GIT_CHERRY_PICK_HELP was introduced to allow assist scripted rebase like 
porcelains rather than as a way to allow users to customize the help and 
setting it has always removed CHERRY_PICK_HEAD. There are however no 
users of GIT_CHERRY_PICK_HELP on codesearch.debian.org

Best Wishes

Phillip

Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[ZheNing Hu]

Junio C Hamano <gitster@pobox.com> 于2021年7月28日周三 上午5:00写道：
>
> Junio C Hamano <gitster@pobox.com> writes:
>
> > Phillip Wood <phillip.wood123@gmail.com> writes:
> >
> >> This will break git-rebase--preserve-merges.sh which uses
> >> GIT_CHERRY_PICK_HELP to set the help and ensure CHERRY_PICK_HEAD is
> >> removed when picking commits.
> >
> > Ahh, I didn't realize we still had scripted rebase backends that
> > called cherry-pick as an executable.  I was hoping that all rebase
> > backends by now would be calling into the cherry-pick machinery
> > directly, bypassing cmd_cherry_pick(), and that was why I suggested
> > to catch stray one the end-users set manually in the environment
> > and clear it there.
> >
> >> I'm a bit confused as to what the
> >> problem is - how is 'git cherry-pick' being run with
> >> GIT_CHERRY_PICK_HELP set in the environment outside of a rebase (your
> >> explanation in [1] does not mention how GIT_CHERRY_PICK_HELP is set)?
> >
> > I didn't press for the information too hard, but I guessed that it
> > was perhaps because somebody like stackoverflow suggested to set a
> > message in their environment to get a "better message."
>
> A good way forward may be to relieve sequencer.c::print_advice() of
> the responsibility of optinally removing CHERRY_PICK_HEAD; make it a
> separate function that bases its decision on a more direct cue, not
> on the presense of a custom message in GIT_CHERRY_PICK_HELP, make
> do_pick_commit(), which is the sole caller of print_advice(), call
> it after calling print_advice().
>

I think this function "check_need_delete_cherry_pick_head()" should be before
print_advice(), like this:

+               const char *help_msgs = NULL;
+
                error(command == TODO_REVERT
                      ? _("could not revert %s... %s")
                      : _("could not apply %s... %s"),
                      short_commit_name(commit), msg.subject);
-               print_advice(r, res == 1, opts);
+               if (((opts->action == REPLAY_PICK &&
+                     !opts->rebase_preserve_merges_mode) ||
+                     (help_msgs = check_need_delete_cherry_pick_head(r))) &&
+                     res == 1)
+                       print_advice(opts, help_msgs);

> I do not offhand know what that "direct cue" should be, but we may
> already have an appropriate field in the replay_opts structure;
> "replay.action is neither REVERT nor PICK" could be a good enough
> approximation, I dunno.
>
> Otherwise we can allocate a new bit in the structure, have relevant
> callers set it, and teach cherry-pick an unadvertised command line
> option that sets the bit, and use that option only from
> git-rebase--preserve-merges when it makes a call to cherry-pick.
> When "rebase -p" is either retired or rewritten in C, we can retire
> the option from cherry-pick.
>

I think this one can be easily achieved.

> Workable?

Thanks.
--
ZheNing Hu

Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[Junio C Hamano]

ZheNing Hu <adlternative@gmail.com> writes:

> I think this function "check_need_delete_cherry_pick_head()" should be before
> print_advice(), like this:
>
> +               const char *help_msgs = NULL;
> +
>                 error(command == TODO_REVERT
>                       ? _("could not revert %s... %s")
>                       : _("could not apply %s... %s"),
>                       short_commit_name(commit), msg.subject);
> -               print_advice(r, res == 1, opts);
> +               if (((opts->action == REPLAY_PICK &&
> +                     !opts->rebase_preserve_merges_mode) ||
> +                     (help_msgs = check_need_delete_cherry_pick_head(r))) &&
> +                     res == 1)
> +                       print_advice(opts, help_msgs);

Sorry, but I am not sure what problem this separation is trying to
solve.

The root cause of the issue we have, I think, is because the
decision to delete or keep the cherry-pick-head pseudoref is tied to
what message we give to users in the current code, and the suggested
split of concern is to limit print_advice() to only print the advice
message, and a new helper to decide and remove the pseudoref,
without relying on what is in the GIT_CHERRY_PICK_HELP environment.

It is unclear where you are making the decision to keep or remove
the pseudoref in the above arrangement that lets the new
check_need_delete_cherry_pick_head() decide if the advice is printed
or not.

Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[ZheNing Hu]

Junio C Hamano <gitster@pobox.com> 于2021年7月28日周三 上午5:00写道：

> Otherwise we can allocate a new bit in the structure, have relevant
> callers set it, and teach cherry-pick an unadvertised command line
> option that sets the bit, and use that option only from
> git-rebase--preserve-merges when it makes a call to cherry-pick.
> When "rebase -p" is either retired or rewritten in C, we can retire
> the option from cherry-pick.
>

Just use a PARSE_OPT_HIDDEN cannot prevent the user from using
the option... Is there a better way?

--
ZheNing Hu

Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[Junio C Hamano]

ZheNing Hu <adlternative@gmail.com> writes:

> Just use a PARSE_OPT_HIDDEN cannot prevent the user from using
> the option... Is there a better way?

If a command can be invoked from a script, you can invoke it
interactively the same way.  Not advertising on short help, not
completing in the completion and documenting it for internal use is
the standard arrangement for such "implementation detail only"
options.

Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[ZheNing Hu]

Phillip Wood <phillip.wood123@gmail.com> 于2021年7月28日周三 上午3:43写道：
>
> > diff --git a/builtin/revert.c b/builtin/revert.c
> > index 237f2f18d4c..ec0abe7db73 100644
> > --- a/builtin/revert.c
> > +++ b/builtin/revert.c
> > @@ -245,6 +245,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
> >
> >       opts.action = REPLAY_PICK;
> >       sequencer_init_config(&opts);
> > +     unsetenv("GIT_CHERRY_PICK_HELP");
>
> This will break git-rebase--preserve-merges.sh which uses
> GIT_CHERRY_PICK_HELP to set the help and ensure CHERRY_PICK_HEAD is
> removed when picking commits. I'm a bit confused as to what the problem

Yeah, I thought it would call some rebase-related code before, I
didn’t expect it to
call cherry-pick. On the other hand, I passed all tests, so I ignore
it, there should be
a test for it.

> is - how is 'git cherry-pick' being run with GIT_CHERRY_PICK_HELP set in
> the environment outside of a rebase (your explanation in [1] does not
> mention how GIT_CHERRY_PICK_HELP is set)? As far as I can see 'git
> rebase -i' does not set it so the only case I can think of is
> cherry-picking from an exec command  while running 'git rebase -p'
>

Ah, because I want to find a way to suppress its advice messages about
"git commit",
and I don’t think anyone else is using this "feature".

> Best Wishes
>
> Phillip
>
> [1]
> https://lore.kernel.org/git/CAOLTT8Ty47fyY7T3d68CYPKh9k+HAHsnCLJ=F0KaLm+0gp3+EQ@mail.gmail.com/
>

Thanks.
--
ZheNing Hu

Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[Phillip Wood]

Hi ZheNing

On 28/07/2021 08:39, ZheNing Hu wrote:
> Phillip Wood <phillip.wood123@gmail.com> 于2021年7月28日周三 上午3:43写道：
>>
>>> diff --git a/builtin/revert.c b/builtin/revert.c
>>> index 237f2f18d4c..ec0abe7db73 100644
>>> --- a/builtin/revert.c
>>> +++ b/builtin/revert.c
>>> @@ -245,6 +245,7 @@ int cmd_cherry_pick(int argc, const char **argv, const char *prefix)
>>>
>>>        opts.action = REPLAY_PICK;
>>>        sequencer_init_config(&opts);
>>> +     unsetenv("GIT_CHERRY_PICK_HELP");
>>
>> This will break git-rebase--preserve-merges.sh which uses
>> GIT_CHERRY_PICK_HELP to set the help and ensure CHERRY_PICK_HEAD is
>> removed when picking commits. I'm a bit confused as to what the problem
> 
> Yeah, I thought it would call some rebase-related code before, I
> didn’t expect it to
> call cherry-pick. On the other hand, I passed all tests, so I ignore
> it, there should be
> a test for it.
> 
>> is - how is 'git cherry-pick' being run with GIT_CHERRY_PICK_HELP set in
>> the environment outside of a rebase (your explanation in [1] does not
>> mention how GIT_CHERRY_PICK_HELP is set)? As far as I can see 'git
>> rebase -i' does not set it so the only case I can think of is
>> cherry-picking from an exec command  while running 'git rebase -p'
>>
> 
> Ah, because I want to find a way to suppress its advice messages about
> "git commit",
> and I don’t think anyone else is using this "feature".

I'd welcome a patch to improve the advice. I suspect the current advice 
predates the introduction of the '--continue' flag for cherry-pick. I 
think that would be a better route forward as it would benefit all 
users. Setting GIT_CHERRY_PICK_HELP is undocumented and has always 
removed CHERRY_PICK_HEAD since CHERRY_PICK_HEAD was introduced in commit
7e5c0cbf (Introduce CHERRY_PICK_HEAD, 2011-02-19).

Best Wishes

Phillip


>> Best Wishes
>>
>> Phillip
>>
>> [1]
>> https://lore.kernel.org/git/CAOLTT8Ty47fyY7T3d68CYPKh9k+HAHsnCLJ=F0KaLm+0gp3+EQ@mail.gmail.com/
>>
> 
> Thanks.
> --
> ZheNing Hu
> 

Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[ZheNing Hu]

Phillip Wood <phillip.wood123@gmail.com> 于2021年7月28日周三 下午5:46写道：
>
> Hi ZheNing
>
> > Ah, because I want to find a way to suppress its advice messages about
> > "git commit",
> > and I don’t think anyone else is using this "feature".
>
> I'd welcome a patch to improve the advice. I suspect the current advice
> predates the introduction of the '--continue' flag for cherry-pick. I
> think that would be a better route forward as it would benefit all
> users. Setting GIT_CHERRY_PICK_HELP is undocumented and has always
> removed CHERRY_PICK_HEAD since CHERRY_PICK_HEAD was introduced in commit
> 7e5c0cbf (Introduce CHERRY_PICK_HEAD, 2011-02-19).
>

After I modify the interface of print_advice() in the way suggested by
Junio, I can provide a
help_msg parameter for print_advice(), and maybe we can use it to
provide better advice later.
Something like this:

+static void print_advice(struct replay_opts *opts, const char *help_msgs)
+{
+       if (help_msgs)
+               fprintf(stderr, "%s\n", help_msgs);
+       else if (opts->no_commit)
+               advise(_("after resolving the conflicts, mark the
corrected paths\n"
+                        "with 'git add <paths>' or 'git rm <paths>'"));
+       else
+               advise(_("after resolving the conflicts, mark the
corrected paths\n"
+                        "with 'git add <paths>' or 'git rm <paths>'\n"
+                        "and commit the result with 'git commit'"));
 }

> Best Wishes
>
> Phillip
>

Thanks.
--
ZheNing Hu

Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[Phillip Wood]

On 28/07/2021 12:01, ZheNing Hu wrote:
> Phillip Wood <phillip.wood123@gmail.com> 于2021年7月28日周三 下午5:46写道：
>>
>> Hi ZheNing
>>
>>> Ah, because I want to find a way to suppress its advice messages about
>>> "git commit",
>>> and I don’t think anyone else is using this "feature".
>>
>> I'd welcome a patch to improve the advice. I suspect the current advice
>> predates the introduction of the '--continue' flag for cherry-pick. I
>> think that would be a better route forward as it would benefit all
>> users. Setting GIT_CHERRY_PICK_HELP is undocumented and has always
>> removed CHERRY_PICK_HEAD since CHERRY_PICK_HEAD was introduced in commit
>> 7e5c0cbf (Introduce CHERRY_PICK_HEAD, 2011-02-19).
>>
> 
> After I modify the interface of print_advice() in the way suggested by
> Junio, I can provide a
> help_msg parameter for print_advice(), and maybe we can use it to
> provide better advice later.

I think that we could change the advice now to suggest using 'git 
cherry-pick --continue' instead of running 'git commit'. I think that in 
the long term we want to move away from being able to customize the 
advice when 'git rebase -p' is retired.

Best Wishes

Phillip

> Something like this:
> 
> +static void print_advice(struct replay_opts *opts, const char *help_msgs)
> +{
> +       if (help_msgs)
> +               fprintf(stderr, "%s\n", help_msgs);
> +       else if (opts->no_commit)
> +               advise(_("after resolving the conflicts, mark the
> corrected paths\n"
> +                        "with 'git add <paths>' or 'git rm <paths>'"));
> +       else
> +               advise(_("after resolving the conflicts, mark the
> corrected paths\n"
> +                        "with 'git add <paths>' or 'git rm <paths>'\n"
> +                        "and commit the result with 'git commit'"));
>   }
> 
>> Best Wishes
>>
>> Phillip
>>
> 
> Thanks.
> --
> ZheNing Hu
>