This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12119

narrow: allow merging non-conflicting change outside of the narrow spec
ClosedPublic

Authored by marmoute on Jan 30 2022, 3:15 PM.

Details

Reviewers
durin42
Alphare
Group Reviewers
hg-reviewers
Commits
rHGf1eb77dceb36: narrow: allow merging non-conflicting change outside of the narrow spec
Summary

We use the mergestate to carry information about these merge action and
reprocess them at commit time to apply the necessary update.

The dirstate itself is never affected and remains "pure", with content only in
the narrow-spec. This file involved in such merge are therefor not listed in `hg
status`.

The current testing is based on a modification of the previous testing, that
refused to do such merges. As a result it is a bit simple and more extensive
code and testing testing will have to be introduced later. I am planning to do
this extra testing, soon.

In addition, this only works for flat manifest. Support for tree manifest will
need more work. I am not currently planning to do this work.

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

marmoute created this revision.Jan 30 2022, 3:15 PM
Alphare requested changes to this revision.Feb 3 2022, 4:33 AM
Alphare added a subscriber: Alphare.

I don't think we can forgo a test of a simple added and removed case.

mercurial/commit.py
217

Should be mergestate.CHANGE_ADDED. This makes me suspect that the testing is not extensive enough even with the limited scope mentioned in the commit message

230

unknown*

This revision now requires changes to proceed.Feb 3 2022, 4:34 AM
marmoute added inline comments.Feb 3 2022, 10:57 AM
mercurial/commit.py
217

Yeah the removed/added case are not tested yet. I'll just drop them from this patch and reintroduce them later with the testing.

baymax updated this revision to Diff 32053.Feb 3 2022, 11:55 AM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

baymax edited the summary of this revision. (Show Details)Feb 3 2022, 1:23 PM
baymax updated this revision to Diff 32058.

✅ refresh by Heptapod after a successful CI run (🐙 💚)

Alphare accepted this revision.Feb 4 2022, 5:24 AM
This revision is now accepted and ready to land.Feb 4 2022, 5:24 AM

Revision Contents
Changeset List

PathPackages
Mmercurial/commit.py (62 lines)
Mmercurial/merge.py (24 lines)
Mtests/test-narrow-merge.t (57 lines)

Diff 32077

mercurial/commit.py

