fix: add extra field to fixed revisions to avoid creating obsolescence cycles
authorDanny Hooper <hooper@google.com>
Thu, 08 Nov 2018 12:35:26 -0800
changeset 40570 ad71c792a8d8
parent 40569 19e1c26213f1
child 40571 95a079ea1e19
fix: add extra field to fixed revisions to avoid creating obsolescence cycles The extra field prevents sequential invocations of fix from producing the same hash twice. Previously, this could cause problems because it would create an obsolescence cycle instead of the expected new successor. This change also adds an explicit check for whether a new revision should be committed. Until now, the code relied on memctx.commit() to quietly do nothing if the node already exists. Because of the new extra field, this no longer covers the case where we don't want to replace an unchanged node. Differential Revision: https://phab.mercurial-scm.org/D5245
hgext/fix.py
tests/test-fix.t
--- a/hgext/fix.py	Thu Nov 08 12:29:56 2018 -0800
+++ b/hgext/fix.py	Thu Nov 08 12:35:26 2018 -0800
@@ -586,6 +586,17 @@
     newp1node = replacements.get(p1ctx.node(), p1ctx.node())
     newp2node = replacements.get(p2ctx.node(), p2ctx.node())
 
+    # We don't want to create a revision that has no changes from the original,
+    # but we should if the original revision's parent has been replaced.
+    # Otherwise, we would produce an orphan that needs no actual human
+    # intervention to evolve. We can't rely on commit() to avoid creating the
+    # un-needed revision because the extra field added below produces a new hash
+    # regardless of file content changes.
+    if (not filedata and
+        p1ctx.node() not in replacements and
+        p2ctx.node() not in replacements):
+        return
+
     def filectxfn(repo, memctx, path):
         if path not in ctx:
             return None
@@ -602,6 +613,9 @@
             isexec=fctx.isexec(),
             copied=copied)
 
+    extra = ctx.extra().copy()
+    extra['fix_source'] = ctx.hex()
+
     memctx = context.memctx(
         repo,
         parents=(newp1node, newp2node),
@@ -610,7 +624,7 @@
         filectxfn=filectxfn,
         user=ctx.user(),
         date=ctx.date(),
-        extra=ctx.extra(),
+        extra=extra,
         branch=ctx.branch(),
         editor=None)
     sucnode = memctx.commit()
--- a/tests/test-fix.t	Thu Nov 08 12:29:56 2018 -0800
+++ b/tests/test-fix.t	Thu Nov 08 12:35:26 2018 -0800
@@ -1195,3 +1195,37 @@
   8
 
   $ cd ..
+
+It's possible for repeated applications of a fixer tool to create cycles in the
+generated content of a file. For example, two users with different versions of
+a code formatter might fight over the formatting when they run hg fix. In the
+absence of other changes, this means we could produce commits with the same
+hash in subsequent runs of hg fix. This is a problem unless we support
+obsolescence cycles well. We avoid this by adding an extra field to the
+successor which forces it to have a new hash. That's why this test creates
+three revisions instead of two.
+
+  $ hg init cyclictool
+  $ cd cyclictool
+
+  $ cat >> .hg/hgrc <<EOF
+  > [fix]
+  > swapletters:command = tr ab ba
+  > swapletters:pattern = foo
+  > EOF
+
+  $ echo ab > foo
+  $ hg commit -Aqm foo
+
+  $ hg fix -r 0
+  $ hg fix -r 1
+
+  $ hg cat -r 0 foo --hidden
+  ab
+  $ hg cat -r 1 foo --hidden
+  ba
+  $ hg cat -r 2 foo
+  ab
+
+  $ cd ..
+