D6218: narrow: send specs as bundle2 data instead of param (issue5952) (issue6019)
pulkit (Pulkit Goyal)
phabricator at mercurial-scm.org
Wed Apr 17 11:22:35 EDT 2019
pulkit added a comment.
In https://phab.mercurial-scm.org/D6218#91119, @martinvonz wrote:
> In https://phab.mercurial-scm.org/D6218#91058, @indygreg wrote:
> > This patch is backwards incompatible over the wire protocol.
> > The problem is a new client will blindly send part data to an old server expecting part parameters. The old server won't read the part data and it would be as if the includes and excludes were not sent.
> It's an experimental feature and I suspect it's used only by Sandu (@idlsoft). Sandu, if we released this without the capability negotiation that Greg is talking about, you would need to make sure to upgrade the server before you upgrade your client(s). Are you okay with that? Is anyone aware of any other users of this feature? Greg, are you okay with making a breaking change (to an experimental feature) if the few existing users are okay with it?
I agree with @martinvonz here. narrow extension is experimental right now, in 4.9 we had a lot of breaking changes. The narrowspecs are only send back in case when ACL is enabled. If there are users who rely on existing behavior, they must have hit the bug just like @idlsoft hit.
I am not sure how we can keep sending narrowspecs back using bundle2 param and fix the issues which this patch is trying to.
>> We need some kind of capability negotiation that allows the client to opt in to the newer behavior if the server advertises support for it.
>> Also, my personal preference is to create new bundle2 parts rather than change behavior of existing bundle2 parts. Doing things this way ensures that behavior for a named bundle2 part is constant over time. This keeps implementations simpler, as individual part handling can do one thing and one thing only.
>> Finally, the internals help docs should be updated to reflect changes to bundle2 part behavior.
To: pulkit, durin42, martinvonz, #hg-reviewers
Cc: indygreg, idlsoft, mercurial-devel
More information about the Mercurial-devel