This is an archive of the discontinued Mercurial Phabricator instance.

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

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

Download Raw Diff

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

Herald added a reviewer: durin42. · View Herald TranscriptJan 30 2022, 3:15 PM

Herald added a reviewer: hg-reviewers. · View Herald Transcript

Herald added a subscriber: mercurial-patches. · View Herald Transcript

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.

✅ 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

marmoute added a commit: rHGf1eb77dceb36: narrow: allow merging non-conflicting change outside of the narrow spec.Feb 4 2022, 5:31 AM

Closed by commit rHGf1eb77dceb36: narrow: allow merging non-conflicting change outside of the narrow spec (authored by marmoute). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/commit.py (62 lines)
M			mercurial/merge.py (24 lines)
M			tests/test-narrow-merge.t (57 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	32015		Jan 30 2022, 3:15 PM	★	★
Diff 2	32053		Feb 3 2022, 11:55 AM	★	★
Diff 3	32058		Feb 3 2022, 1:22 PM	★	★
Diff 4	32077	rHGf1eb77dceb360ff3590a10ca8f7efb7629193fd5	Feb 3 2022, 12:14 PM	★	★

Status	Author	Revision
Closed	marmoute	D12119 narrow: allow merging non-conflicting change outside of the narrow spec
Closed	marmoute	D12118 merge-actions: add some information about the "changes" the action do
Closed	marmoute	D12117 merge-actions: have an attribute for narrow safetiness
Closed	marmoute	D12116 merge-actions: add an explicite "no_op" attribute
Closed	marmoute	D12115 merge-actions: gather all created action into a set
Closed	marmoute	D12114 merge-actions: make merge action a full featured object
Closed	marmoute	D12113 convert: use the merge action constant
Closed	marmoute	D12112 large-file: use the merge action constant
Closed	marmoute	D12111 merge: stop using merge action for pathconflict option
Closed	marmoute	D12110 merge: break up a not-so-one-liner for readability
Closed	marmoute	D12109 merge: break up a not-so-one-liner for readability
Closed	marmoute	D12108 merge: break up a not-so-one-liner for readability
Closed	marmoute	D12107 merge: break up a not-so-one-liner for readability
Closed	marmoute	D12106 merge: break up two not-so-one-liner for extra readability
Closed	marmoute	D12105 merge: break up a not-so-one-liner for readability
Closed	marmoute	D12104 merge: break up a not-so-one-liner for readability
Closed	marmoute	D12103 merge: break up a not-so-one-liner for readability
Closed	marmoute	D12102 merge: break up a not-so-one-liner for readability
Closed	marmoute	D12101 merge: break up a not-so-one-liner for readability

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]