Subject: Re: [PATCH 2/2] Let deny.currentBranch=updateInstead ignore submodules

From: Junio C Hamano <[email-masked]>
Date: Thu, 13 Nov 2014 23:49:16 -0800

Junio C Hamano <[email-masked]> writes:

>> I agree that a new value "mergeInstead" or something should be
>> invented when/if different workflows want a looser semantics.
>> People would rely not only on "being able to push when clean" but
>> also on "being safely prevented from pushing when not" (and that is
>> where my earlier comment to test both sides comes from).  Loosening
>> the check to an existing "updateInstead" would break users who have
>> been using updateInstead.
>
> And thinking about the names again, I have a feeling that
> updateInstead and mergeInstead are both probably misnomer.

Let me take this part back.  After all, I do not think I would
design the mechanism to implement an alternative logic that decides
when it is safe to allow the update of the ref and to reflect the
changes to the working tree, and that actually does the checkout to
the working tree by using a new value like mergeInstead.  So we
would only need a single name, and updateInstead is not too bad.

The word "update" is so heavily loaded in the context of accepting a
push (i.e. it is unclear what update we are talking about---updating
the ref that we normally refuse to update?  updating the index?
updating the working tree?  Some combination of them?), so as a
single word, "checkoutInstead" may probably be a better one, though.
Upon hearing "checkout", by definition anybody would know that we
are involving the working tree.

The mechanism I would employ when doing an alternative logic,
possibly looser one but does not necessarily so, would be to have a
hook script "push-to-checkout".  When denyCurrentBranch is set to
updateInstead, if there is no such hook, the "working tree has to be
absolutely clean and we would do a 'read-tree -m -u $old $new'
(which is the same as 'reset --hard $new' under the precondition)"
you implemented will be used as the "logic that decides when it is
safe, and that does the checkout to the working tree".  When the
"push-to-checkout" hook exists, however, we just invoke that hook
with $new as argument, and it can decide when it is safe in whatever
way it chooses to, and it can checkout the $new to the working tree
in whatever way it wants.  The users of "mergeInstead" (now a dead
and unnecessary name) mode would just have a single-liner

	#!/bin/sh
	git reset --keep "$1" --

in there, as this single command would both decide when it is safe
and does the safe (according to its own definition) updating of the
working tree.

In your other example (not the "deploy to live website" one) of
unidirectional SSH connection, you would be able to connect from
machine A to machine B but not the other way, so while sitting on
machine A you would typically have one SSH session to have an
interactive shell session running on machine B.  You may have local
modification on machine B but your changes to history on machine A
cannot be pulled, so you would emulate it by pushing from A into B.
In such a case, unlike the "live website" example, it would be
useful to loosen the condition even more than "reset --keep" (which
is an equivalent of "checkout -B $current_branch $new_commit").

In such a case, what you want to do is to simulate "git pull" that
could conflict and give you a chance to resolve with a push in the
reverse direction.  You want to run an equivalent of the same
"checkout -B" command but with the "-m" option when accepting such a
push.

There are other definitions of what is safe and how update should
happen depending on the user, and such a logic can be placed in the
push-to-checkout hook without harming other users.