writechangesetcopy, writefilecopymeta = _write_copy_meta(repo) writechangesetcopy, writefilecopymeta = _write_copy_meta(repo)
files = metadata.ChangingFiles() files = metadata.ChangingFiles()
ms = mergestate.mergestate.read(repo) ms = mergestate.mergestate.read(repo)
salvaged = _get_salvaged(repo, ms, ctx) salvaged = _get_salvaged(repo, ms, ctx)
for s in salvaged: for s in salvaged:
files.mark_salvaged(s) files.mark_salvaged(s)
if ctx.manifestnode(): narrow_files = {}
if not ctx.repo().narrowmatch().always():
for f, e in ms.allextras().items():
action = e.get(b'outside-narrow-merge-action')
if action is not None:
narrow_files[f] = action
if ctx.manifestnode() and not narrow_files:
# reuse an existing manifest revision # reuse an existing manifest revision
repo.ui.debug(b'reusing known manifest\n') repo.ui.debug(b'reusing known manifest\n')
mn = ctx.manifestnode() mn = ctx.manifestnode()
files.update_touched(ctx.files()) files.update_touched(ctx.files())
if writechangesetcopy: if writechangesetcopy:
files.update_added(ctx.filesadded()) files.update_added(ctx.filesadded())
files.update_removed(ctx.filesremoved()) files.update_removed(ctx.filesremoved())
elif not ctx.files(): elif not ctx.files() and not narrow_files:
repo.ui.debug(b'reusing manifest from p1 (no file change)\n') repo.ui.debug(b'reusing manifest from p1 (no file change)\n')
mn = p1.manifestnode() mn = p1.manifestnode()
else: else:
mn = _process_files(tr, ctx, ms, files, error=error) mn = _process_files(tr, ctx, ms, files, narrow_files, error=error)
if origctx and origctx.manifestnode() == mn: if origctx and origctx.manifestnode() == mn:
origfiles = origctx.files() origfiles = origctx.files()
assert files.touched.issubset(origfiles) assert files.touched.issubset(origfiles)
files.update_touched(origfiles) files.update_touched(origfiles)
if writechangesetcopy: if writechangesetcopy:
files.update_copies_from_p1(ctx.p1copies()) files.update_copies_from_p1(ctx.p1copies())
for fname in sorted(ms.allextras().keys()): for fname in sorted(ms.allextras().keys()):
might_removed = ms.extras(fname).get(b'merge-removal-candidate') might_removed = ms.extras(fname).get(b'merge-removal-candidate')
if might_removed == b'yes': if might_removed == b'yes':
if fname in ctx: if fname in ctx:
salvaged.append(fname) salvaged.append(fname)
return salvaged return salvaged
def _process_files(tr, ctx, ms, files, error=False): def _process_files(tr, ctx, ms, files, narrow_files=None, error=False):
repo = ctx.repo() repo = ctx.repo()
p1 = ctx.p1() p1 = ctx.p1()
p2 = ctx.p2() p2 = ctx.p2()
writechangesetcopy, writefilecopymeta = _write_copy_meta(repo) writechangesetcopy, writefilecopymeta = _write_copy_meta(repo)
m1ctx = p1.manifestctx() m1ctx = p1.manifestctx()
m2ctx = p2.manifestctx() m2ctx = p2.manifestctx()
mctx = m1ctx.copy() mctx = m1ctx.copy()
m = mctx.read() m = mctx.read()
m1 = m1ctx.read() m1 = m1ctx.read()
m2 = m2ctx.read() m2 = m2ctx.read()
# check in files # check in files
added = [] added = []
removed = list(ctx.removed()) removed = list(ctx.removed())
linkrev = len(repo) linkrev = len(repo)
repo.ui.note(_(b"committing files:\n")) repo.ui.note(_(b"committing files:\n"))
uipathfn = scmutil.getuipathfn(repo) uipathfn = scmutil.getuipathfn(repo)
for f in sorted(ctx.modified() + ctx.added()): all_files = ctx.modified() + ctx.added()
all_files.extend(narrow_files.keys())
all_files.sort()
for f in all_files:
repo.ui.note(uipathfn(f) + b"\n") repo.ui.note(uipathfn(f) + b"\n")
if f in narrow_files:
narrow_action = narrow_files.get(f)
if narrow_action == mergestate.CHANGE_MODIFIED:
files.mark_touched(f)
added.append(f)
m[f] = m2[f]
AlphareUnsubmitted
Not Done

Should be mergestate.CHANGE_ADDED. This makes me suspect that the testing is not extensive enough even with the limited scope mentioned in the commit message

Alphare: Should be `mergestate.CHANGE_ADDED`. This makes me suspect that the testing is not extensive…
marmouteAuthorUnsubmitted
Done

Yeah the removed/added case are not tested yet. I'll just drop them from this patch and reintroduce them later with the testing.

marmoute: Yeah the removed/added case are not tested yet. I'll just drop them from this patch and…
flags = m2ctx.find(f)[1] or b''
m.setflag(f, flags)
else:
msg = _(b"corrupted mergestate, unknown narrow action: %b")
hint = _(b"restart the merge")
raise error.Abort(msg, hint=hint)
continue
try: try:
fctx = ctx[f] fctx = ctx[f]
if fctx is None: if fctx is None:
removed.append(f) removed.append(f)
else: else:
added.append(f) added.append(f)
AlphareUnsubmitted
Not Done

unknown*

