Patchwork D1857: pull: re-run discovery and pullbundle2 if server didn't send all heads

login
register
mail settings
Submitter phabricator
Date Jan. 12, 2018, 10:08 p.m.
Message ID <differential-rev-PHID-DREV-2hgrapajaiydx2b5uxle-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/26708/
State Superseded
Headers show

Comments

phabricator - Jan. 12, 2018, 10:08 p.m.
joerg.sonnenberger created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/exchange.py
  tests/test-pull-r.t

CHANGE DETAILS




To: joerg.sonnenberger, #hg-reviewers
Cc: mercurial-devel
phabricator - Jan. 12, 2018, 10:27 p.m.
joerg.sonnenberger added a comment.


  This is a proof-of-concept. There is an interaction with obsoletion that I am still trying to understand (and fix).

REPOSITORY
  rHG Mercurial

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

To: joerg.sonnenberger, #hg-reviewers
Cc: mercurial-devel
phabricator - Jan. 13, 2018, 11:12 a.m.
joerg.sonnenberger added a comment.


  With the last update, the obs issues are resolved.

REPOSITORY
  rHG Mercurial

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

To: joerg.sonnenberger, #hg-reviewers
Cc: mercurial-devel
phabricator - Jan. 14, 2018, 10:04 p.m.
indygreg requested changes to this revision.
indygreg added a comment.
This revision now requires changes to proceed.


  I'm generally in favor of this functionality. It enables some interesting server features (such as pullbundles).
  
  This review needs to get hooked up to the Phabricator "stack" as the pullbundles patch(es). Using `hg phabsend A::B` should do that.
  
  I'd like to see a test around a server not sending changegroup data. I /think/ the existing code will abort the `while True` loop in this case. But I don't fully understand when various attributes on `pullop` are updated. I'd also feel more comfortable about things if there were an explicit check in the loop that the tip of changelog increased if revisions were requested. I'm worried about getting into an infinite loop due to a misbehaving server. I /think/ I'd like to see the establishment of a //contract// that if revision data is requested, the server **MUST** respond with revision data.

REPOSITORY
  rHG Mercurial

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

To: joerg.sonnenberger, #hg-reviewers, indygreg
Cc: indygreg, mercurial-devel
phabricator - Jan. 15, 2018, 7:26 p.m.
joerg.sonnenberger abandoned this revision.
joerg.sonnenberger added a comment.


  Merging into the parent review.

REPOSITORY
  rHG Mercurial

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

To: joerg.sonnenberger, #hg-reviewers, indygreg
Cc: indygreg, mercurial-devel

Patch

diff --git a/tests/test-pull-r.t b/tests/test-pull-r.t
--- a/tests/test-pull-r.t
+++ b/tests/test-pull-r.t
@@ -201,3 +201,40 @@ 
   sending pullbundle "0.hg"
   sending pullbundle "1.hg"
   sending pullbundle "2.hg"
+
+Test pullbundle functionality for incremental pulls
+
+  $ cd repo
+  $ hg serve --debug -p $HGPORT2 --pid-file=../repo.pid > ../repo-server.txt 2>&1 &
+  $ while ! grep listening ../repo-server.txt > /dev/null; do sleep 1; done
+  $ cat ../repo.pid >> $DAEMON_PIDS
+  $ cd ..
+  $ hg clone http://localhost:$HGPORT2/ repo.pullbundle2
+  requesting all changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files (+1 heads)
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  new changesets bbd179dfa0a7:66e3ba28d0d7
+  updating to branch default
+  3 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  $ killdaemons.py
+  $ grep 'sending pullbundle ' repo-server.txt
+  sending pullbundle "0.hg"
+  sending pullbundle "2.hg"
+  sending pullbundle "1.hg"
diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -1351,9 +1351,15 @@ 
         # before discovery to avoid extra work.
         _maybeapplyclonebundle(pullop)
         streamclone.maybeperformlegacystreamclone(pullop)
-        _pulldiscovery(pullop)
-        if pullop.canusebundle2:
+        while True:
+            _pulldiscovery(pullop)
+            if not pullop.canusebundle2:
+                break
             _pullbundle2(pullop)
+            if not pullop.cgresult or not pullop.rheads:
+                break
+            if all(repo.changelog.hasnode(n) for n in pullop.rheads):
+                break
         _pullchangeset(pullop)
         _pullphase(pullop)
         _pullbookmarks(pullop)