This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8567

mergestate: implement trivial in-memory mergestate
AbandonedPublic

Authored by durin42 on May 18 2020, 6:08 PM.

Details

Reviewers
marmoute
Group Reviewers
hg-reviewers
Summary

This is the dumbest possible "mergestate" implementation: it doesn't actually
handle conflict storage in the slightest, but for the current in-memory merge
cases it works great, and prevents us from accidentally touching .hg/merge
while doing non-wdir operations.

NOT DONE: this breaks tests in future changes in the series, currently getting
stuck on the "premerge()" method in the mergestate object.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

durin42 created this revision.May 18 2020, 6:08 PM
martinvonz added a subscriber: martinvonz.May 19 2020, 3:20 AM
martinvonz added inline comments.
mercurial/mergestate.py
95

Can we really get the merge driver to work with memmergestate? Maybe it's better to make memmergestate.mergedriver raise an exception so we catch it?

Ah, I was curious how that could possibly work so far and it turns out that there is indeed a check at merge.py:2039. You could move that exception to memmergestate.mergedriver, I think.

durin42 updated this revision to Diff 21452.May 19 2020, 3:52 PM
durin42 updated this revision to Diff 21512.May 28 2020, 4:26 PM
durin42 updated this revision to Diff 21518.May 28 2020, 5:10 PM
durin42 added a comment.May 28 2020, 5:12 PM

This is now much closer! I'm down to a handful of failures, all like this:

--- tests/test-copies-in-changeset.t
+++ hgtest/tests/test-copies-in-changeset.t#sidedata.err
@@ -358,6 +358,8 @@
   $ hg ci -qm 'rename a to b'
   $ hg rebase -d 1 --config rebase.experimental.inmemory=yes
   rebasing 2:* "rename a to b" (tip) (glob)