Alphare: unknown*
m[f], is_touched = _filecommit( m[f], is_touched = _filecommit(
repo, fctx, m1, m2, linkrev, tr, writefilecopymeta, ms repo, fctx, m1, m2, linkrev, tr, writefilecopymeta, ms
) )
if is_touched: if is_touched:
if is_touched == 'added': if is_touched == 'added':
files.mark_added(f) files.mark_added(f)
elif is_touched == 'merged': elif is_touched == 'merged':
files.mark_merged(f) files.mark_merged(f)
if p2.rev() == nullrev: if p2.rev() == nullrev:
files.update_removed(removed) files.update_removed(removed)
else: else:
rf = metadata.get_removal_filter(ctx, (p1, p2, m1, m2)) rf = metadata.get_removal_filter(ctx, (p1, p2, m1, m2))
for f in removed: for f in removed:
if not rf(f): if not rf(f):
files.mark_removed(f) files.mark_removed(f)
mn = _commit_manifest(tr, linkrev, ctx, mctx, m, files.touched, added, drop) mn = _commit_manifest(
tr,
linkrev,
ctx,
mctx,
m,
files.touched,
added,
drop,
bool(narrow_files),
)
return mn return mn
def _filecommit( def _filecommit(
repo, repo,
fctx, fctx,
manifest1, manifest1,
elif fname in manifest1 and manifest1.flags(fname) != fctx.flags(): elif fname in manifest1 and manifest1.flags(fname) != fctx.flags():
touched = 'modified' touched = 'modified'
fnode = fparent1 fnode = fparent1
else: else:
fnode = fparent1 fnode = fparent1
return fnode, touched return fnode, touched
def _commit_manifest(tr, linkrev, ctx, mctx, manifest, files, added, drop): def _commit_manifest(
tr,
linkrev,
ctx,
mctx,
manifest,
files,
added,
drop,
has_some_narrow_action=False,
):
"""make a new manifest entry (or reuse a new one) """make a new manifest entry (or reuse a new one)
given an initialised manifest context and precomputed list of given an initialised manifest context and precomputed list of
- files: files affected by the commit - files: files affected by the commit
- added: new entries in the manifest - added: new entries in the manifest
- drop: entries present in parents but absent of this one - drop: entries present in parents but absent of this one
Create a new manifest revision, reuse existing ones if possible. Create a new manifest revision, reuse existing ones if possible.
repo.ui.note(_(b"committing manifest\n")) repo.ui.note(_(b"committing manifest\n"))
# we're using narrowmatch here since it's already applied at # we're using narrowmatch here since it's already applied at
# other stages (such as dirstate.walk), so we're already # other stages (such as dirstate.walk), so we're already
# ignoring things outside of narrowspec in most cases. The # ignoring things outside of narrowspec in most cases. The
# one case where we might have files outside the narrowspec # one case where we might have files outside the narrowspec
# at this point is merges, and we already error out in the # at this point is merges, and we already error out in the
# case where the merge has files outside of the narrowspec, # case where the merge has files outside of the narrowspec,
# so this is safe. # so this is safe.
if has_some_narrow_action:
match = None
else:
match = repo.narrowmatch()
mn = mctx.write( mn = mctx.write(
tr, tr,
linkrev, linkrev,
p1.manifestnode(), p1.manifestnode(),
p2.manifestnode(), p2.manifestnode(),
added, added,
drop, drop,
match=repo.narrowmatch(), match=match,
) )
else: else:
repo.ui.debug( repo.ui.debug(
b'reusing manifest from p1 (listed files ' b'actually unchanged)\n' b'reusing manifest from p1 (listed files ' b'actually unchanged)\n'
) )
mn = p1.manifestnode() mn = p1.manifestnode()
return mn return mn

mercurial/merge.py

# over a copy. # over a copy.
for f, action in mresult.filemap(): for f, action in mresult.filemap():
if narrowmatch(f): if narrowmatch(f):
pass pass
elif not branchmerge: elif not branchmerge:
mresult.removefile(f) # just updating, ignore changes outside clone mresult.removefile(f) # just updating, ignore changes outside clone
elif action[0].no_op: elif action[0].no_op:
mresult.removefile(f) # merge does not affect file mresult.removefile(f) # merge does not affect file
elif action[0].narrow_safe: # TODO: handle these cases elif action[0].narrow_safe:
if (
not f.endswith(b'/')
and action[0].changes == mergestatemod.CHANGE_MODIFIED
):
mresult.removefile(f) # merge won't affect on-disk files
mresult.addcommitinfo(
f, b'outside-narrow-merge-action', action[0].changes
)
else: # TODO: handle the tree case
msg = _( msg = _(
b'merge affects file \'%s\' outside narrow, ' b'merge affects file \'%s\' outside narrow, '
b'which is not yet supported' b'which is not yet supported'
) )
hint = _(b'merging in the other direction may work') hint = _(b'merging in the other direction may work')
raise error.Abort(msg % f, hint=hint) raise error.Abort(msg % f, hint=hint)
else: else:
msg = _(b'conflict in file \'%s\' is outside narrow clone') msg = _(b'conflict in file \'%s\' is outside narrow clone')
raise error.StateError(msg % f) raise error.StateError(msg % f)
class mergeresult(object): class mergeresult(object):
"""An object representing result of merging manifests. """An object representing result of merging manifests.

tests/test-narrow-merge.t

$ echo modified3 > inside/f1 $ echo modified3 > inside/f1
$ hg resolve -m $ hg resolve -m
(no more unresolved files) (no more unresolved files)
$ hg commit -m 'merge inside/f1' $ hg commit -m 'merge inside/f1'
TODO: Can merge non-conflicting changes outside narrow spec TODO: Can merge non-conflicting changes outside narrow spec
$ hg update -q 'desc("modify inside/f1")' $ hg update -q 'desc("modify inside/f1")'
#if flat
$ hg merge 'desc("modify outside/f1")'
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit)
status should be clean
$ hg status
? inside/f1.orig
file out of the spec should still not be in the dirstate at all
$ hg debugdirstate | grep outside/f1
[1]
Commit that merge
$ hg ci -m 'merge from outside to inside'
status should be clean
$ hg status
? inside/f1.orig
file out of the spec should not be in the mergestate anymore
$ hg debugmergestate | grep outside/f1
[1]
file out of the spec should still not be in the dirstate at all
$ hg debugdirstate | grep outside/f1
[1]
The filenode used should come from p2
$ hg manifest --debug --rev . | grep outside/f1
83cd11431a3b2aff8a3995e5f27bcf33cdb5be98 644 outside/f1
$ hg manifest --debug --rev 'p1(.)' | grep outside/f1
c6b956c48be2cd4fa94be16002aba311143806fa 644 outside/f1
$ hg manifest --debug --rev 'p2(.)' | grep outside/f1
83cd11431a3b2aff8a3995e5f27bcf33cdb5be98 644 outside/f1
remove the commit to get in the previous situation again
$ hg debugstrip -r .
0 files updated, 0 files merged, 0 files removed, 0 files unresolved
saved backup bundle to $TESTTMP/narrow/.hg/strip-backup/48eb25338b19-a1bb8350-backup.hg
#else
$ hg merge 'desc("modify outside/f1")' $ hg merge 'desc("modify outside/f1")'
abort: merge affects file 'outside/f1' outside narrow, which is not yet supported (flat !)
abort: merge affects file 'outside/' outside narrow, which is not yet supported (tree !) abort: merge affects file 'outside/' outside narrow, which is not yet supported (tree !)
(merging in the other direction may work) (merging in the other direction may work)
[255] [255]
#endif
$ hg update -q 'desc("modify outside/f1")' $ hg update -q 'desc("modify outside/f1")'
$ hg merge 'desc("modify inside/f1")' $ hg merge 'desc("modify inside/f1")'
1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
(branch merge, don't forget to commit) (branch merge, don't forget to commit)
$ hg ci -m 'merge from inside to outside' $ hg ci -m 'merge from inside to outside'
Refuses merge of conflicting outside changes Refuses merge of conflicting outside changes
$ hg update -q 'desc("modify outside/f1")' $ hg update -q 'desc("modify outside/f1")'
$ hg merge 'desc("conflicting outside/f1")' $ hg merge 'desc("conflicting outside/f1")'
abort: conflict in file 'outside/f1' is outside narrow clone (flat !) abort: conflict in file 'outside/f1' is outside narrow clone (flat !)
abort: conflict in file 'outside/' is outside narrow clone (tree !) abort: conflict in file 'outside/' is outside narrow clone (tree !)
[20] [20]