Re: [PATCH v2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Phillip Wood <phillip.wood123@gmail.com> writes:
>
>> This will break git-rebase--preserve-merges.sh which uses
>> GIT_CHERRY_PICK_HELP to set the help and ensure CHERRY_PICK_HEAD is 
>> removed when picking commits.
>
> Ahh, I didn't realize we still had scripted rebase backends that
> called cherry-pick as an executable.  I was hoping that all rebase
> backends by now would be calling into the cherry-pick machinery
> directly, bypassing cmd_cherry_pick(), and that was why I suggested
> to catch stray one the end-users set manually in the environment
> and clear it there.
>
>> I'm a bit confused as to what the
>> problem is - how is 'git cherry-pick' being run with
>> GIT_CHERRY_PICK_HELP set in the environment outside of a rebase (your
>> explanation in [1] does not mention how GIT_CHERRY_PICK_HELP is set)?
>
> I didn't press for the information too hard, but I guessed that it
> was perhaps because somebody like stackoverflow suggested to set a
> message in their environment to get a "better message."

A good way forward may be to relieve sequencer.c::print_advice() of
the responsibility of optinally removing CHERRY_PICK_HEAD; make it a
separate function that bases its decision on a more direct cue, not
on the presense of a custom message in GIT_CHERRY_PICK_HELP, make
do_pick_commit(), which is the sole caller of print_advice(), call
it after calling print_advice().

I do not offhand know what that "direct cue" should be, but we may
already have an appropriate field in the replay_opts structure;
"replay.action is neither REVERT nor PICK" could be a good enough
approximation, I dunno.

Otherwise we can allocate a new bit in the structure, have relevant
callers set it, and teach cherry-pick an unadvertised command line
option that sets the bit, and use that option only from
git-rebase--preserve-merges when it makes a call to cherry-pick.
When "rebase -p" is either retired or rewritten in C, we can retire
the option from cherry-pick.

Workable?