D6776: bookmarks: validate changes on push (issue6193) (BC)
valentin.gatienbaron (Valentin Gatien-Baron)
phabricator at mercurial-scm.org
Sat Sep 14 01:56:18 EDT 2019
valentin.gatienbaron added a comment.
In D6776#100506 <https://phab.mercurial-scm.org/D6776#100506>, @durin42 wrote:
> In D6776#100487 <https://phab.mercurial-scm.org/D6776#100487>, @valentin.gatienbaron wrote:
>> I showed in D6847 <https://phab.mercurial-scm.org/D6847> the same change but implemented in exchange._processcompared. Tests pass.
>> I think it'd make for a simpler final state: with the current change, the client sees that the bookmark is going to move sideways, decides this is fine, requests that the server validates that the bookmark is indeed moving sideways (which it does), but in the end the server rejects the move. In the suggested change, the client sees that the bookmark is going sideways and rejects it. This should be consistent with the way new heads or new branches or diverging rewrites are prevented.
> Isn't that a client-side change only though, so we still need functionality on the server to reject bad pushes? (I could be missing something.)
If you mean "reject bad pushes as long as people don't push -f", both versions implement this (the client-side check is not racy thanks to these check:bookmark bundle parts).
If you mean "reject bad pushes, push -f or not", a server-side hook is necessary for that, and hook.pretxnclose-bookmark should be able to do the job without any server code change. Server code change can certainly simplify writing such checks, but passing the force parameter on the procol is not needed.
CHANGES SINCE LAST ACTION
To: idlsoft, #hg-reviewers, durin42
Cc: pulkit, durin42, valentin.gatienbaron, mercurial-devel
More information about the Mercurial-devel