Re: [PATCH v2 6/7] rebase: write better reflog messages

Johannes Sixt <j.sixt@viscovery.net> writes:

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.