git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4] [GSOC] cherry-pick: use better advice message

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 14, 2021, 20:32 UTC
Message-ID
<xmqqa6lju5qd.fsf@gitster.g>
In-Reply-To
<pull.1010.v4.git.1628936863733.gitgitgadget@gmail.com>
"ZheNing Hu via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 7 quoted lines
> From: ZheNing Hu <adlternative@gmail.com>
>
> In the past, git cherry-pick would print such advice when
> there was a conflict:
> 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'

In our log messages, we desciribe the current state in the present tense.

It may be worth mentioning that this is because the program originally was about picking only one commit and the hint was inherited from those days. Or it may not. I dunno.

Show 5 quoted lines
> But in fact, when we want to cherry-pick multiple commits,
> we should not use "git commit" after resolving conflicts, which
> will make Git generate some errors. We should recommend users to
> use `git cherry-pick --continue`, `git cherry-pick --abort`, just
> like git rebase does.

I am not sure "should not" is the right phrase. Also it does not help readers to have a vague "generate some errors" than not saying anything---it leaves readers puzzled with "what errors???"

Whether picking a single commit or a series of commits, after resolving conflicts in the current step, wouldn't

    $ git commit ;# to conclude the resolution
    $ git cherry-pick --continue
do the right thing?
> This is the improved advice:

It may be an improved advice, but just omit it and say something like:

	Suggest use of "git cherry-pick --contiue", so that it would
	also apply to cases where multiple commits are being picked.

The actual message does not have to be reproduced here, as it is in the source, and it can be seen in the test ;-)

That would make the proposed log message conform to our norm, i.e. brief description of what happens with the current system, followed by description of the perceived problem, followed by an order to the codebase to become different in a specific way that solves the problem.

Taken together, perhaps
	"git cherry-pick", upon seeing a conflict, says:
	    hint: ...
	as if running "git commit" to conclude the resolution of
	this single step were the end of the story.  This stems from
	the fact that the command originally was to pick a single
	commit and not a range of commits, and the message was
	written back then and has not been adjusted.
	When picking a range of commits and the command stops with a
	conflict in the middle of the range, however, after
	resolving the conflict and (optionally) recording the result
	with "git commit", the user has to run "git cherry-pick
	--continue" to have the rest of the range dealt with,
	"--skip" to drop the current commit, or "--abort" to discard
	the series.
	Suggest use of "git cherry-pick --continue/--skip/--abort"
	so that the message also covers the case where a range of
	commits are being picked.
> Helped-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> Hepled-by: Junio C Hamano <gitster@pobox.com>

It seems that unlike other people I keep hepling you, whatever that verb means ;-).

