Patchwork discovery: properly exclude locally known but filtered heads

login
register
mail settings
Submitter Pierre-Yves David
Date Jan. 30, 2015, 9:51 p.m.
Message ID <12216e8a6001039925fd.1422654711@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/7584/
State Accepted
Commit 3b7088a5c64c6e4a4e9dc9859ad86986c151ab58
Headers show

Comments

Pierre-Yves David - Jan. 30, 2015, 9:51 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1422652262 0
#      Fri Jan 30 21:11:02 2015 +0000
# Branch stable
# Node ID 12216e8a6001039925fd4ab61bb7aed92bc728f8
# Parent  6becb9dbca25057c6186e255a48dd2c2ce5701a5
discovery: properly exclude locally known but filtered heads

The conditional was a bit too narrow and produced buggy result when a node was
present in both common and heads (because it pleased the discovery) and it was
locally known but filtered.

This resulted in buggy getbundle request and server side crash.
Matt Mackall - Feb. 1, 2015, 8:14 p.m.
On Fri, 2015-01-30 at 21:51 +0000, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1422652262 0
> #      Fri Jan 30 21:11:02 2015 +0000
> # Branch stable
> # Node ID 12216e8a6001039925fd4ab61bb7aed92bc728f8
> # Parent  6becb9dbca25057c6186e255a48dd2c2ce5701a5
> discovery: properly exclude locally known but filtered heads

Queued for stable, thanks.

Patch

diff --git a/mercurial/exchange.py b/mercurial/exchange.py
--- a/mercurial/exchange.py
+++ b/mercurial/exchange.py
@@ -944,12 +944,13 @@  def _pulldiscoverychangegroup(pullop):
         # If a set of such "common but filtered" changeset exist on the server
         # but are not including a remote heads, we'll not be able to detect it,
         scommon = set(common)
         filteredrheads = []
         for n in rheads:
-            if n in nm and n not in scommon:
-                common.append(n)
+            if n in nm:
+                if n not in scommon:
+                    common.append(n)
             else:
                 filteredrheads.append(n)
         if not filteredrheads:
             fetch = []
         rheads = filteredrheads
diff --git a/tests/test-treediscovery.t b/tests/test-treediscovery.t
--- a/tests/test-treediscovery.t
+++ b/tests/test-treediscovery.t
@@ -518,11 +518,11 @@  Both have new stuff in existing named br
   "GET /?cmd=listkeys HTTP/1.1" 200 - x-hgarg-1:namespace=bookmarks
   "GET /?cmd=listkeys HTTP/1.1" 200 - x-hgarg-1:namespace=bookmarks
   "GET /?cmd=heads HTTP/1.1" 200 -
   "GET /?cmd=branches HTTP/1.1" 200 - x-hgarg-1:nodes=d8f638ac69e9ae8dea4f09f11d696546a912d961
   "GET /?cmd=between HTTP/1.1" 200 - x-hgarg-1:pairs=d8f638ac69e9ae8dea4f09f11d696546a912d961-d57206cc072a18317c1e381fb60aa31bd3401785
-  "GET /?cmd=changegroupsubset HTTP/1.1" 200 - x-hgarg-1:bases=d8f638ac69e9ae8dea4f09f11d696546a912d961&heads=d8f638ac69e9ae8dea4f09f11d696546a912d961+2c8d5d5ec612be65cdfdeac78b7662ab1696324a
+  "GET /?cmd=changegroupsubset HTTP/1.1" 200 - x-hgarg-1:bases=d8f638ac69e9ae8dea4f09f11d696546a912d961&heads=d8f638ac69e9ae8dea4f09f11d696546a912d961
   "GET /?cmd=listkeys HTTP/1.1" 200 - x-hgarg-1:namespace=phases
   "GET /?cmd=capabilities HTTP/1.1" 200 -
   "GET /?cmd=heads HTTP/1.1" 200 -
   "GET /?cmd=listkeys HTTP/1.1" 200 - x-hgarg-1:namespace=phases
   "GET /?cmd=listkeys HTTP/1.1" 200 - x-hgarg-1:namespace=bookmarks