This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8974

merge: add `ACTION_KEEP_ABSENT` to represent files we want to keep absent
ClosedPublic

Authored by pulkit on Sep 1 2020, 8:13 AM.

Download Raw Diff

Details

Reviewers

marmoute

Group Reviewers

hg-reviewers

Commits

rHGbb9888d32601: merge: add `ACTION_KEEP_ABSENT` to represent files we want to keep absent

Summary

There are files which were deleted/not present in working copy parent but were
present on other side of merge. On merge, we might decide to keep them deleted.
We want to track such cases more closely, rather all kind of cases which results
from some kind of merging logic.

We do have ACTION_KEEP but having a dedicated action for the absent case is
more cleaner.

Initially I named the action as ACTION_KEEP_DELETED but later realized that
file can be not-present because of other reasons than deletion like rename,
hence decided to use more generic name ACTION_KEEP_ABSENT.

Diff Detail

Repository

rHG Mercurial

Branch

default

Lint

No Linters Available

Unit

No Unit Test Coverage

Event Timeline

pulkit created this revision.Sep 1 2020, 8:13 AM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptSep 1 2020, 8:13 AM

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

pulkit added a child revision: D8940: merge: store ACTION_KEEP_ABSENT when we are keeping the file absent locally.Sep 1 2020, 8:13 AM

pulkit retitled this revision from merge: add `ACTION_KEEP_DELETED` to represent files we want to keep deleted to merge: add `ACTION_KEEP_ABSENT` to represent files we want to keep absent.Sep 2 2020, 7:53 AM

pulkit edited the summary of this revision. (Show Details)

pulkit updated this revision to Diff 22508.

marmoute requested changes to this revision.Sep 11 2020, 4:55 AM

marmoute added a subscriber: marmoute.

marmoute added inline comments.

mercurial/merge.py
566–570	Can you split the introduction on NO_OP_ACTIONS in its own changeset so that the change coming with ACTION_KEEP_ABSENT get clearer?

This revision now requires changes to proceed.Sep 11 2020, 4:55 AM

