Subject: [PATCH v2 0/2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

From: "ZheNing Hu via GitGitGadget" <[email-masked]>
Date: Tue, 03 Aug 2021 01:16:20 +0000

This patch fixes the bug when git cherry-pick is used with environment
variable GIT_CHERRY_PICK_HELP, and makes git chery-pick advice message
better.

v4:
https://lore.kernel.org/git/[email-masked]/

v4-->v5:

 1. Delete struct rebase_options member delete_cherry_pick_head, and set the
    delete_cherry_pick_head in struct replay_opts replay to 1 in
    get_replay_opts().
 2. Avoid too long line length of advice of cherry-pick, split "git
    cherry-pick --continue" in advice to new line.

ZheNing Hu (2):
  [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP
  [GSOC] cherry-pick: use better advice message

 builtin/rebase.c                |  1 +
 builtin/revert.c                |  2 ++
 git-rebase--preserve-merges.sh  |  2 +-
 sequencer.c                     | 38 +++++++++++++++-----------
 sequencer.h                     |  1 +
 t/t3507-cherry-pick-conflict.sh | 48 ++++++++++++++++++++++-----------
 6 files changed, 60 insertions(+), 32 deletions(-)

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

Range-diff vs v1:

 1:  0d0a55bd9c4 ! 1:  5d2fd55c580 [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP
     @@ Commit message
          conflict occurs, which provided for some porcelain commands of git like
          `git-rebase--preserve-merges.sh`. After `git rebase -p` completely
          abolished, this option should be removed. At the same time, add the flag
     -    `delete_cherry_pick_head` to `struct rebase_options` and
     -    `struct replay_opts`, We can decide whether to delete CHERRY_PICK_HEAD
     -    by setting, passing, and checking this flag bit.
     +    `delete_cherry_pick_head` to `struct replay_opts`, We can decide whether
     +    to delete CHERRY_PICK_HEAD by setting and checking this flag bit.
      
          Then we split print_advice() into two part: Firstly, print_advice()
          will only be responsible for outputting content; Secondly, check if
     @@ Commit message
          In this way, the steps of printing advice and deleting CHERRY_PICK_HEAD
          are decoupled. Finally, let `git-rebase--preserve-merges.sh` use the
          `--delete-cherry-pick-head` option when it executes git cherry-pick, and
     -    set the `delete_cherry_pick_head` flag in run_specific_rebase() when we
     +    set the `delete_cherry_pick_head` flag in get_replay_opts() when we
          are using `git rebase --merge`, which can fix this breakage.
      
     +    It is worth mentioning that now we use advice() to print the content
     +    of GIT_CHERRY_PICK_HELP in print_advice(), each line of output will
     +    start with "hint: ".
     +
          Mentored-by: Christian Couder <[email-masked]>
          Mentored-by Hariom Verma <[email-masked]>:
          Helped-by: Phillip Wood <[email-masked]>
     @@ Commit message
          Signed-off-by: ZheNing Hu <[email-masked]>
      
       ## builtin/rebase.c ##
     -@@ builtin/rebase.c: struct rebase_options {
     - 		REBASE_FORCE = 1<<3,
     - 		REBASE_INTERACTIVE_EXPLICIT = 1<<4,
     - 	} flags;
     -+	int delete_cherry_pick_head;
     - 	struct strvec git_am_opts;
     - 	const char *action;
     - 	int signoff;
      @@ builtin/rebase.c: static struct replay_opts get_replay_opts(const struct rebase_options *opts)
       		oidcpy(&replay.squash_onto, opts->squash_onto);
       		replay.have_squash_onto = 1;
       	}
     -+	replay.delete_cherry_pick_head = opts->delete_cherry_pick_head;
     ++	replay.delete_cherry_pick_head = 1;
       
       	return replay;
       }
     -@@ builtin/rebase.c: static int run_specific_rebase(struct rebase_options *opts, enum action action)
     - 	if (opts->type == REBASE_MERGE) {
     - 		/* Run sequencer-based rebase */
     - 		setenv("GIT_CHERRY_PICK_HELP", resolvemsg, 1);
     -+		opts->delete_cherry_pick_head = 1;
     - 		if (!(opts->flags & REBASE_INTERACTIVE_EXPLICIT)) {
     - 			setenv("GIT_SEQUENCE_EDITOR", ":", 1);
     - 			opts->autosquash = 0;
      
       ## builtin/revert.c ##
      @@ builtin/revert.c: static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
     @@ t/t3507-cherry-pick-conflict.sh: test_expect_success 'advice from failed cherry-
       test_expect_success 'failed cherry-pick sets CHERRY_PICK_HEAD' '
       	pristine_detach initial &&
       	test_must_fail git cherry-pick picked &&
     + 	test_cmp_rev picked CHERRY_PICK_HEAD
     + '
     + 
     ++test_expect_success 'failed cherry-pick with --delete-cherry-pick-head does not set CHERRY_PICK_HEAD' '
     ++	pristine_detach initial &&
     ++	test_must_fail git cherry-pick --delete-cherry-pick-head picked &&
     ++	test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
     ++'
     ++
     + test_expect_success 'successful cherry-pick does not set CHERRY_PICK_HEAD' '
     + 	pristine_detach initial &&
     + 	git cherry-pick base &&
      @@ t/t3507-cherry-pick-conflict.sh: test_expect_success \
       	test_must_fail git rev-parse --verify CHERRY_PICK_HEAD
       '
 2:  7e1ed49728d ! 2:  5279bca7a79 [GSOC] cherry-pick: use better advice message
     @@ Commit message
          This is the improved advice:
      
          hint: Resolve all conflicts manually, mark them as resolved with
     -    hint: "git add/rm <conflicted_files>", then run "git cherry-pick \
     -    --continue".
     +    hint: "git add/rm <conflicted_files>", then run
     +    hint: "git cherry-pick --continue".
          hint: You can instead skip this commit: run "git cherry-pick --skip".
          hint: To abort and get back to the state before "git cherry-pick",
          hint: run "git cherry-pick --abort".
     @@ sequencer.c: static void print_advice(struct replay_opts *opts, int show_hint)
      -		if (opts->no_commit)
      +		if (opts->action == REPLAY_PICK) {
      +			advise(_("Resolve all conflicts manually, mark them as resolved with\n"
     -+				 "\"git add/rm <conflicted_files>\", then run \"git cherry-pick --continue\".\n"
     ++				 "\"git add/rm <conflicted_files>\", then run\n"
     ++				 "\"git cherry-pick --continue\".\n"
      +				 "You can instead skip this commit: run \"git cherry-pick --skip\".\n"
      +				 "To abort and get back to the state before \"git cherry-pick\",\n"
      +				 "run \"git cherry-pick --abort\"."));
     @@ t/t3507-cherry-pick-conflict.sh: test_expect_success 'advice from failed cherry-
      -	hint: with 'git add <paths>' or 'git rm <paths>'
      -	hint: and commit the result with 'git commit'
      +	hint: Resolve all conflicts manually, mark them as resolved with
     -+	hint: \"git add/rm <conflicted_files>\", then run \"git cherry-pick --continue\".
     ++	hint: \"git add/rm <conflicted_files>\", then run
     ++	hint: \"git cherry-pick --continue\".
      +	hint: You can instead skip this commit: run \"git cherry-pick --skip\".
      +	hint: To abort and get back to the state before \"git cherry-pick\",
      +	hint: run \"git cherry-pick --abort\".
     @@ t/t3507-cherry-pick-conflict.sh: test_expect_success 'advice from failed cherry-
      -	hint: after resolving the conflicts, mark the corrected paths
      -	hint: with 'git add <paths>' or 'git rm <paths>'
      +	hint: Resolve all conflicts manually, mark them as resolved with
     -+	hint: \"git add/rm <conflicted_files>\", then run \"git cherry-pick --continue\".
     ++	hint: \"git add/rm <conflicted_files>\", then run
     ++	hint: \"git cherry-pick --continue\".
      +	hint: You can instead skip this commit: run \"git cherry-pick --skip\".
      +	hint: To abort and get back to the state before \"git cherry-pick\",
      +	hint: run \"git cherry-pick --abort\".

-- 
gitgitgadget