Thanks.
Show 100 quoted lines
> diff --git a/sequencer.c b/sequencer.c
> index 0bec01cf38e..2dd73d24a87 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -403,7 +403,7 @@ static void print_advice(struct repository *r, int show_hint,
>  	char *msg = getenv("GIT_CHERRY_PICK_HELP");
>  
>  	if (msg) {
> -		fprintf(stderr, "%s\n", msg);
> +		advise("%s\n", msg);
>  		/*
>  		 * A conflict has occurred but the porcelain
>  		 * (typically rebase --interactive) wants to take care
> @@ -418,6 +418,20 @@ static void print_advice(struct repository *r, int show_hint,
>  		if (opts->no_commit)
>  			advise(_("after resolving the conflicts, mark the corrected paths\n"
>  				 "with 'git add <paths>' or 'git rm <paths>'"));
> +		else if (opts->action == REPLAY_PICK)
> +			advise(_("Resolve all conflicts manually, mark them as resolved with\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\"."));
> +		else if (opts->action == REPLAY_REVERT)
> +			advise(_("Resolve all conflicts manually, mark them as resolved with\n"
> +				 "\"git add/rm <conflicted_files>\", then run\n"
> +				 "\"git revert --continue\".\n"
> +				 "You can instead skip this commit: run \"git revert --skip\".\n"
> +				 "To abort and get back to the state before \"git revert\",\n"
> +				 "run \"git revert --abort\"."));
>  		else
>  			advise(_("after resolving the conflicts, mark the corrected paths\n"
>  				 "with 'git add <paths>' or 'git rm <paths>'\n"
> diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
> index 9d100cd1884..6766aed7282 100755
> --- a/t/t3501-revert-cherry-pick.sh
> +++ b/t/t3501-revert-cherry-pick.sh
> @@ -158,4 +158,24 @@ test_expect_success 'cherry-pick works with dirty renamed file' '
>  	grep -q "^modified$" renamed
>  '
>  
> +test_expect_success 'advice from failed revert' '
> +	echo dream >dream &&
> +	git add dream &&
> +	git commit -m "add dream" &&
> +	dream_oid=$(git rev-parse --short HEAD) &&
> +	cat <<-EOF >expected &&
> +	error: could not revert $dream_oid... add dream
> +	hint: Resolve all conflicts manually, mark them as resolved with
> +	hint: "git add/rm <conflicted_files>", then run
> +	hint: "git revert --continue".
> +	hint: You can instead skip this commit: run "git revert --skip".
> +	hint: To abort and get back to the state before "git revert",
> +	hint: run "git revert --abort".
> +	EOF
> +	echo dream >>dream &&
> +	git add dream &&
> +	git commit -m "double-add dream" &&
> +	test_must_fail git revert HEAD^ 2>actual &&
> +	test_cmp expected actual
> +'
>  test_done
> diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
> index 014001b8f32..cb2ebea9ad3 100755
> --- a/t/t3507-cherry-pick-conflict.sh
> +++ b/t/t3507-cherry-pick-conflict.sh
> @@ -47,20 +47,23 @@ test_expect_success 'failed cherry-pick does not advance HEAD' '
>  	test "$head" = "$newhead"
>  '
>  
> -test_expect_success 'advice from failed cherry-pick' "
> +test_expect_success 'advice from failed cherry-pick' '
>  	pristine_detach initial &&
>  
> -	picked=\$(git rev-parse --short picked) &&
> +	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'
> +	error: could not apply $picked... picked
> +	hint: Resolve all conflicts manually, mark them as resolved with
> +	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".
>  	EOF
>  	test_must_fail git cherry-pick picked 2>actual &&
>  
>  	test_cmp expected actual
> -"
> +'
>  
>  test_expect_success 'advice from failed cherry-pick --no-commit' "
>  	pristine_detach initial &&
>
> base-commit: daab8a564f8bbac55f70f8bf86c070e001a9b006
Previous: ZheNing Hu via GitGitGadgetNext: ZheNing Hu
Message 23 of 32 in “[GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP”
  1. 0/2 [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELPZheNing Hu via GitGitGadget, Jul 31, 2021
  2. 2/2 [GSOC] cherry-pick: use better advice messageZheNing Hu via GitGitGadget, Jul 31, 2021
  3. Phillip WoodAug 1, 2021
  4. ZheNing HuAug 2, 2021
  5. 1/2 [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELPZheNing Hu via GitGitGadget, Jul 31, 2021
  6. Phillip WoodAug 1, 2021
  7. ZheNing HuAug 2, 2021
  8. 0/2 [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELPZheNing Hu via GitGitGadget, Aug 3, 2021
  9. 1/2 [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELPZheNing Hu via GitGitGadget, Aug 3, 2021
  10. Junio C HamanoAug 3, 2021
  11. ZheNing HuAug 4, 2021
  12. Phillip WoodAug 4, 2021
  13. Junio C HamanoAug 4, 2021
  14. ZheNing HuAug 5, 2021
  15. 2/2 [GSOC] cherry-pick: use better advice messageZheNing Hu via GitGitGadget, Aug 3, 2021
  16. [GSOC] cherry-pick: use better advice messageZheNing Hu via GitGitGadget, Aug 5, 2021
  17. Phillip WoodAug 11, 2021
  18. ZheNing HuAug 13, 2021
  19. Junio C HamanoAug 13, 2021
  20. ZheNing HuAug 14, 2021
  21. Phillip WoodAug 17, 2021
  22. [GSOC] cherry-pick: use better advice messageZheNing Hu via GitGitGadget, Aug 14, 2021
  23. Junio C HamanoAug 14, 2021
  24. ZheNing HuAug 15, 2021
  25. [GSOC] cherry-pick: use better advice messageZheNing Hu via GitGitGadget, Aug 16, 2021
  26. Phillip WoodAug 18, 2021
  27. ZheNing HuAug 19, 2021
  28. ZheNing HuAug 19, 2021
  29. [GSOC] cherry-pick: use better advice messageZheNing Hu via GitGitGadget, Aug 19, 2021
  30. Junio C HamanoAug 19, 2021
  31. ZheNing HuAug 21, 2021
  32. [GSOC] cherry-pick: use better advice messageZheNing Hu via GitGitGadget, Aug 22, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.