+  hit merge conflicts; re-running rebase without in-memory merge
+  rebasing 2:b977edf6f839 "rename a to b" (tip)
   merging a and b to b
   saved backup bundle to $TESTTMP/rebase-rename/.hg/strip-backup/*-*-rebase.hg (glob)
   $ hg st --change . --copies

ERROR: test-copies-in-changeset.t#sidedata output changed
--- tests/test-rebase-inmemory.t
+++ tests/test-rebase-inmemory.t.err
@@ -780,6 +780,8 @@
   $ hg ci -qm 'rename a to b'
   $ hg rebase -d 1
   rebasing 2:b977edf6f839 "rename a to b" (tip)
+  hit merge conflicts; re-running rebase without in-memory merge
+  rebasing 2:b977edf6f839 "rename a to b" (tip)
   merging a and b to b
   saved backup bundle to $TESTTMP/rebase-rename/.hg/strip-backup/b977edf6f839-0864f570-rebase.hg
   $ hg st --copies --change .
@@ -801,6 +803,8 @@
   $ hg ci -qm 'rename a to b'
   $ hg rebase -d 1
   rebasing 2:b977edf6f839 "rename a to b" (tip)
+  hit merge conflicts; re-running rebase without in-memory merge
+  rebasing 2:b977edf6f839 "rename a to b" (tip)
   merging a and b to b
   saved backup bundle to $TESTTMP/rebase-rename-empty/.hg/strip-backup/b977edf6f839-0864f570-rebase.hg
   $ hg st --copies --change .
@@ -820,6 +824,8 @@
   $ echo a2 > a
   $ hg ci -qm 'modify a'
   $ hg rebase -r . -d 1 --collapse
+  rebasing 2:41c4ea50d4cf "modify a" (tip)
+  hit merge conflicts; re-running rebase without in-memory merge
   rebasing 2:41c4ea50d4cf "modify a" (tip)
   merging b and a to b
   saved backup bundle to $TESTTMP/rebase-rename-collapse/.hg/strip-backup/41c4ea50d4cf-b90b7994-rebase.hg

ERROR: test-rebase-inmemory.t output changed

Clearly I'm tickling some merge logic that previously went unused in the IMM case, and it'll take some investigating, but I suspect that long-term this will let us run more merges in-memory without having to revert to on-disk merge logic.

durin42 updated this revision to Diff 21519.May 28 2020, 5:33 PM
durin42 updated this revision to Diff 21544.Jun 1 2020, 11:50 AM
durin42 updated this revision to Diff 21611.Jun 11 2020, 12:07 PM
durin42 updated this revision to Diff 21620.Jun 11 2020, 1:04 PM
durin42 updated this revision to Diff 21623.Jun 11 2020, 3:45 PM
marmoute requested changes to this revision.Jul 31 2020, 12:02 PM
marmoute added a subscriber: marmoute.

According to IRC discussion, this series is not ready for review, so moving it out of yadda.

This revision now requires changes to proceed.Jul 31 2020, 12:02 PM
durin42 abandoned this revision.Oct 2 2020, 2:49 PM

Revision Contents
Changeset List

PathPackages
Mmercurial/mergestate.py (168 lines)

Diff 21620

mercurial/mergestate.py

ACTION_DIR_RENAME_MOVE_LOCAL = b'dm' ACTION_DIR_RENAME_MOVE_LOCAL = b'dm'
ACTION_KEEP = b'k' ACTION_KEEP = b'k'
ACTION_EXEC = b'e' ACTION_EXEC = b'e'
ACTION_CREATED_MERGE = b'cm' ACTION_CREATED_MERGE = b'cm'
# GET the other/remote side and store this info in mergestate # GET the other/remote side and store this info in mergestate
ACTION_GET_OTHER_AND_STORE = b'gs' ACTION_GET_OTHER_AND_STORE = b'gs'
class mergestate(object): class _basemergestate(object):
def __init__(self, repo):
self._repo = repo
self._readmergedriver = None
@util.propertycache
def mergedriver(self):
martinvonzUnsubmitted
Not Done

Can we really get the merge driver to work with memmergestate? Maybe it's better to make memmergestate.mergedriver raise an exception so we catch it?

Ah, I was curious how that could possibly work so far and it turns out that there is indeed a check at merge.py:2039. You could move that exception to memmergestate.mergedriver, I think.

martinvonz: Can we really get the merge driver to work with `memmergestate`? Maybe it's better to make…
# protect against the following:
# - A configures a malicious merge driver in their hgrc, then
# pauses the merge
# - A edits their hgrc to remove references to the merge driver
# - A gives a copy of their entire repo, including .hg, to B
# - B inspects .hgrc and finds it to be clean
# - B then continues the merge and the malicious merge driver
# gets invoked
configmergedriver = self._repo.ui.config(
b'experimental', b'mergedriver'
)
if (
self._readmergedriver is not None
and self._readmergedriver != configmergedriver
):
raise error.ConfigError(
_(b"merge driver changed since merge started"),
hint=_(b"revert merge driver change or abort merge"),
)
return configmergedriver
def reset(self, node=None, other=None, labels=None):
self._readmergedriver = None
class mergestate(_basemergestate):
'''track 3-way merge state of individual files '''track 3-way merge state of individual files
The merge state is stored on disk when needed. Two files are used: one with The merge state is stored on disk when needed. Two files are used: one with
an old format (version 1), and one with a new format (version 2). Version 2 an old format (version 1), and one with a new format (version 2). Version 2
stores a superset of the data in version 1, including new kinds of records stores a superset of the data in version 1, including new kinds of records
in the future. For more about the new format, see the documentation for in the future. For more about the new format, see the documentation for
`_readrecordsv2`. `_readrecordsv2`.
ms = mergestate(repo) ms = mergestate(repo)
ms._read() ms._read()
return ms return ms
def __init__(self, repo): def __init__(self, repo):
"""Initialize the merge state. """Initialize the merge state.
Do not use this directly! Instead call read() or clean().""" Do not use this directly! Instead call read() or clean()."""
self._repo = repo super(mergestate, self).__init__(repo)
self._dirty = False self._dirty = False
self._labels = None self._labels = None
def reset(self, node=None, other=None, labels=None): def reset(self, node=None, other=None, labels=None):
super(mergestate, self).reset(node=node, other=other, labels=labels)
self._state = {} self._state = {}
self._stateextras = {} self._stateextras = {}
self._local = None self._local = None
self._other = None self._other = None
self._labels = labels self._labels = labels
for var in ('localctx', 'otherctx'): for var in ('localctx', 'otherctx'):
if var in vars(self): if var in vars(self):
delattr(self, var) delattr(self, var)
if node: if node:
self._local = node self._local = node
self._other = other self._other = other
self._readmergedriver = None
if self.mergedriver: if self.mergedriver:
self._mdstate = MERGE_DRIVER_STATE_SUCCESS self._mdstate = MERGE_DRIVER_STATE_SUCCESS
else: else:
self._mdstate = MERGE_DRIVER_STATE_UNMARKED self._mdstate = MERGE_DRIVER_STATE_UNMARKED
shutil.rmtree(self._repo.vfs.join(b'merge'), True) shutil.rmtree(self._repo.vfs.join(b'merge'), True)
self._results = {} self._results = {}
self._dirty = False self._dirty = False
records.append((rtype, record)) records.append((rtype, record))
f.close() f.close()
except IOError as err: except IOError as err:
if err.errno != errno.ENOENT: if err.errno != errno.ENOENT:
raise raise
return records return records
@util.propertycache @util.propertycache
def mergedriver(self):
# protect against the following:
# - A configures a malicious merge driver in their hgrc, then
# pauses the merge
# - A edits their hgrc to remove references to the merge driver
# - A gives a copy of their entire repo, including .hg, to B
# - B inspects .hgrc and finds it to be clean
# - B then continues the merge and the malicious merge driver
# gets invoked
configmergedriver = self._repo.ui.config(
b'experimental', b'mergedriver'
)
if (
self._readmergedriver is not None
and self._readmergedriver != configmergedriver
):
raise error.ConfigError(
_(b"merge driver changed since merge started"),
hint=_(b"revert merge driver change or abort merge"),
)
return configmergedriver
@util.propertycache
def local(self): def local(self):
if self._local is None: if self._local is None:
msg = b"local accessed but self._local isn't set" msg = b"local accessed but self._local isn't set"
raise error.ProgrammingError(msg) raise error.ProgrammingError(msg)
return self._local return self._local
@util.propertycache @util.propertycache
def localctx(self): def localctx(self):
# directory rename, get # directory rename, get
for f, args, msg in actions.get(ACTION_LOCAL_DIR_RENAME_GET, []): for f, args, msg in actions.get(ACTION_LOCAL_DIR_RENAME_GET, []):
f0, flag = args f0, flag = args
if branchmerge: if branchmerge:
repo.dirstate.add(f) repo.dirstate.add(f)
repo.dirstate.copy(f0, f) repo.dirstate.copy(f0, f)
else: else:
repo.dirstate.normal(f) repo.dirstate.normal(f)
class memmergestate(_basemergestate):
def __init__(self, repo, ctx):
super(memmergestate, self).__init__(repo)
self._basectx = ctx
self.reset()
self._ancestor_filectxs = {}
def __iter__(self):
return iter(self._state)
def add(self, fcl, fco, fca, fd):
"""add a new (potentially?) conflicting file to the merge state"""
self._conflicts.add(fcl.path())
# TODO(augie): the rebase codepath depends on non-implicit
# ancestor. I think we should fix things so that ancestor can
# be passed in to reset().
self._ancestor_filectxs[fcl.path()] = fca
# Since memmergestate isn't mutable yet, these are all trivial
# implementations used by the "happy path" in merge code.
def reset(self, node=None, other=None, labels=None):
super(memmergestate, self).reset(node=node, other=other, labels=labels)
self._local = node
self._other = other
if self._local is not None:
try:
self._ancestor_ctx = next(
self._repo.set(b'ancestor(%ln)', (node, other))
)
except StopIteration:
# This will almost certainly be an issue, but if
# callers consistently pass a filectx in add() when we
# can't figure out the ancestor this won't matter. It
# would be a better API for reset() to take an
# ancestor ctx, but that's a broader cleanup.
self._ancestor_ctx = None
self._labels = labels
self._conflicts = set()
def commit(self):
if self._conflicts:
error.InMemoryMergeConflictsError(
'cannot commit memmergestate with conflicts, have %d conflicts'
% self.unresolvedcount()
)
def counts(self):
return 0, 0, 0
def unresolvedcount(self):
return len(self._conflicts)
def actions(self):
return {}
@property
def mergedriver(self):
md = super(memmergestate, self).mergedriver
if md:
raise error.InMemoryMergeConflictsError(
b"in-memory merge does not support mergedriver"
)
return md
def addmergedother(self, path):
# I'm very dubious this is right.
pass
def preresolve(self, dfile, wctx):
return self._resolve(True, dfile, wctx)
def resolve(self, dfile, wctx):
return self._resolve(False, dfile, wctx)[1]
def _resolve(self, preresolve, dfile, wctx):
# TODO: clean up _filectxorabsent API?
fcd = _filectxorabsent(
nullhex if dfile not in wctx else None, wctx, dfile
)
octx = self._repo[self._other]
fco = _filectxorabsent(
nullhex if dfile not in octx else None, octx, dfile
)
actx = self._ancestor_ctx
if dfile in self._ancestor_filectxs:
fca = self._ancestor_filectxs[dfile]
else:
fca = _filectxorabsent(
nullhex if dfile not in actx else None, actx, dfile
)
fn = filemerge.premerge if preresolve else filemerge.filemerge
complete, mergeret, deleted = fn(
self._repo,
wctx,
self._local,
dfile, # orig
fcd,
fco,
fca,
labels=self._labels,
)
del deleted # unused
return complete, mergeret