D5245: fix: add extra field to fixed revisions to avoid creating obsolescence cycles

[hooper (Danny Hooper)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHGad71c792a8d8: fix: add extra field to fixed revisions to avoid creating obsolescence cycles (authored by hooper, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D5245?vs=12481&id=12484

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

AFFECTED FILES
  hgext/fix.py
  tests/test-fix.t

CHANGE DETAILS

diff --git a/tests/test-fix.t b/tests/test-fix.t
--- a/tests/test-fix.t
+++ b/tests/test-fix.t
@@ -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 ..
+
diff --git a/hgext/fix.py b/hgext/fix.py
--- a/hgext/fix.py
+++ b/hgext/fix.py
@@ -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,15 +613,18 @@
             isexec=fctx.isexec(),
             copied=copied)
 
+    extra = ctx.extra().copy()
+    extra['fix_source'] = ctx.hex()
+
     memctx = context.memctx(
         repo,
         parents=(newp1node, newp2node),
         text=ctx.description(),
         files=set(ctx.files()) | set(filedata.keys()),
         filectxfn=filectxfn,
         user=ctx.user(),
         date=ctx.date(),
-        extra=ctx.extra(),
+        extra=extra,
         branch=ctx.branch(),
         editor=None)
     sucnode = memctx.commit()



To: hooper, #hg-reviewers
Cc: mercurial-devel