obsolete: cheap detection of nullid as successors
authorPierre-Yves David <pierre-yves.david@logilab.fr>
Mon, 15 Oct 2012 00:12:06 +0200
changeset 17774 0496d4f73cf4
parent 17773 434e5bd615fc
child 17775 13744acc4ad7
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.
mercurial/obsolete.py
tests/test-obsolete.t
--- a/mercurial/obsolete.py	Tue Oct 16 11:43:15 2012 -0700
+++ b/mercurial/obsolete.py	Mon Oct 15 00:12:06 2012 +0200
@@ -52,7 +52,7 @@
   cannot contain '\0'.
 """
 import struct
-import util, base85
+import util, base85, node
 from i18n import _
 
 _pack = struct.pack
@@ -237,6 +237,9 @@
             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
--- a/tests/test-obsolete.t	Tue Oct 16 11:43:15 2012 -0700
+++ b/tests/test-obsolete.t	Mon Oct 15 00:12:06 2012 +0200
@@ -100,6 +100,13 @@
   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