Re: [PATCH v4] Allow update hooks to update refs on their own.

Junio C Hamano [off-list ref] writes:

Jeff King [off-list ref] writes:

quoted

..., but an
"ok, but btw I changed your commit" status from receive-pack seems like
it would be useful, for two reasons:

  - it can be displayed differently, so the user is reminded to do a
    fetch afterwards
  - we can avoid updating the tracking ref, which makes it less likely
    to result in a non-fast forward fetch next time.  For example,
    consider:

      1. The remote master and my origin/master are at A.
      2. I make a commit B on top of A.
      3. I push B to remote, who rewrites it to B' on top of A. At the
         same time, I move my origin/master to B.
      4. I fetch, and get non-ff going from B to B'.

    If I had never written anything to my origin/master, it would be a
    fast forward. And obviously git handles it just fine, but it is more
    useful to the user during the next fetch to see A..B rather than
    B'...B.

Sensible argument.  I stand corrected.

Having said that, I think the workflow that this "letting update hook
munge" patch supports has a bit more implications for the people who
interact with it.  The pusher will need to force fetch the result, but
after that he needs to discard his own commit and replace it with
whatever the hook did.  The question is how much to discard, and how to
reconstruct the changes since the last push that was made on top of the
commit that was pushed.

Maybe the push pushed out a string of five pearls and the hook may have
rewritten only the tip, or all of them.  You may have built a few
commits on top since then.  If what you pushed out contained merges and
the hook rewrote it, you would need to potentially replay such a merge
that was rewritten by the hook.

This is all the same with a workflow that deals with a branch that is
advertised to constantly rewound and rebased, and the common approach is
to use "fetch + rebase" (see recent discussion between Nico and Bruce).
So this patch is not creating a new problem (iow, I am not mentioning
this as the reason to reject this patch), but I thought I should bring
it up so that people know what they are doing.