[PATCH 1 of 9 V2] obsolete: cheap detection of nullid as successors

pierre-yves.david at logilab.fr pierre-yves.david at logilab.fr
Mon Oct 15 07:46:15 CDT 2012


# HG changeset patch
# User Pierre-Yves David <pierre-yves.david at logilab.fr>
# Date 1350252726 -7200
# Node ID d5311633f0fc23b2fa7a1d9189fe082cb1cf2df9
# Parent  9837cafc25b10c51659b65b5971622eab0bc9197
obsolete: cheap detection of nullid as successors

Nullid as successors create multiple issues:

- Nullid revnum is -1, confusing algorithm that use revnum unless you add
  special handling in all of them.
- Nullid confuses "divergent" changeset detection and resolution. As you can't
  add any successors to Nullid without being in even more troubles

Fortunately, there is no good reason to use nullid as a successor. The only
sensible meaning of "succeed by nullid" is "dropped" and this meaning is already
covered by obsolescence marker with empty successors set.

However, letting some nullid successors to slip in may cause terrible damage in
such algorithm difficult to debug. So I prefer to perform and clear detection of
of such pathological changeset. We could be much smarter by cleaning up nullid
successors on the fly but it would be much for expensive. As core Mercurial does
not create any such changeset, I think it is fine to just abort when suspicious
situation is detected.

Earlier experimental version created such changesets, so there are some out
there.  The evolve extension added the necessary logic to clean up its mess.

diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
--- a/mercurial/obsolete.py
+++ b/mercurial/obsolete.py
@@ -50,11 +50,11 @@ The header is followed by the markers. E
   string contains a key and a value, separated by a color ':', without
   additional encoding. Keys cannot contain '\0' or ':' and values
   cannot contain '\0'.
 """
 import struct
-import util, base85
+import util, base85, node
 from i18n import _
 
 _pack = struct.pack
 _unpack = struct.unpack
 
@@ -235,10 +235,13 @@ class obsstore(object):
             self._all.append(mark)
             pre, sucs = mark[:2]
             self.precursors.setdefault(pre, set()).add(mark)
             for suc in sucs:
                 self.successors.setdefault(suc, set()).add(mark)
+        if node.nullid in self.successors:
+            raise util.Abort(_('bad obsolescence marker detected: '
+                               'invalid successors nullid'))
 
 def _encodemarkers(markers, addheader=False):
     # Kept separate from flushmarkers(), it will be reused for
     # markers exchange.
     if addheader:
diff --git a/tests/test-obsolete.t b/tests/test-obsolete.t
--- a/tests/test-obsolete.t
+++ b/tests/test-obsolete.t
@@ -98,10 +98,17 @@ Register two markers with a missing node
   245bde4270cd1072a27757984f9cda8ba26f08ca cdbce2fbb16313928851e97e0d85413f3f7eb77f 0 {'date': '56 12', 'user': 'test'}
   cdbce2fbb16313928851e97e0d85413f3f7eb77f ca819180edb99ed25ceafb3e9584ac287e240b00 0 {'date': '1337 0', 'user': 'test'}
   ca819180edb99ed25ceafb3e9584ac287e240b00 1337133713371337133713371337133713371337 0 {'date': '1338 0', 'user': 'test'}
   1337133713371337133713371337133713371337 5601fb93a350734d935195fee37f4054c529ff39 0 {'date': '1339 0', 'user': 'test'}
 
+Refuse pathological nullid successors
+  $ hg debugobsolete -d '9001 0' 1337133713371337133713371337133713371337 0000000000000000000000000000000000000000
+  transaction abort!
+  rollback completed
+  abort: bad obsolescence marker detected: invalid successors nullid
+  [255]
+
 Check that graphlog detect that a changeset is obsolete:
 
   $ hg glog
   @  changeset:   5:5601fb93a350
   |  tag:         tip


More information about the Mercurial-devel mailing list