Junio C Hamano <gits...@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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html