(I wonder if we don't use "missing" more than "absent" through the code. However quick grepping is not very conclusive and "missing" could be confused with the hg status state)

pulkit added a commit: rHGbb9888d32601: merge: add `ACTION_KEEP_ABSENT` to represent files we want to keep absent.Sep 16 2020, 7:49 AM

This revision was not accepted when it landed; it landed in state Needs Revision.

Closed by commit rHGbb9888d32601: merge: add `ACTION_KEEP_ABSENT` to represent files we want to keep absent (authored by pulkit). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/merge.py (22 lines)
M			mercurial/mergestate.py (8 lines)
M			mercurial/sparse.py (2 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	22497		Sep 1 2020, 8:13 AM	★	★
Diff 2	22508		Sep 2 2020, 7:52 AM	★	★
Diff 3	22650	rHGbb9888d32601bd1adc80663e06f7c400e1ecb356	Sep 1 2020, 7:38 AM	★	★

Commit	Parents	Author	Summary	Date
66cd6e8c90df	fb219a9d8497	Pulkit Goyal		Sep 1 2020, 7:38 AM

Status	Author	Revision
Closed	pulkit	D8967 merge: show number of ancestors in bid merge debug notes
Closed	pulkit	D8966 merge: show list of bids for each file in bid-merge in ui.note()
Closed	pulkit	D8965 merge: update commitinfo from all mergeresults during bid merge
Closed	pulkit	D8942 debugmergestate: show extras for files which are not in mergestate
Closed	pulkit	D8941 merge: add missing ACTION_KEEP when both remote and ancestor are not present
Closed	pulkit	D8940 merge: store ACTION_KEEP_ABSENT when we are keeping the file absent locally
Closed	pulkit	D8974 merge: add `ACTION_KEEP_ABSENT` to represent files we want to keep absent
Closed	pulkit	D8939 tests: add criss cross merging tests whose behavior need to be fixed

Diff 22508

mercurial/merge.py

	def _filternarrowactions(narrowmatch, branchmerge, mresult):			def _filternarrowactions(narrowmatch, branchmerge, mresult):
	"""			"""
	Filters out actions that can ignored because the repo is narrowed.			Filters out actions that can ignored because the repo is narrowed.

	Raise an exception if the merge cannot be completed because the repo is			Raise an exception if the merge cannot be completed because the repo is
	narrowed.			narrowed.
	"""			"""
	# TODO: handle with nonconflicttypes			# TODO: handle with nonconflicttypes
	nooptypes = {mergestatemod.ACTION_KEEP}
	nonconflicttypes = {			nonconflicttypes = {
	mergestatemod.ACTION_ADD,			mergestatemod.ACTION_ADD,
	mergestatemod.ACTION_ADD_MODIFIED,			mergestatemod.ACTION_ADD_MODIFIED,
	mergestatemod.ACTION_CREATED,			mergestatemod.ACTION_CREATED,
	mergestatemod.ACTION_CREATED_MERGE,			mergestatemod.ACTION_CREATED_MERGE,
	mergestatemod.ACTION_FORGET,			mergestatemod.ACTION_FORGET,
	mergestatemod.ACTION_GET,			mergestatemod.ACTION_GET,
	mergestatemod.ACTION_REMOVE,			mergestatemod.ACTION_REMOVE,
	mergestatemod.ACTION_EXEC,			mergestatemod.ACTION_EXEC,
	}			}
	# We mutate the items in the dict during iteration, so iterate			# We mutate the items in the dict during iteration, so iterate
	# 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] in nooptypes:			elif action[0] in mergeresult.NO_OP_ACTIONS:
	mresult.removefile(f) # merge does not affect file			mresult.removefile(f) # merge does not affect file
	elif action[0] in nonconflicttypes:			elif action[0] in nonconflicttypes:
	raise error.Abort(			raise error.Abort(
	_(			_(
	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'
	)			)
	% f,			% f,
	hint=_(b'merging in the other direction may work'),			hint=_(b'merging in the other direction may work'),
	)			)
	else:			else:
	raise error.Abort(			raise error.Abort(
	_(b'conflict in file \'%s\' is outside narrow clone') % f			_(b'conflict in file \'%s\' is outside narrow clone') % f
	)			)


	class mergeresult(object):			class mergeresult(object):
	''''An object representing result of merging manifests.			''''An object representing result of merging manifests.

	It has information about what actions need to be performed on dirstate			It has information about what actions need to be performed on dirstate
	mapping of divergent renames and other such cases. '''			mapping of divergent renames and other such cases. '''

				NO_OP_ACTIONS = (
				mergestatemod.ACTION_KEEP,
				mergestatemod.ACTION_KEEP_ABSENT,
				)

				marmouteUnsubmitted Not Done Can you split the introduction on NO_OP_ACTIONS in its own changeset so that the change coming with ACTION_KEEP_ABSENT get clearer? marmoute: Can you split the introduction on NO_OP_ACTIONS in its own changeset so that the change coming…
	def __init__(self):			def __init__(self):
	"""			"""
	filemapping: dict of filename as keys and action related info as values			filemapping: dict of filename as keys and action related info as values
	diverge: mapping of source name -> list of dest name for			diverge: mapping of source name -> list of dest name for
	divergent renames			divergent renames
	renamedelete: mapping of source name -> list of destinations for files			renamedelete: mapping of source name -> list of destinations for files
	deleted on one side and renamed on other.			deleted on one side and renamed on other.
	commitinfo: dict containing data which should be used on commit			commitinfo: dict containing data which should be used on commit
	def hasconflicts(self):			def hasconflicts(self):
	""" tells whether this merge resulted in some actions which can			""" tells whether this merge resulted in some actions which can
	result in conflicts or not """			result in conflicts or not """
	for a in self._actionmapping.keys():			for a in self._actionmapping.keys():
	if (			if (
	a			a
	not in (			not in (
	mergestatemod.ACTION_GET,			mergestatemod.ACTION_GET,
	mergestatemod.ACTION_KEEP,
	mergestatemod.ACTION_EXEC,			mergestatemod.ACTION_EXEC,
	mergestatemod.ACTION_REMOVE,			mergestatemod.ACTION_REMOVE,
	mergestatemod.ACTION_PATH_CONFLICT_RESOLVE,			mergestatemod.ACTION_PATH_CONFLICT_RESOLVE,
	)			)
	and self._actionmapping[a]			and self._actionmapping[a]
				and a not in self.NO_OP_ACTIONS
	):			):
	return True			return True

	return False			return False


	def manifestmerge(			def manifestmerge(
	repo,			repo,
	repo.ui.note(_(b" %s: consensus for %s\n") % (f, m))			repo.ui.note(_(b" %s: consensus for %s\n") % (f, m))
	mresult.addfile(f, *l[0])			mresult.addfile(f, *l[0])
	continue			continue
	# If keep is an option, just do it.			# If keep is an option, just do it.
	if mergestatemod.ACTION_KEEP in bids:			if mergestatemod.ACTION_KEEP in bids:
	repo.ui.note(_(b" %s: picking 'keep' action\n") % f)			repo.ui.note(_(b" %s: picking 'keep' action\n") % f)
	mresult.addfile(f, *bids[mergestatemod.ACTION_KEEP][0])			mresult.addfile(f, *bids[mergestatemod.ACTION_KEEP][0])
	continue			continue
				# If keep absent is an option, just do that
				if mergestatemod.ACTION_KEEP_ABSENT in bids:
				repo.ui.note(_(b" %s: picking 'keep absent' action\n") % f)
				mresult.addfile(f, *bids[mergestatemod.ACTION_KEEP_ABSENT][0])
				continue
	# If there are gets and they all agree [how could they not?], do it.			# If there are gets and they all agree [how could they not?], do it.
	if mergestatemod.ACTION_GET in bids:			if mergestatemod.ACTION_GET in bids:
	ga0 = bids[mergestatemod.ACTION_GET][0]			ga0 = bids[mergestatemod.ACTION_GET][0]
	if all(a == ga0 for a in bids[mergestatemod.ACTION_GET][1:]):			if all(a == ga0 for a in bids[mergestatemod.ACTION_GET][1:]):
	repo.ui.note(_(b" %s: picking 'get' action\n") % f)			repo.ui.note(_(b" %s: picking 'get' action\n") % f)
	mresult.addfile(f, *ga0)			mresult.addfile(f, *ga0)
	continue			continue
	# TODO: Consider other simple actions such as mode changes			# TODO: Consider other simple actions such as mode changes

	# remove renamed files after safely stored			# remove renamed files after safely stored
	for f in moves:			for f in moves:
	if wctx[f].lexists():			if wctx[f].lexists():
	repo.ui.debug(b"removing %s\n" % f)			repo.ui.debug(b"removing %s\n" % f)
	wctx[f].audit()			wctx[f].audit()
	wctx[f].remove()			wctx[f].remove()

	numupdates = mresult.len() - mresult.len((mergestatemod.ACTION_KEEP,))			numupdates = mresult.len() - mresult.len(mergeresult.NO_OP_ACTIONS)
	progress = repo.ui.makeprogress(			progress = repo.ui.makeprogress(
	_(b'updating'), unit=_(b'files'), total=numupdates			_(b'updating'), unit=_(b'files'), total=numupdates
	)			)

	if b'.hgsubstate' in mresult._actionmapping[mergestatemod.ACTION_REMOVE]:			if b'.hgsubstate' in mresult._actionmapping[mergestatemod.ACTION_REMOVE]:
	subrepoutil.submerge(repo, wctx, mctx, wctx, overwrite, labels)			subrepoutil.submerge(repo, wctx, mctx, wctx, overwrite, labels)

	# record path conflicts			# record path conflicts
	progress.increment(item=f)			progress.increment(item=f)

	# keep (noop, just log it)			# keep (noop, just log it)
	for f, args, msg in mresult.getactions(			for f, args, msg in mresult.getactions(
	(mergestatemod.ACTION_KEEP,), sort=True			(mergestatemod.ACTION_KEEP,), sort=True
	):			):
	repo.ui.debug(b" %s: %s -> k\n" % (f, msg))			repo.ui.debug(b" %s: %s -> k\n" % (f, msg))
	# no progress			# no progress
				for f, args, msg in mresult.getactions(
				(mergestatemod.ACTION_KEEP_ABSENT,), sort=True
				):
				repo.ui.debug(b" %s: %s -> ka\n" % (f, msg))
				# no progress

	# directory rename, move local			# directory rename, move local
	for f, args, msg in mresult.getactions(			for f, args, msg in mresult.getactions(
	(mergestatemod.ACTION_DIR_RENAME_MOVE_LOCAL,), sort=True			(mergestatemod.ACTION_DIR_RENAME_MOVE_LOCAL,), sort=True
	):			):
	repo.ui.debug(b" %s: %s -> dm\n" % (f, msg))			repo.ui.debug(b" %s: %s -> dm\n" % (f, msg))
	progress.increment(item=f)			progress.increment(item=f)
	f0, flags = args			f0, flags = args

mercurial/mergestate.py

	ACTION_ADD_MODIFIED = b'am'			ACTION_ADD_MODIFIED = b'am'
	ACTION_CREATED = b'c'			ACTION_CREATED = b'c'
	ACTION_DELETED_CHANGED = b'dc'			ACTION_DELETED_CHANGED = b'dc'
	ACTION_CHANGED_DELETED = b'cd'			ACTION_CHANGED_DELETED = b'cd'
	ACTION_MERGE = b'm'			ACTION_MERGE = b'm'
	ACTION_LOCAL_DIR_RENAME_GET = b'dg'			ACTION_LOCAL_DIR_RENAME_GET = b'dg'
	ACTION_DIR_RENAME_MOVE_LOCAL = b'dm'			ACTION_DIR_RENAME_MOVE_LOCAL = b'dm'
	ACTION_KEEP = b'k'			ACTION_KEEP = b'k'
				# the file was absent on local side before merge and we should
				# keep it absent (absent means file not present, it can be a result
				# of file deletion, rename etc.)
				ACTION_KEEP_ABSENT = b'ka'
	ACTION_EXEC = b'e'			ACTION_EXEC = b'e'
	ACTION_CREATED_MERGE = b'cm'			ACTION_CREATED_MERGE = b'cm'


	class mergestate(object):			class mergestate(object):
	'''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
	# exec change			# exec change
	for f, args, msg in actions.get(ACTION_EXEC, []):			for f, args, msg in actions.get(ACTION_EXEC, []):
	repo.dirstate.normallookup(f)			repo.dirstate.normallookup(f)

	# keep			# keep
	for f, args, msg in actions.get(ACTION_KEEP, []):			for f, args, msg in actions.get(ACTION_KEEP, []):
	pass			pass

				# keep deleted
				for f, args, msg in actions.get(ACTION_KEEP_ABSENT, []):
				pass

	# get			# get
	for f, args, msg in actions.get(ACTION_GET, []):			for f, args, msg in actions.get(ACTION_GET, []):
	if branchmerge:			if branchmerge:
	repo.dirstate.otherparent(f)			repo.dirstate.otherparent(f)
	else:			else:
	parentfiledata = getfiledata[f] if getfiledata else None			parentfiledata = getfiledata[f] if getfiledata else None
	repo.dirstate.normal(f, parentfiledata=parentfiledata)			repo.dirstate.normal(f, parentfiledata=parentfiledata)

mercurial/sparse.py

	type, args, msg = action			type, args, msg = action
	files.add(file)			files.add(file)
	if sparsematch(file):			if sparsematch(file):
	prunedactions[file] = action			prunedactions[file] = action
	elif type == mergestatemod.ACTION_MERGE:			elif type == mergestatemod.ACTION_MERGE:
	temporaryfiles.append(file)			temporaryfiles.append(file)
	prunedactions[file] = action			prunedactions[file] = action
	elif branchmerge:			elif branchmerge:
	if type != mergestatemod.ACTION_KEEP:			if type not in mergemod.mergeresult.NO_OP_ACTIONS:
	temporaryfiles.append(file)			temporaryfiles.append(file)
	prunedactions[file] = action			prunedactions[file] = action
	elif type == mergestatemod.ACTION_FORGET:			elif type == mergestatemod.ACTION_FORGET:
	prunedactions[file] = action			prunedactions[file] = action
	elif file in wctx:			elif file in wctx:
	prunedactions[file] = (mergestatemod.ACTION_REMOVE, args, msg)			prunedactions[file] = (mergestatemod.ACTION_REMOVE, args, msg)

	# in case or rename on one side, it is possible that f1 might not			# in case or rename on one side, it is possible that f1 might not