{"thread":{"id":"56178","subject":"[PATCH 0/2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP","startedAt":"2021-07-29T12:27:51Z","lastAt":"2021-07-30T14:15:01Z","messageCount":5,"participants":["ZheNing Hu via GitGitGadget","Junio C Hamano","ZheNing Hu"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"431478","messageId":"pull.1007.git.1627561665.gitgitgadget@gmail.com","threadId":"56178","inReplyTo":null,"subject":"[PATCH 0/2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-29T12:27:42Z","receivedAt":"2021-07-29T12:27:51Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"This patch fixes the bug when git cherry-pick is used with environment\nvariable GIT_CHERRY_PICK_HELP, and makes git chery-pick advice message\nbetter.\n\nv2:\nhttps://lore.kernel.org/git/pull.1001.v2.git.1627135281887.gitgitgadget@gmail.com/\n\nv2-->v3:\n\n 1. Add hidden option --rebase-preserve-merges-mode to git cherry-pick.\n 2. Split print_advice() into itself and\n    check_need_delete_cherry_pick_head().\n 3. Only allow git cherry-pick skip check_need_delete_cherry_pick_head().\n 4. Use better git cherry-pick advice message.\n\nZheNing Hu (2):\n  [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP\n  [GSOC] cherry-pick: use better advice message\n\n builtin/revert.c                |  2 ++\n git-rebase--preserve-merges.sh  |  2 +-\n sequencer.c                     | 45 +++++++++++++++++++++++----------\n sequencer.h                     |  1 +\n t/t3507-cherry-pick-conflict.sh | 44 +++++++++++++++++++++-----------\n 5 files changed, 64 insertions(+), 30 deletions(-)\n\n\nbase-commit: daab8a564f8bbac55f70f8bf86c070e001a9b006\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1007%2Fadlternative%2Fcherry-pick-help-fix-2-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1007/adlternative/cherry-pick-help-fix-2-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1007\n-- \ngitgitgadget\n"},{"id":"431479","messageId":"5f08f4dc1180ffd6f7fae7419a4c7214fe770b0a.1627561665.git.gitgitgadget@gmail.com","threadId":"56178","inReplyTo":"pull.1007.git.1627561665.gitgitgadget@gmail.com","subject":"[PATCH 2/2] [GSOC] cherry-pick: use better advice message","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-29T12:27:44Z","receivedAt":"2021-07-29T12:27:54Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\nIn the past, git cherry-pick would print such advice when\nthere was a conflict:\n\nhint: after resolving the conflicts, mark the corrected paths\nhint: with 'git add <paths>' or 'git rm <paths>'\nhint: and commit the result with 'git commit'\n\nBut in fact, when we want to cherry-pick multiple commits,\nwe should not use \"git commit\" after resolving conflicts, which\nwill make Git generate some errors. We should recommend users to\nuse `git cherry-pick --continue`, `git cherry-pick --abort`, just\nlike git rebase does.\n\nThis is the improved advice:\n\nhint: Resolve all conflicts manually, mark them as resolved with\nhint: \"git add/rm <conflicted_files>\", then run \"git cherry-pick \\\n--continue\".\nhint: You can instead skip this commit: run \"git cherry-pick --skip\".\nhint: To abort and get back to the state before \"git cherry-pick\",\nhint: run \"git cherry-pick --abort\".\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by Hariom Verma <hariom18599@gmail.com>:\nHelped-by: Phillip Wood <phillip.wood@dunelm.org.uk>\nHepled-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n sequencer.c                     | 19 ++++++++++++++-----\n t/t3507-cherry-pick-conflict.sh | 23 +++++++++++++++--------\n 2 files changed, 29 insertions(+), 13 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex ceaf73a34df..b9adeb7e5f5 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -39,6 +39,16 @@\n \n static const char sign_off_header[] = \"Signed-off-by: \";\n static const char cherry_picked_prefix[] = \"(cherry picked from commit \";\n+static const char *no_commit_advice = N_(\"after resolving the conflicts, mark the corrected paths\\n\"\n+\t\t\t\t\"with 'git add <paths>' or 'git rm <paths>'\");\n+static const char *commit_advice = N_(\"after resolving the conflicts, mark the corrected paths\\n\"\n+\t\t\t    \"with 'git add <paths>' or 'git rm <paths>'\\n\"\n+\t\t\t    \"and commit the result with 'git commit'\");\n+static const char *cherry_pick_advice = N_(\"Resolve all conflicts manually, mark them as resolved with\\n\"\n+\t\t\t\t\t\"\\\"git add/rm <conflicted_files>\\\", then run \\\"git cherry-pick --continue\\\".\\n\"\n+\t\t\t\t\t\"You can instead skip this commit: run \\\"git cherry-pick --skip\\\".\\n\"\n+\t\t\t\t\t\"To abort and get back to the state before \\\"git cherry-pick\\\",\\n\"\n+\t\t\t\t\t\"run \\\"git cherry-pick --abort\\\".\");\n \n GIT_PATH_FUNC(git_path_commit_editmsg, \"COMMIT_EDITMSG\")\n \n@@ -419,12 +429,9 @@ static void print_advice(struct replay_opts *opts, const char *help_msgs)\n \tif (help_msgs)\n \t\tadvise(\"%s\\n\", help_msgs);\n \telse if (opts->no_commit)\n-\t\tadvise(_(\"after resolving the conflicts, mark the corrected paths\\n\"\n-\t\t\t \"with 'git add <paths>' or 'git rm <paths>'\"));\n+\t\tadvise(\"%s\\n\", _(no_commit_advice));\n \telse\n-\t\tadvise(_(\"after resolving the conflicts, mark the corrected paths\\n\"\n-\t\t\t \"with 'git add <paths>' or 'git rm <paths>'\\n\"\n-\t\t\t \"and commit the result with 'git commit'\"));\n+\t\tadvise(\"%s\\n\", _(commit_advice));\n }\n \n static int write_message(const void *buf, size_t len, const char *filename,\n@@ -2269,6 +2276,8 @@ static int do_pick_commit(struct repository *r,\n \t\t      ? _(\"could not revert %s... %s\")\n \t\t      : _(\"could not apply %s... %s\"),\n \t\t      short_commit_name(commit), msg.subject);\n+\t\tif (opts->action == REPLAY_PICK)\n+\t\t\thelp_msgs = _(cherry_pick_advice);\n \t\tif (((opts->action == REPLAY_PICK &&\n \t\t      !opts->rebase_preserve_merges_mode) ||\n \t\t      (help_msgs = check_need_delete_cherry_pick_head(r))) &&\ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex 6f8035399d9..bf77bb0bd50 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -53,9 +53,11 @@ test_expect_success 'advice from failed cherry-pick' \"\n \tpicked=\\$(git rev-parse --short picked) &&\n \tcat <<-EOF >expected &&\n \terror: could not apply \\$picked... picked\n-\thint: after resolving the conflicts, mark the corrected paths\n-\thint: with 'git add <paths>' or 'git rm <paths>'\n-\thint: and commit the result with 'git commit'\n+\thint: Resolve all conflicts manually, mark them as resolved with\n+\thint: \\\"git add/rm <conflicted_files>\\\", then run \\\"git cherry-pick --continue\\\".\n+\thint: You can instead skip this commit: run \\\"git cherry-pick --skip\\\".\n+\thint: To abort and get back to the state before \\\"git cherry-pick\\\",\n+\thint: run \\\"git cherry-pick --abort\\\".\n \tEOF\n \ttest_must_fail git cherry-pick picked 2>actual &&\n \n@@ -68,8 +70,11 @@ test_expect_success 'advice from failed cherry-pick --no-commit' \"\n \tpicked=\\$(git rev-parse --short picked) &&\n \tcat <<-EOF >expected &&\n \terror: could not apply \\$picked... picked\n-\thint: after resolving the conflicts, mark the corrected paths\n-\thint: with 'git add <paths>' or 'git rm <paths>'\n+\thint: Resolve all conflicts manually, mark them as resolved with\n+\thint: \\\"git add/rm <conflicted_files>\\\", then run \\\"git cherry-pick --continue\\\".\n+\thint: You can instead skip this commit: run \\\"git cherry-pick --skip\\\".\n+\thint: To abort and get back to the state before \\\"git cherry-pick\\\",\n+\thint: run \\\"git cherry-pick --abort\\\".\n \tEOF\n \ttest_must_fail git cherry-pick --no-commit picked 2>actual &&\n \n@@ -82,9 +87,11 @@ test_expect_success 'advice from failed cherry-pick with GIT_CHERRY_PICK_HELP' \"\n \t\tpicked=\\$(git rev-parse --short picked) &&\n \t\tcat <<-EOF >expected &&\n \t\terror: could not apply \\$picked... picked\n-\t\thint: after resolving the conflicts, mark the corrected paths\n-\t\thint: with 'git add <paths>' or 'git rm <paths>'\n-\t\thint: and commit the result with 'git commit'\n+\t\thint: Resolve all conflicts manually, mark them as resolved with\n+\t\thint: \\\"git add/rm <conflicted_files>\\\", then run \\\"git cherry-pick --continue\\\".\n+\t\thint: You can instead skip this commit: run \\\"git cherry-pick --skip\\\".\n+\t\thint: To abort and get back to the state before \\\"git cherry-pick\\\",\n+\t\thint: run \\\"git cherry-pick --abort\\\".\n \t\tEOF\n \t\tGIT_CHERRY_PICK_HELP='and then do something else' &&\n \t\texport GIT_CHERRY_PICK_HELP &&\n-- \ngitgitgadget\n"},{"id":"431480","messageId":"5c5ec310b9230ee19bde4b3c8733b359d5218b32.1627561665.git.gitgitgadget@gmail.com","threadId":"56178","inReplyTo":"pull.1007.git.1627561665.gitgitgadget@gmail.com","subject":"[PATCH 1/2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP","fromName":"ZheNing Hu via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-07-29T12:27:43Z","receivedAt":"2021-07-29T12:27:55Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"From: ZheNing Hu <adlternative@gmail.com>\n\nGIT_CHERRY_PICK_HELP is an environment variable, as the\nimplementation detail of some porcelain in git to help realize\nthe rebasing steps. E.g. `git rebase -p` set GIT_CHERRY_PICK_HELP\nvalue in git-rebase--preserve-merges.sh. But If we set the value\nof GIT_CHERRY_PICK_HELP when using `git cherry-pick`, CHERRY_PICK_HEAD\nwill be deleted, then we will get an error when we try to use\n`git cherry-pick --continue` or other cherr-pick command.\n\nIntroduce new \"hidden\" option --rebase-preserve-merges-mode for git\ncherry-pick which indicates that git cherry-pick is currently called by\ngit-rebase--preserve-merges.sh. After `git rebase -p` completely\nabolished, this option should be removed.\n\nAnd then we split print_advice() into two part:\ncheck_need_delete_cherry_pick_head() check if we set\nGIT_CHERRY_PICK_HELP, if set, delete CHERRY_PICK_HEAD and return\nGIT_CHERRY_PICK_HELP's value, if not set, return NULL; The parameters\nof print_advice() have changed, which now accept a\n`struct replay_opts *opt` and a `const char *help_msgs`. We can pass\nthe value of GIT_CHERRY_PICK_HELP into print_advice(), and output it.\nIn this way, the steps of printing advice and checking\nGIT_CHERRY_PICK_HELP are decoupled.\n\nFinally, avoid deleting CHERRY_PICK_HEAD when we are truly\ncherry-picking, which can fix this breakage.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by Hariom Verma <hariom18599@gmail.com>:\nHelped-by: Phillip Wood <phillip.wood@dunelm.org.uk>\nHepled-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: ZheNing Hu <adlternative@gmail.com>\n---\n builtin/revert.c                |  2 ++\n git-rebase--preserve-merges.sh  |  2 +-\n sequencer.c                     | 36 ++++++++++++++++++++-------------\n sequencer.h                     |  1 +\n t/t3507-cherry-pick-conflict.sh | 27 ++++++++++++++++---------\n 5 files changed, 43 insertions(+), 25 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 237f2f18d4c..6165bb10143 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -127,6 +127,8 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)\n \t\t\tOPT_BOOL(0, \"allow-empty\", &opts->allow_empty, N_(\"preserve initially empty commits\")),\n \t\t\tOPT_BOOL(0, \"allow-empty-message\", &opts->allow_empty_message, N_(\"allow commits with empty messages\")),\n \t\t\tOPT_BOOL(0, \"keep-redundant-commits\", &opts->keep_redundant_commits, N_(\"keep redundant, empty commits\")),\n+\t\t\tOPT_BOOL_F(0, \"rebase-preserve-merges-mode\", &opts->rebase_preserve_merges_mode,\n+\t\t\t\t   N_(\"use for git-rebase--preserve-merges backend\"), PARSE_OPT_HIDDEN),\n \t\t\tOPT_END(),\n \t\t};\n \t\toptions = parse_options_concat(options, cp_extra);\ndiff --git a/git-rebase--preserve-merges.sh b/git-rebase--preserve-merges.sh\nindex b9c71d2a71b..ca97d9b6539 100644\n--- a/git-rebase--preserve-merges.sh\n+++ b/git-rebase--preserve-merges.sh\n@@ -444,7 +444,7 @@ pick_one_preserving_merges () {\n \t\t\toutput eval git cherry-pick $allow_rerere_autoupdate \\\n \t\t\t\t$allow_empty_message \\\n \t\t\t\t${gpg_sign_opt:+$(git rev-parse --sq-quote \"$gpg_sign_opt\")} \\\n-\t\t\t\t\"$strategy_args\" \"$@\" ||\n+\t\t\t\t\"$strategy_args\" --rebase-preserve-merges-mode \"$@\" ||\n \t\t\t\tdie_with_patch $sha1 \"$(eval_gettext \"Could not pick \\$sha1\")\"\n \t\t\t;;\n \t\tesac\ndiff --git a/sequencer.c b/sequencer.c\nindex 0bec01cf38e..ceaf73a34df 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -397,13 +397,11 @@ static void free_message(struct commit *commit, struct commit_message *msg)\n \tunuse_commit_buffer(commit, msg->message);\n }\n \n-static void print_advice(struct repository *r, int show_hint,\n-\t\t\t struct replay_opts *opts)\n+static char *check_need_delete_cherry_pick_head(struct repository *r)\n {\n \tchar *msg = getenv(\"GIT_CHERRY_PICK_HELP\");\n \n \tif (msg) {\n-\t\tfprintf(stderr, \"%s\\n\", msg);\n \t\t/*\n \t\t * A conflict has occurred but the porcelain\n \t\t * (typically rebase --interactive) wants to take care\n@@ -411,18 +409,22 @@ static void print_advice(struct repository *r, int show_hint,\n \t\t */\n \t\trefs_delete_ref(get_main_ref_store(r), \"\", \"CHERRY_PICK_HEAD\",\n \t\t\t\tNULL, 0);\n-\t\treturn;\n+\t\treturn msg;\n \t}\n+\treturn NULL;\n+}\n \n-\tif (show_hint) {\n-\t\tif (opts->no_commit)\n-\t\t\tadvise(_(\"after resolving the conflicts, mark the corrected paths\\n\"\n-\t\t\t\t \"with 'git add <paths>' or 'git rm <paths>'\"));\n-\t\telse\n-\t\t\tadvise(_(\"after resolving the conflicts, mark the corrected paths\\n\"\n-\t\t\t\t \"with 'git add <paths>' or 'git rm <paths>'\\n\"\n-\t\t\t\t \"and commit the result with 'git commit'\"));\n-\t}\n+static void print_advice(struct replay_opts *opts, const char *help_msgs)\n+{\n+\tif (help_msgs)\n+\t\tadvise(\"%s\\n\", help_msgs);\n+\telse if (opts->no_commit)\n+\t\tadvise(_(\"after resolving the conflicts, mark the corrected paths\\n\"\n+\t\t\t \"with 'git add <paths>' or 'git rm <paths>'\"));\n+\telse\n+\t\tadvise(_(\"after resolving the conflicts, mark the corrected paths\\n\"\n+\t\t\t \"with 'git add <paths>' or 'git rm <paths>'\\n\"\n+\t\t\t \"and commit the result with 'git commit'\"));\n }\n \n static int write_message(const void *buf, size_t len, const char *filename,\n@@ -2261,11 +2263,17 @@ static int do_pick_commit(struct repository *r,\n \t\tres = -1;\n \n \tif (res) {\n+\t\tconst char *help_msgs = NULL;\n+\n \t\terror(command == TODO_REVERT\n \t\t      ? _(\"could not revert %s... %s\")\n \t\t      : _(\"could not apply %s... %s\"),\n \t\t      short_commit_name(commit), msg.subject);\n-\t\tprint_advice(r, res == 1, opts);\n+\t\tif (((opts->action == REPLAY_PICK &&\n+\t\t      !opts->rebase_preserve_merges_mode) ||\n+\t\t      (help_msgs = check_need_delete_cherry_pick_head(r))) &&\n+\t\t      res == 1)\n+\t\t\tprint_advice(opts, help_msgs);\n \t\trepo_rerere(r, opts->allow_rerere_auto);\n \t\tgoto leave;\n \t}\ndiff --git a/sequencer.h b/sequencer.h\nindex d57d8ea23d7..5a40b6d8bdc 100644\n--- a/sequencer.h\n+++ b/sequencer.h\n@@ -49,6 +49,7 @@ struct replay_opts {\n \tint reschedule_failed_exec;\n \tint committer_date_is_author_date;\n \tint ignore_date;\n+\tint rebase_preserve_merges_mode;\n \n \tint mainline;\n \ndiff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh\nindex 014001b8f32..6f8035399d9 100755\n--- a/t/t3507-cherry-pick-conflict.sh\n+++ b/t/t3507-cherry-pick-conflict.sh\n@@ -76,6 +76,23 @@ test_expect_success 'advice from failed cherry-pick --no-commit' \"\n \ttest_cmp expected actual\n \"\n \n+test_expect_success 'advice from failed cherry-pick with GIT_CHERRY_PICK_HELP' \"\n+\tpristine_detach initial &&\n+\t(\n+\t\tpicked=\\$(git rev-parse --short picked) &&\n+\t\tcat <<-EOF >expected &&\n+\t\terror: could not apply \\$picked... picked\n+\t\thint: after resolving the conflicts, mark the corrected paths\n+\t\thint: with 'git add <paths>' or 'git rm <paths>'\n+\t\thint: and commit the result with 'git commit'\n+\t\tEOF\n+\t\tGIT_CHERRY_PICK_HELP='and then do something else' &&\n+\t\texport GIT_CHERRY_PICK_HELP &&\n+\t\ttest_must_fail git cherry-pick picked 2>actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+\"\n+\n test_expect_success 'failed cherry-pick sets CHERRY_PICK_HEAD' '\n \tpristine_detach initial &&\n \ttest_must_fail git cherry-pick picked &&\n@@ -109,16 +126,6 @@ test_expect_success \\\n \ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD\n '\n \n-test_expect_success 'GIT_CHERRY_PICK_HELP suppresses CHERRY_PICK_HEAD' '\n-\tpristine_detach initial &&\n-\t(\n-\t\tGIT_CHERRY_PICK_HELP=\"and then do something else\" &&\n-\t\texport GIT_CHERRY_PICK_HELP &&\n-\t\ttest_must_fail git cherry-pick picked\n-\t) &&\n-\ttest_must_fail git rev-parse --verify CHERRY_PICK_HEAD\n-'\n-\n test_expect_success 'git reset clears CHERRY_PICK_HEAD' '\n \tpristine_detach initial &&\n \n-- \ngitgitgadget\n\n"},{"id":"431525","messageId":"xmqqzgu43kkz.fsf@gitster.g","threadId":"56178","inReplyTo":"5c5ec310b9230ee19bde4b3c8733b359d5218b32.1627561665.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-29T20:56:44Z","receivedAt":"2021-07-29T20:56:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"ZheNing Hu via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> -static void print_advice(struct repository *r, int show_hint,\n> -\t\t\t struct replay_opts *opts)\n> +static char *check_need_delete_cherry_pick_head(struct repository *r)\n>  {\n>  \tchar *msg = getenv(\"GIT_CHERRY_PICK_HELP\");\n>  \n>  \tif (msg) {\n> -\t\tfprintf(stderr, \"%s\\n\", msg);\n>  \t\t/*\n>  \t\t * A conflict has occurred but the porcelain\n>  \t\t * (typically rebase --interactive) wants to take care\n> @@ -411,18 +409,22 @@ static void print_advice(struct repository *r, int show_hint,\n>  \t\t */\n>  \t\trefs_delete_ref(get_main_ref_store(r), \"\", \"CHERRY_PICK_HEAD\",\n>  \t\t\t\tNULL, 0);\n> -\t\treturn;\n> +\t\treturn msg;\n>  \t}\n> +\treturn NULL;\n> +}\n\nThe patch is a bit hard to read, but I do not think this improves\nthe root cause of the issue at all, which is that \"do we need to\ndelete the pseudoref?\" decision is tied to and based on the presense\nof the custom message in the environment variable.\n\nThe original issue was that we declared (erroneously) that the\npresence of a custom message _means_ the caller is not a human but a\nsequencing machinery that wants to deal with what is done via the\nCHERRY_PICK_HEAD pseudoref itself, and your original issue was that\nwhen a human user has the custom message envirnoment (either on\npurpose or by mistake), the logic _misidentifies_ the call as if it\nis made from a sequencing machinery and we lose the CHERRY_PICK_HEAD\npseudoref.  The root cause is mistaking the correlation (sequencing\nmachinery uses the environment and it also does not want pseudoref)\nas the causation (presense of the environment means we should remove\npseudoref).\n"},{"id":"431573","messageId":"CAOLTT8Tj_8ESALm16OV=XxO604Qq_8KKJWYvzA7T=c=92uSyZg@mail.gmail.com","threadId":"56178","inReplyTo":"xmqqzgu43kkz.fsf@gitster.g","subject":"Re: [PATCH 1/2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-07-30T14:15:36Z","receivedAt":"2021-07-30T14:15:01Z","isPatch":true,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Junio C Hamano <gitster@pobox.com> 于2021年7月30日周五 上午4:56写道：\n>\n> The patch is a bit hard to read, but I do not think this improves\n> the root cause of the issue at all, which is that \"do we need to\n> delete the pseudoref?\" decision is tied to and based on the presense\n> of the custom message in the environment variable.\n>\n\nMy original idea was to distinguish between cherry-pick calls by machines\n(git-rebase--preserve-merges.sh) and cherry-pick calls by people through\nan additional option \"rebase-preserve-merges-mode\".\n\n> The original issue was that we declared (erroneously) that the\n> presence of a custom message _means_ the caller is not a human but a\n> sequencing machinery that wants to deal with what is done via the\n> CHERRY_PICK_HEAD pseudoref itself, and your original issue was that\n> when a human user has the custom message envirnoment (either on\n> purpose or by mistake), the logic _misidentifies_ the call as if it\n> is made from a sequencing machinery and we lose the CHERRY_PICK_HEAD\n> pseudoref.  The root cause is mistaking the correlation (sequencing\n> machinery uses the environment and it also does not want pseudoref)\n> as the causation (presense of the environment means we should remove\n> pseudoref).\n\nOk... Now I guess what you mean is to provide an option for git cherry-pick\nto specifically delete \"CHERRY_PICK_HEAD\", and this options is \"hidden\",\nit is not provided to users, it is only reserved for sequencing\nmachinery. In this\nway, we separate print_advice() from deleting \"CHERRY_PICK_HEAD\".\nThen in the future, GIT_CHERRY_PICK_HELP will be deleted.\n\nThanks,\n--\nZheNing Hu\n"}]}