This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10518

rewriteutil: check for divergence
ClosedPublic

Authored by martinvonz on Apr 27 2021, 6:53 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHGba6881c6a178: rewriteutil: check for divergence
Summary

This code is adapted from the code in the evolve extension. It seems
to be equivalent as far as the evolve extension's test suite can tell
(the only impact when making their precheck() delegate to our
version is that error messages are less detailed).

I had to change the error message to work with "change branch of"
being inserted as the action.

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

martinvonz created this revision.Apr 27 2021, 6:53 PM
martinvonz edited the summary of this revision. (Show Details)Apr 27 2021, 7:03 PM
pulkit added a subscriber: pulkit.Apr 28 2021, 6:23 AM
pulkit added inline comments.
tests/test-branch-change.t
153–154

s/of of/ of

pulkit accepted this revision.Apr 28 2021, 6:23 AM
This revision is now accepted and ready to land.Apr 28 2021, 6:23 AM
martinvonz added inline comments.Apr 28 2021, 11:43 AM
tests/test-branch-change.t
153–154

Good catch. The bug was actually already there: we sometimes would say "cannot change branch of while merging". It's a consequence of the simplistic way we produce these messages. We simply insert something that should be a verb into a sentence. I assume that's also not great for i18n. I suppose we could change the "verb" from "change branch of" to "branch-change", so it becomes these:

"cannot branch-change the null revision"
"cannot branch-change while merging"
"cannot branch-change public changesets"
"cannot branch-change changeset with children"
"branch-change of 3938acfb5c0f creates content-divergence with 7c1991464886'"

But it's probably better to change the messages to fit better. In
"cannot change branch of the null revision"
"cannot change branch of changeset while merging" (added "changeset")
"cannot change branch of public changesets"
"cannot change branch of changeset with children"
"cannot change branch of 3938acfb5c0f, as that creates content-divergence with 7c1991464886'"

I'll do the latter.

martinvonz added a commit: rHGba6881c6a178: rewriteutil: check for divergence.Apr 30 2021, 10:06 AM
Closed by commit rHGba6881c6a178: rewriteutil: check for divergence (authored by martinvonz). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 27249

hgext/rebase.py

# Calculate self.obsolete_* sets # Calculate self.obsolete_* sets
self._handleskippingobsolete() self._handleskippingobsolete()
if not self.keepf: if not self.keepf:
rebaseset = set(destmap.keys()) rebaseset = set(destmap.keys())
rebaseset -= set(self.obsolete_with_successor_in_destination) rebaseset -= set(self.obsolete_with_successor_in_destination)
rebaseset -= self.obsolete_with_successor_in_rebase_set rebaseset -= self.obsolete_with_successor_in_rebase_set
# We have our own divergence-checking in the rebase extension
overrides = {}
if obsolete.isenabled(self.repo, obsolete.createmarkersopt):
overrides = {
(b'experimental', b'evolution.allowdivergence'): b'true'
}
try: try:
with self.ui.configoverride(overrides):
rewriteutil.precheck(self.repo, rebaseset, action=b'rebase') rewriteutil.precheck(self.repo, rebaseset, action=b'rebase')
except error.Abort as e: except error.Abort as e:
if e.hint is None: if e.hint is None:
e.hint = _(b'use --keep to keep original changesets') e.hint = _(b'use --keep to keep original changesets')
raise e raise e
self.prepared = True self.prepared = True
def _assignworkingcopy(self): def _assignworkingcopy(self):

mercurial/obsolete.py

_pack = struct.pack _pack = struct.pack
_unpack = struct.unpack _unpack = struct.unpack
_calcsize = struct.calcsize _calcsize = struct.calcsize
propertycache = util.propertycache propertycache = util.propertycache
# Options for obsolescence # Options for obsolescence
createmarkersopt = b'createmarkers' createmarkersopt = b'createmarkers'
allowunstableopt = b'allowunstable' allowunstableopt = b'allowunstable'
allowdivergenceopt = b'allowdivergence'
exchangeopt = b'exchange' exchangeopt = b'exchange'
def _getoptionvalue(repo, option): def _getoptionvalue(repo, option):
"""Returns True if the given repository has the given obsolete option """Returns True if the given repository has the given obsolete option
enabled. enabled.
""" """
configkey = b'evolution.%s' % option configkey = b'evolution.%s' % option
return option in result return option in result
def getoptions(repo): def getoptions(repo):
"""Returns dicts showing state of obsolescence features.""" """Returns dicts showing state of obsolescence features."""
createmarkersvalue = _getoptionvalue(repo, createmarkersopt) createmarkersvalue = _getoptionvalue(repo, createmarkersopt)
unstablevalue = _getoptionvalue(repo, allowunstableopt) unstablevalue = _getoptionvalue(repo, allowunstableopt)
divergencevalue = _getoptionvalue(repo, allowdivergenceopt)
exchangevalue = _getoptionvalue(repo, exchangeopt) exchangevalue = _getoptionvalue(repo, exchangeopt)
# createmarkers must be enabled if other options are enabled # createmarkers must be enabled if other options are enabled
if (unstablevalue or exchangevalue) and not createmarkersvalue: if (
unstablevalue or divergencevalue or exchangevalue
) and not createmarkersvalue:
raise error.Abort( raise error.Abort(
_( _(
b"'createmarkers' obsolete option must be enabled " b"'createmarkers' obsolete option must be enabled "
b"if other obsolete options are enabled" b"if other obsolete options are enabled"
) )
) )
return { return {
createmarkersopt: createmarkersvalue, createmarkersopt: createmarkersvalue,
allowunstableopt: unstablevalue, allowunstableopt: unstablevalue,
allowdivergenceopt: divergencevalue,
exchangeopt: exchangevalue, exchangeopt: exchangevalue,
} }
def isenabled(repo, option): def isenabled(repo, option):
"""Returns True if the given repository has the given obsolete option """Returns True if the given repository has the given obsolete option
enabled. enabled.
""" """

