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/pull.1010.git.1627714877.gitgitgadget@xxxxxxxxx/ 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 <christian.couder@xxxxxxxxx> Mentored-by Hariom Verma <hariom18599@xxxxxxxxx>: Helped-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx> @@ Commit message Signed-off-by: ZheNing Hu <adlternative@xxxxxxxxx> ## 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
