This is an archive of the discontinued Mercurial Phabricator instance.

incoming: detect if server send partial replies
AbandonedPublic

Authored by joerg.sonnenberger on Apr 17 2019, 5:36 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Summary

incoming is not using the normal exchange logic and therefore doesn't
know how to tell with pullbundles. Fixing that is involved and it is
currently not sure if the current incoming code will survive, so apply a
band aid for now.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg requested changes to this revision.Apr 17 2019, 5:56 PM
indygreg added a subscriber: indygreg.

+1 for feature. Needs some minor work to get review.

mercurial/bundlerepo.py
639

localrepo.__contains__ can be slow in tight loops because it has to resolve self.changelog. Also, localrepo.__getitem__ (which __contains__ calls) accepts a myriad of different values (integers, hex and binary hashes, etc). We want to user a lower-level API to perform lookups. I think localrepo.changelog.hasnode() is what we want. Please alias that to a local to avoid the localrepo.changelog property resolution in a loop.

640

+1 for this feature. But the warning message is a bit ambiguous to me. How about something like warning: server sent partial data; not all remote changesets are available. This still isn't great. But I'm struggling to find a way to better state the issue here.

641

Same comment as above regarding the x in localrepo issues.

This revision now requires changes to proceed.Apr 17 2019, 5:56 PM
indygreg requested changes to this revision.Apr 19 2019, 11:07 AM
indygreg added inline comments.
mercurial/bundlerepo.py
642

Something I didn't notice before is that this function doesn't do any UI presentation. So it feels like a bug to print a message in this function assuming that fetched changesets will be displayed.

I suppose that means we'll need to include the "was partial reply" state in the return value and format it to a warning elsewhere.

Sorry for not catching this on first review. I intend to queue this patch for stable so it gets in the 5.0 release, as I think the improved UI is good to have.

This revision now requires changes to proceed.Apr 19 2019, 11:07 AM
joerg.sonnenberger marked 2 inline comments as done.Jul 17 2019, 10:02 AM
joerg.sonnenberger added inline comments.
mercurial/bundlerepo.py
642

We know that this whole code fragment for "incoming" is seriously limited and should go away. Do we really want to change the whole API for that one special use case?

Will be implemented properly.