D4813: narrow: the first version of narrow_widen wireprotocol command

[pulkit (Pulkit Goyal)]

pulkit added inline comments.

INLINE COMMENTS

> martinvonz wrote in narrowcommands.py:293-295
> As I said on IRC, I think we (Google) would appreciate it if this could remain here for a while to give us more time to migrate our custom server to the new wire protocol command. I was thinking we could just fall back to the getbundle command if narrow_widen isn't supported.  That probably means changing the capability to "exp-narrow-2" now and call getbundle if "exp-narrow-1" is supported but "exp-narrow-2" isn't. That shouldn't be very difficult, but I don't think we've done kind of thing before.
> 
> Actually, you can ignore all that because our server has the ellipses capability, so it will not get here anyway :) We should instead think about such a migration path when we add support for ellipses to narrow_widen.

We can have a 'exp-ellipses-2' which will tell whether the server supports ellipses widening using narrow_widen() wireprotocol command or not. I think that should help in the meantime. Also will a week, or 10-15 days be enough for you? I think it will be better if can prevent releasing this compatibility because exp-ellipses-1 was introduced in this cycle only.

> martinvonz wrote in narrowcommands.py:302
> Should we call this something like "heads" instead? It's the set of common heads between the server and client and "common" makes sense together with "heads" in the getbundle call, but it doesn't seem to make as much sense here.

Made this commonheads which makes more sense. Thanks for the suggestion!

> martinvonz wrote in narrowcommands.py:307-310
> It looks like this doesn't need to be in in the commandexecutor block and the transaction probably doesn't need to span the wire protocol request. IOW, I think you can drop `repo.transaction()`from line 293 and instead put that togetgher this with this with-block.

Did that, thanks!

> martinvonz wrote in narrowwirepeer.py:49
> Should we just put this in core from the beginning?

I implemented the peer initially in core only, but while implementing server side, I realized it rely on logic in narrowbundle2.py which also needs to be moved to core. Then I decided to implement it cleanly in the extension and then move it to core.

> martinvonz wrote in narrowwirepeer.py:79-82
> I suppose we want the default to be False?  Isn't `ellipses = (args.get('ellipses') == '1')` enough? Or is the idea to be flexible with what values we accept? Is that what other wire protocol commands do?

I am not sure, I just copied from getbundle() handling: https://www.mercurial-scm.org/repo/hg/file/1a4c1a3cc3f5/mercurial/wireprotov1server.py#l404

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D4813

To: pulkit, durin42, #hg-reviewers, martinvonz
Cc: indygreg, mercurial-devel