git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Christian Couder" <christian.couder@gmail.com>,
	"Hariom Verma" <hariom18599@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Han-Wen Nienhuys" <hanwen@google.com>,
	"Ramkumar Ramachandra" <artagnon@gmail.com>,
	"Felipe Contreras" <felipe.contreras@gmail.com>,
	"ZheNing Hu" <adlternative@gmail.com>,
	"ZheNing Hu" <adlternative@gmail.com>
Subject: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP
Date: Sat, 24 Jul 2021 14:01:21 +0000	[thread overview]
Message-ID: <pull.1001.v2.git.1627135281887.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1001.git.1626962763373.gitgitgadget@gmail.com>

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

  parent reply	other threads:[~2021-07-24 14:01 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-22 14:06 [PATCH] " ZheNing Hu via GitGitGadget
2021-07-22 21:25 ` Junio C Hamano
2021-07-23  9:37   ` ZheNing Hu
2021-07-23 17:01     ` Felipe Contreras
2021-07-24 14:01 ` ZheNing Hu via GitGitGadget [this message]
2021-07-27 19:43   ` [PATCH v2] " Phillip Wood
2021-07-27 20:44     ` Junio C Hamano
2021-07-27 21:00       ` Junio C Hamano
2021-07-28  9:56         ` Phillip Wood
2021-07-28 10:56         ` ZheNing Hu
2021-07-28 17:24           ` Junio C Hamano
2021-07-28 11:34         ` ZheNing Hu
2021-07-28 17:26           ` Junio C Hamano
2021-07-28  7:39     ` ZheNing Hu
2021-07-28  9:46       ` Phillip Wood
2021-07-28 11:01         ` ZheNing Hu
2021-07-28 16:52           ` 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

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

  git send-email \
    --in-reply-to=pull.1001.v2.git.1627135281887.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=adlternative@gmail.com \
    --cc=artagnon@gmail.com \
    --cc=avarab@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=hanwen@google.com \
    --cc=hariom18599@gmail.com \
    --subject='Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP' \
    /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

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 NNTP newsgroup(s).