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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 14, 2014, 07:49 UTC
Message-ID: <xmqq1tp643yb.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqqzjbuzu6t.fsf@gitster.dls.corp.google.com>

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

Show 10 quoted lines

>> 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.