Re: [PATCH 7/7] push: document --lockref

From: John Keeping <john@keeping.me.uk>
Date: Jul 14, 2013, 14:28 UTC
Message-ID: <20130714142806.GA2239@serenity.lan>
In-Reply-To: <7vr4f2gr4m.fsf@alter.siamese.dyndns.org>

On Sat, Jul 13, 2013 at 01:08:09PM -0700, Junio C Hamano wrote:

Show 44 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > If "--lockref" automatically implies "--allow-no-ff" (the design in
> > the reposted patch), you cannot express that combination.  But once
> > you use "--lockref" in such a situation , for the push to succeed,
> > you know that the push replaces not just _any_ ancestor of what you
> > are pushing, but replaces the exact current value.  So I do not think
> > your implicit introduction of --allow-no-ff via redefining the
> > semantics of the plus prefix is not adding much value (if any),
> > while making the common case less easy to use.
> >
> >> No; --lockref only adds the check that the destination is at the
> >> expected revision, but does *NOT* override the no-ff check.
> >
> > You _could_ do it in that way, but that is less useful.
> 
> Another issue I have with the proposal is that we close the door to
> "force only this one" convenience we have with "+ref" vs "--force
> ref".  Assuming that it is useful to require lockref while still
> making sure that the usual "must fast-forward" rule is followed (if
> that is not the case, I do not see a reason why your proposal is any
> useful---am I missing something?), I would prefer to allow users a
> way to decorate this basic syntax to say:
> 
>     git push --lockref master jch pu
> 
> things like
> 
>  (1) pu may not fast-forward and please override that "must
>      fast-forward" check from it, while still keeping the lockref
>      safety (e.g. "+pu" that does not --force, which is your
>      proposal);
> 
>  (2) any of them may not fast-forward and please override that "must
>      fast-forward" check from it, while still keeping the lockref
>      safety (without adding "--allow-no-ff", I do not see how it is
>      possible with your proposal, short of forcing user to add "+"
>      everywhere);
> 
>  (3) I know jch does not fast-forward so please override the "must
>      fast-forward", but still apply the lockref safety, pu may not
>      even satisfy lockref safety so please force it (as the "only
>      force this one" semantics is removed from "+", I do not see how
>      it is possible with your proposal).

I haven't been following this thread too closely, but I was assuming that the interface would be something like this:

    git push origin +master
    git push --force origin master

mean the same thing and do what they do now.

    git push origin *master
    git push --lockref origin master

both mean the same thing: using the new compare-and-swap mode only update master if the remote side corresponds to remotes/origin/master [1].

    git push origin *master:refs/heads/master:@{1}

means to push the local ref master to the remote ref refs/heads/master if it currently points at "@{1}".

In this scenario, giving both --lockref and --force should be an error because the user is probably confused (the obvious interpretation is that --force wins, but I don't think that's sensible).

I'm not sure what should happen with:

    git push --force origin *master

where it appears that the user is asking for a compare-and-swap update of master but the --force is overriding this. I think we have to let --force win because when the refspec comes from remote.<name>.push we have to let the command-line --force override the specified behaviour.

I don't particularly like the name --lockref, the original --cas feels more descriptive to me.

[1] In fact, I suspect this would have to be "the ref that
    refs/heads/master maps to using remote.origin.fetch".