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

[Junio C Hamano <gitster@pobox.com>]

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

So I would understand if your proposal _were_ to

 * add "--allow-no-ff" option;

 * change the meaning of "+ref" to "--allow-no-ff for only this
   ref"; and

 * add a new "*ref" (or whatever new syntax) to still allow people
   to say "--force only this ref".

but we still need to assume that it makes sense to ask lockref but
still want to ensure the update fast-forwards.  I personally do not
think it does [*1*].

The semantics the posted patch (rerolled to allow "--force" push
anything) implements lets "--lockref" to imply "--allow-no-ff" and
that makes it much simpler; we do not have to deal with any of the
above complexity.


[Footnote]

 *1* The assurance --lockref gives is a lot stronger than "must
     fast-forward".  You may have fetched the topic whose tip was at
     commit X, and rebased it on top of X~4 to create a new history
     leading to Y.

           o----o----Y
          /
     o---o----o----o----o----X
	X~4

     When you "git push --lockref=topic:X Y:X", you are requiring
     their tip to be still at X.  Other people's change cannot be to
     add something on top of X (which will be lost if we replace the
     tip of the topic with Y).

     If your change were not a rebase but to build one of you own:

     o---o----o----o----o----X---Y

     your "git push --lockref=topic:X Y:X" still requires the tip is
     at X.  If somebody rewound the tip to X~2 in the meantime
     (because they decided the tip 2 commits were not good), your
     "git push Y:X" without the "--lockref" will lose their rewind,
     because Y will still be a fast-forward update of X~2.
     "--lockref=topic:X" will protect you in this case as well.

     So I think "--lockref" that automatically disables "must
     fast-forward" check is the right thing to do, as we are
     replacing the weaker "must fast-forward" with something
     stronger.  I do not think we are getting anything from forcing
     the user to say "--allow-no-ff" with "+ref" syntax when the
     user says "--lockref".  It is not making it safer, and it is
     making it less convenient.