Johannes Sixt <j.sixt@xxxxxxxxxxxxx> writes:
> Am 6/18/2013 20:55, schrieb Ramkumar Ramachandra:
>> Now that the "checkout" invoked internally from "rebase" knows to honor
>> GIT_REFLOG_ACTION, we can start to use it to write a better reflog
>> message when "rebase anotherbranch", "rebase --onto branch",
>> etc. internally checks out the new fork point. We will write:
>>
>> rebase: checkout master
>>
>> instead of the old
>>
>> rebase
>
>> diff --git a/git-rebase--am.sh b/git-rebase--am.sh
>> index 34e3102..69fae7a 100644
>> --- a/git-rebase--am.sh
>> +++ b/git-rebase--am.sh
>> @@ -5,11 +5,13 @@
>>
>> case "$action" in
>> continue)
>> + GIT_REFLOG_ACTION="$base_reflog_action"
>
> I haven't followed the topic closely, but I wonder why there are so many
> explicit assignments to GIT_REFLOG_ACTION. Is calling set_reflog_action
> (defined in git-sh-setup) the wrong thing to do?
Excellent question, and I think this illustrates why the recent
reroll that uses an approach to use base_reflog_action is not
complete and needs further work (to put it mildly).
set_reflog_action is designed to be used once at the very top of a
program, like this in "git am", for example:
set_reflog_action am
The helper function sets the given string to GIT_REFLOG_ACTION only
when GIT_REFLOG_ACTION is not yet set. Thanks to this, "git am",
when run as the top-level program, will use "am" in GIT_REFLOG_ACTION
and the reflog entries made by whatever it does will say "am did this".
Because of the conditional assignment, when "git am" is run as a
subprogram (i.e. an implementation detail) of "git rebase", the call
to the helper function at the beginning will *not* have any effect.
So "git rebase" can do this:
set_reflog_action rebase
... do its own preparation, like checking out "onto" commit
... decide to do "format-patch" to "am" pipeline
git format-patch --stdout >mbox
git am mbox
and the reflog entries made inside "git am" invocation will say
"rebase", not "am".
The approach to introduce base_reflog_action may be valid, but if we
were to go that route, set_reflog_action needs to learn the new
convention. Perhaps by doing something like this:
1. set_reflog_action to set GIT_REFLOG_NAME and GIT_REFLOG_ACTION;
Program names like "am", "rebase" will be set to this value.
2. If the program does not want to say anything more than its
program name in the reflog, it does not have to do anything.
GIT_REFLOG_ACTION that is set upfront (or inherited from the
calling program) will be written in the reflog.
3. If the program wants to say more than just its name, it needs to
arrange GIT_REFLOG_ACTION not to be clobbered. It can do so in
various ways:
a) A one-shot invocation of any program that writes to reflog can
do:
GIT_REFLOG_ACTION="$GIT_REFLOG_NAME: custom message" \
git cmd
An important thing is that GIT_REFLOG_ACTION is left as the
original, i.e. "am" or "rebase", so that calls to programs that
write reflog using the default (i.e. program name only) will
not be affected.
b) Or it can temporarily change it and revert it back (necessary
to use shell function like "output" that cannot use the above
"single shot export" technique):
GIT_REFLOG_ACTION="$GIT_REFLOG_NAME: custom message"
output git cmd
GIT_REFLOG_ACTION=$GIT_REFLOG_NAME
But after writing it down this way, I realize that introduction of
base_reflog_action (or GIT_REFLOG_NAME which is a moral equivalent)
is not helping us at all. As long as calls to "git" command in the
second category exists in these scripts, GIT_REFLOG_ACTION *must* be
kept pristine after set_reflog_action sets it, so we can get rid of
this new variable, and rewrite 3.a and 3.b like so:
3-a)
GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: custom message" \
git cmd
3-b)
SAVED=$GIT_REFLOG_ACTION
GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: custom message"
output git cmd
GIT_REFLOG_ACTION=$SAVED
or
(
GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: custom message"
output git cmd
)
That essentially boils down to the very original suggestion I made
before Ram introduced the base_reflog_action.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html