This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5245

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

Authored by hooper on Nov 8 2018, 4:32 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHGad71c792a8d8: fix: add extra field to fixed revisions to avoid creating obsolescence cycles
Summary

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.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

hooper created this revision.Nov 8 2018, 4:32 PM
Closed by commit rHGad71c792a8d8: fix: add extra field to fixed revisions to avoid creating obsolescence cycles (authored by hooper). · Explain WhyNov 9 2018, 7:18 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/fix.py (16 lines)
Mtests/test-fix.t (34 lines)

Diff 12484

hgext/fix.py

there is no need to rebase them into the original topology later. there is no need to rebase them into the original topology later.
""" """
p1rev, p2rev = repo.changelog.parentrevs(ctx.rev()) p1rev, p2rev = repo.changelog.parentrevs(ctx.rev())
p1ctx, p2ctx = repo[p1rev], repo[p2rev] p1ctx, p2ctx = repo[p1rev], repo[p2rev]
newp1node = replacements.get(p1ctx.node(), p1ctx.node()) newp1node = replacements.get(p1ctx.node(), p1ctx.node())
newp2node = replacements.get(p2ctx.node(), p2ctx.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): def filectxfn(repo, memctx, path):
if path not in ctx: if path not in ctx:
return None return None
fctx = ctx[path] fctx = ctx[path]
copied = fctx.renamed() copied = fctx.renamed()
if copied: if copied:
copied = copied[0] copied = copied[0]
return context.memfilectx( return context.memfilectx(
repo, repo,
memctx, memctx,
path=fctx.path(), path=fctx.path(),
data=filedata.get(path, fctx.data()), data=filedata.get(path, fctx.data()),
islink=fctx.islink(), islink=fctx.islink(),
isexec=fctx.isexec(), isexec=fctx.isexec(),
copied=copied) copied=copied)
extra = ctx.extra().copy()
extra['fix_source'] = ctx.hex()
memctx = context.memctx( memctx = context.memctx(
repo, repo,
parents=(newp1node, newp2node), parents=(newp1node, newp2node),
text=ctx.description(), text=ctx.description(),
files=set(ctx.files()) | set(filedata.keys()), files=set(ctx.files()) | set(filedata.keys()),
filectxfn=filectxfn, filectxfn=filectxfn,
user=ctx.user(), user=ctx.user(),
date=ctx.date(), date=ctx.date(),
extra=ctx.extra(), extra=extra,
branch=ctx.branch(), branch=ctx.branch(),
editor=None) editor=None)
sucnode = memctx.commit() sucnode = memctx.commit()
prenode = ctx.node() prenode = ctx.node()
if prenode == sucnode: if prenode == sucnode:
ui.debug('node %s already existed\n' % (ctx.hex())) ui.debug('node %s already existed\n' % (ctx.hex()))
else: else:
replacements[ctx.node()] = sucnode replacements[ctx.node()] = sucnode

tests/test-fix.t

$ cat numbers.txt $ cat numbers.txt
2 2
3 3
6 6
7 7
8 8
$ cd .. $ 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 ..