mercurial/rewriteutil.py

hint = _(b"no changeset checked out") hint = _(b"no changeset checked out")
raise error.InputError(msg, hint=hint) raise error.InputError(msg, hint=hint)
if any(util.safehasattr(r, 'rev') for r in revs): if any(util.safehasattr(r, 'rev') for r in revs):
repo.ui.develwarn(b"rewriteutil.precheck called with ctx not revs") repo.ui.develwarn(b"rewriteutil.precheck called with ctx not revs")
revs = (r.rev() for r in revs) revs = (r.rev() for r in revs)
if len(repo[None].parents()) > 1: if len(repo[None].parents()) > 1:
raise error.StateError(_(b"cannot %s changesets while merging") % action) raise error.StateError(
_(b"cannot %s changesets while merging") % action
)
publicrevs = repo.revs(b'%ld and public()', revs) publicrevs = repo.revs(b'%ld and public()', revs)
if publicrevs: if publicrevs:
msg = _(b"cannot %s public changesets") % action msg = _(b"cannot %s public changesets") % action
hint = _(b"see 'hg help phases' for details") hint = _(b"see 'hg help phases' for details")
raise error.InputError(msg, hint=hint) raise error.InputError(msg, hint=hint)
newunstable = disallowednewunstable(repo, revs) newunstable = disallowednewunstable(repo, revs)
if newunstable: if newunstable:
hint = _(b"see 'hg help evolution.instability'") hint = _(b"see 'hg help evolution.instability'")
raise error.InputError( raise error.InputError(
_(b"cannot %s changeset with children") % action, hint=hint _(b"cannot %s changeset with children") % action, hint=hint
) )
if not obsolete.isenabled(repo, obsolete.allowdivergenceopt):
new_divergence = _find_new_divergence(repo, revs)
if new_divergence:
local_ctx, other_ctx, base_ctx = new_divergence
msg = _(
b'cannot %s %s, as that creates content-divergence with %s'
) % (
action,
local_ctx,
other_ctx,
)
if local_ctx.rev() != base_ctx.rev():
msg += _(b', from %s') % base_ctx
if repo.ui.verbose:
if local_ctx.rev() != base_ctx.rev():
msg += _(
b'\n changeset %s is a successor of ' b'changeset %s'
) % (local_ctx, base_ctx)
msg += _(
b'\n changeset %s already has a successor in '
b'changeset %s\n'
b' rewriting changeset %s would create '
b'"content-divergence"\n'
b' set experimental.evolution.allowdivergence=True to '
b'skip this check'
) % (base_ctx, other_ctx, local_ctx)
raise error.InputError(msg)
else:
raise error.InputError(
msg, hint=_(b"add --verbose for details")
)
def disallowednewunstable(repo, revs): def disallowednewunstable(repo, revs):
"""Checks whether editing the revs will create new unstable changesets and """Checks whether editing the revs will create new unstable changesets and
are we allowed to create them. are we allowed to create them.
To allow new unstable changesets, set the config: To allow new unstable changesets, set the config:
`experimental.evolution.allowunstable=True` `experimental.evolution.allowunstable=True`
""" """
allowunstable = obsolete.isenabled(repo, obsolete.allowunstableopt) allowunstable = obsolete.isenabled(repo, obsolete.allowunstableopt)
if allowunstable: if allowunstable:
return revset.baseset() return revset.baseset()
return repo.revs(b"(%ld::) - %ld", revs, revs) return repo.revs(b"(%ld::) - %ld", revs, revs)
def _find_new_divergence(repo, revs):
obsrevs = repo.revs(b'%ld and obsolete()', revs)
for r in obsrevs:
div = find_new_divergence_from(repo, repo[r])
if div:
return (repo[r], repo[div[0]], repo[div[1]])
return None
def find_new_divergence_from(repo, ctx):
"""return divergent revision if rewriting an obsolete cset (ctx) will
create divergence
Returns (<other node>, <common ancestor node>) or None
"""
if not ctx.obsolete():
return None
# We need to check two cases that can cause divergence:
# case 1: the rev being rewritten has a non-obsolete successor (easily
# detected by successorssets)
sset = obsutil.successorssets(repo, ctx.node())
if sset:
return (sset[0][0], ctx.node())
else:
# case 2: one of the precursors of the rev being revived has a
# non-obsolete successor (we need divergentsets for this)
divsets = obsutil.divergentsets(repo, ctx)
if divsets:
nsuccset = divsets[0][b'divergentnodes']
prec = divsets[0][b'commonpredecessor']
return (nsuccset[0], prec)
return None
def skip_empty_successor(ui, command): def skip_empty_successor(ui, command):
empty_successor = ui.config(b'rewrite', b'empty-successor') empty_successor = ui.config(b'rewrite', b'empty-successor')
if empty_successor == b'skip': if empty_successor == b'skip':
return True return True
elif empty_successor == b'keep': elif empty_successor == b'keep':
return False return False
else: else:
raise error.ConfigError( raise error.ConfigError(

tests/test-amend.t

$ hg add bar $ hg add bar
$ hg amend --note 'yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy' $ hg amend --note 'yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy'
abort: cannot store a note of more than 255 bytes abort: cannot store a note of more than 255 bytes
[10] [10]
$ hg amend --note "adding bar" $ hg amend --note "adding bar"
$ hg debugobsolete -r . $ hg debugobsolete -r .
112478962961147124edd43549aedd1a335e44bf be169c7e8dbe21cd10b3d79691cbe7f241e3c21c 0 (Thu Jan 01 00:00:00 1970 +0000) {'ef1': '8', 'operation': 'amend', 'user': 'test'} 112478962961147124edd43549aedd1a335e44bf be169c7e8dbe21cd10b3d79691cbe7f241e3c21c 0 (Thu Jan 01 00:00:00 1970 +0000) {'ef1': '8', 'operation': 'amend', 'user': 'test'}
be169c7e8dbe21cd10b3d79691cbe7f241e3c21c 16084da537dd8f84cfdb3055c633772269d62e1b 0 (Thu Jan 01 00:00:00 1970 +0000) {'ef1': '8', 'note': 'adding bar', 'operation': 'amend', 'user': 'test'} be169c7e8dbe21cd10b3d79691cbe7f241e3c21c 16084da537dd8f84cfdb3055c633772269d62e1b 0 (Thu Jan 01 00:00:00 1970 +0000) {'ef1': '8', 'note': 'adding bar', 'operation': 'amend', 'user': 'test'}
Cannot cause divergence by default
$ hg co --hidden 1
1 files updated, 0 files merged, 1 files removed, 0 files unresolved
$ hg amend -m divergent
abort: cannot amend 112478962961, as that creates content-divergence with 16084da537dd
(add --verbose for details)
[10]
$ hg amend -m divergent --config experimental.evolution.allowdivergence=true
2 new content-divergent changesets
#endif #endif
Cannot amend public changeset Cannot amend public changeset
$ hg phase -r A --public $ hg phase -r A --public
$ hg update -C -q A $ hg update -C -q A
$ hg amend -m AMEND $ hg amend -m AMEND
abort: cannot amend public changesets abort: cannot amend public changesets

tests/test-branch-change.t

Changing branch of an obsoleted changeset Changing branch of an obsoleted changeset
$ hg branch -r 4 foobar $ hg branch -r 4 foobar
abort: hidden revision '4' was rewritten as: 7c1991464886 abort: hidden revision '4' was rewritten as: 7c1991464886
(use --hidden to access hidden revisions) (use --hidden to access hidden revisions)
[255] [255]
$ hg branch -r 4 --hidden foobar $ hg branch -r 4 --hidden foobar
abort: cannot change branch of a obsolete changeset abort: cannot change branch of 3938acfb5c0f, as that creates content-divergence with 7c1991464886
(add --verbose for details)
pulkitUnsubmitted
Not Done

s/of of/ of

pulkit: `s/of of/ of`
martinvonzAuthorUnsubmitted
Done

Good catch. The bug was actually already there: we sometimes would say "cannot change branch of while merging". It's a consequence of the simplistic way we produce these messages. We simply insert something that should be a verb into a sentence. I assume that's also not great for i18n. I suppose we could change the "verb" from "change branch of" to "branch-change", so it becomes these:

"cannot branch-change the null revision"
"cannot branch-change while merging"
"cannot branch-change public changesets"
"cannot branch-change changeset with children"
"branch-change of 3938acfb5c0f creates content-divergence with 7c1991464886'"

But it's probably better to change the messages to fit better. In
"cannot change branch of the null revision"
"cannot change branch of changeset while merging" (added "changeset")
"cannot change branch of public changesets"
"cannot change branch of changeset with children"
"cannot change branch of 3938acfb5c0f, as that creates content-divergence with 7c1991464886'"

I'll do the latter.

martinvonz: Good catch. The bug was actually already there: we sometimes would say "cannot change branch of…
[10] [10]
Make sure bookmark movement is correct Make sure bookmark movement is correct
$ hg bookmark b1 $ hg bookmark b1
$ hg glog -r '.^::' $ hg glog -r '.^::'
@ 6:7c1991464886 Added e @ 6:7c1991464886 Added e
| bar (b1) | bar (b1)

tests/test-obshistory.t

This test file test the various messages when accessing obsolete This test file test the various messages when accessing obsolete
revisions. revisions.
Global setup Global setup
============ ============
$ . $TESTDIR/testlib/obsmarker-common.sh $ . $TESTDIR/testlib/obsmarker-common.sh
$ cat >> $HGRCPATH <<EOF $ cat >> $HGRCPATH <<EOF
> [ui] > [ui]
> interactive = true > interactive = true
> [phases] > [phases]
> publish=False > publish=False
> [experimental] > [experimental]
> evolution.createmarkers = yes > evolution.createmarkers = yes
> evolution.effect-flags = yes > evolution.effect-flags = yes
> evolution.allowdivergence=true
> EOF > EOF
Test output on amended commit Test output on amended commit
============================= =============================
Test setup Test setup
---------- ----------

tests/test-obsmarker-template.t

This test file test the various templates related to obsmarkers. This test file test the various templates related to obsmarkers.
Global setup Global setup
============ ============
$ . $TESTDIR/testlib/obsmarker-common.sh $ . $TESTDIR/testlib/obsmarker-common.sh
$ cat >> $HGRCPATH <<EOF $ cat >> $HGRCPATH <<EOF
> [ui] > [ui]
> interactive = true > interactive = true
> [phases] > [phases]
> publish=False > publish=False
> [experimental] > [experimental]
> evolution=true > evolution=true
> evolution.allowdivergence=true
> [templates] > [templates]
> obsfatesuccessors = "{if(successors, " as ")}{join(successors, ", ")}" > obsfatesuccessors = "{if(successors, " as ")}{join(successors, ", ")}"
> obsfateverb = "{obsfateverb(successors, markers)}" > obsfateverb = "{obsfateverb(successors, markers)}"
> obsfateoperations = "{if(obsfateoperations(markers), " using {join(obsfateoperations(markers), ", ")}")}" > obsfateoperations = "{if(obsfateoperations(markers), " using {join(obsfateoperations(markers), ", ")}")}"
> obsfateusers = "{if(obsfateusers(markers), " by {join(obsfateusers(markers), ", ")}")}" > obsfateusers = "{if(obsfateusers(markers), " by {join(obsfateusers(markers), ", ")}")}"
> obsfatedate = "{if(obsfatedate(markers), "{ifeq(min(obsfatedate(markers)), max(obsfatedate(markers)), " (at {min(obsfatedate(markers))|isodate})", " (between {min(obsfatedate(markers))|isodate} and {max(obsfatedate(markers))|isodate})")}")}" > obsfatedate = "{if(obsfatedate(markers), "{ifeq(min(obsfatedate(markers)), max(obsfatedate(markers)), " (at {min(obsfatedate(markers))|isodate})", " (between {min(obsfatedate(markers))|isodate} and {max(obsfatedate(markers))|isodate})")}")}"
> obsfatetempl = "{obsfateverb}{obsfateoperations}{obsfatesuccessors}{obsfateusers}{obsfatedate}; " > obsfatetempl = "{obsfateverb}{obsfateoperations}{obsfatesuccessors}{obsfateusers}{obsfatedate}; "
> [alias] > [alias]

tests/test-unamend.t

Test for command `hg unamend` which lives in uncommit extension Test for command `hg unamend` which lives in uncommit extension
=============================================================== ===============================================================
$ cat >> $HGRCPATH << EOF $ cat >> $HGRCPATH << EOF
> [alias] > [alias]
> glog = log -G -T '{rev}:{node|short} {desc}' > glog = log -G -T '{rev}:{node|short} {desc}'
> [experimental] > [experimental]
> evolution = createmarkers, allowunstable > evolution = createmarkers, allowunstable
> evolution.allowdivergence = true
> [extensions] > [extensions]
> rebase = > rebase =
> amend = > amend =
> uncommit = > uncommit =
> EOF > EOF
Repo Setup Repo Setup