This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8876

merge: pass mergeresult obj instead of actions in applyupdates() (API)
ClosedPublic

Authored by pulkit on Aug 5 2020, 4:51 AM.

Download Raw Diff

Details

Reviewers

indygreg

Group Reviewers

hg-reviewers

Commits

rHGb9b055f15035: merge: pass mergeresult obj instead of actions in applyupdates() (API)

Summary

This is similar to past 20 patches or so where we are replacing use of a bare
actions dict with a dedicated mergeresult object. The goal is to have a
dedicated powerful object instead of a dict while making the code more easier to
understand.
In past few patches, we have already simplified the code at some places using
the newly introduced object.

This patch does not updates applyupdates() to use the mergeresult object
directly. That will be done in next patch to make things easier to review.

Diff Detail

Repository

rHG Mercurial

Branch

default

Lint

No Linters Available

Unit

No Unit Test Coverage

Event Timeline

pulkit created this revision.Aug 5 2020, 4:51 AM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptAug 5 2020, 4:51 AM

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

pulkit added a child revision: D8877: merge: pass mergeresult obj in merge._prefetchfiles().Aug 5 2020, 4:51 AM

indygreg accepted this revision.Aug 8 2020, 12:12 PM

indygreg added a subscriber: indygreg.

indygreg added inline comments.

mercurial/sparse.py
278	Shouldn't we be using the constant for `r` here? But this appeared in the old code, so not a new issue.
519	More opportunities for constants if you feel so inclined...

This revision is now accepted and ready to land.Aug 8 2020, 12:12 PM

pulkit added a commit: rHGb9b055f15035: merge: pass mergeresult obj instead of actions in applyupdates() (API).Aug 8 2020, 12:55 PM

Closed by commit rHGb9b055f15035: merge: pass mergeresult obj instead of actions in applyupdates() (API) (authored by pulkit). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			hgext/remotefilelog/__init__.py (6 lines)
M			mercurial/merge.py (13 lines)
M			mercurial/narrowspec.py (12 lines)
M			mercurial/sparse.py (41 lines)

Commit	Parents	Author	Summary	Date
03319d6ac183	54bc30e022fb	Pulkit Goyal		Aug 3 2020, 4:42 AM

Status	Author	Revision
Closed	pulkit	D8904 merge: drop commitinfo argument to applyupdates (API)
Closed	pulkit	D8903 merge: remove emptyactions() and use collections.defaultdict(list) instead
Closed	pulkit	D8889 merge: pass mergeresult obj in _forgetremoved() (API)
Closed	pulkit	D8888 mergeresult: introduce filemap() which yields filename based mapping
Closed	pulkit	D8887 mergeresult: add `files()` and use it
Closed	pulkit	D8886 mergeresult: introduce getfile() and use it where required
Closed	pulkit	D8885 merge: use ACTION_* constants instead of values in _filternarrowactions()
Closed	pulkit	D8884 merge: rework iteration over mergeresult object in checkpathconflicts()
Closed	pulkit	D8883 applyupdates: simplfy calculation of number of updated files
Closed	pulkit	D8882 mergeresult: yield from getactions() instead of buidling a list and returning
Closed	pulkit	D8881 sparse: replace merge action values with mergestate.ACTION_* constants
Closed	pulkit	D8880 mergeresult: implement a len() function and use it
Closed	pulkit	D8879 merge: replace use of actions dict with mergeresult object
Closed	pulkit	D8878 mergeresult: add sort argument to getactions() method
Closed	pulkit	D8877 merge: pass mergeresult obj in merge._prefetchfiles()
Closed	pulkit	D8876 merge: pass mergeresult obj instead of actions in applyupdates() (API)
Closed	pulkit	D8875 merge: pass mergeresult obj instead of actions in _checkcollision() (API)

Diff 22257

hgext/remotefilelog/init.py

	if constants.SHALLOWREPO_REQUIREMENT in requirements:			if constants.SHALLOWREPO_REQUIREMENT in requirements:
	s = shallowstore.wrapstore(s)			s = shallowstore.wrapstore(s)

	return s			return s


	# prefetch files before update			# prefetch files before update
	def applyupdates(			def applyupdates(
	orig, repo, actions, wctx, mctx, overwrite, wantfiledata, **opts			orig, repo, mresult, wctx, mctx, overwrite, wantfiledata, **opts
	):			):
	if isenabled(repo):			if isenabled(repo):
	manifest = mctx.manifest()			manifest = mctx.manifest()
	files = []			files = []
	for f, args, msg in actions[mergestatemod.ACTION_GET]:			for f, args, msg in mresult.getactions([mergestatemod.ACTION_GET]):
	files.append((f, hex(manifest[f])))			files.append((f, hex(manifest[f])))
	# batch fetch the needed files from the server			# batch fetch the needed files from the server
	repo.fileservice.prefetch(files)			repo.fileservice.prefetch(files)
	return orig(repo, actions, wctx, mctx, overwrite, wantfiledata, **opts)			return orig(repo, mresult, wctx, mctx, overwrite, wantfiledata, **opts)


	# Prefetch merge checkunknownfiles			# Prefetch merge checkunknownfiles
	def checkunknownfiles(orig, repo, wctx, mctx, force, mresult, args, *kwargs):			def checkunknownfiles(orig, repo, wctx, mctx, force, mresult, args, *kwargs):
	if isenabled(repo):			if isenabled(repo):
	files = []			files = []
	sparsematch = repo.maybesparsematch(mctx.rev())			sparsematch = repo.maybesparsematch(mctx.rev())
	for f, (m, actionargs, msg) in pycompat.iteritems(mresult.actions):			for f, (m, actionargs, msg) in pycompat.iteritems(mresult.actions):

mercurial/merge.py

	mergestatemod.ACTION_PATH_CONFLICT,			mergestatemod.ACTION_PATH_CONFLICT,
	mergestatemod.ACTION_PATH_CONFLICT_RESOLVE,			mergestatemod.ACTION_PATH_CONFLICT_RESOLVE,
	)			)
	}			}


	def applyupdates(			def applyupdates(
	repo,			repo,
	actions,			mresult,
	wctx,			wctx,
	mctx,			mctx,
	overwrite,			overwrite,
	wantfiledata,			wantfiledata,
	labels=None,			labels=None,
	commitinfo=None,			commitinfo=None,
	):			):
	"""apply the merge action list to the working directory			"""apply the merge action list to the working directory

				mresult is a mergeresult object representing result of the merge
	wctx is the working copy context			wctx is the working copy context
	mctx is the context to be merged into the working copy			mctx is the context to be merged into the working copy
	commitinfo is a mapping of information which needs to be stored somewhere			commitinfo is a mapping of information which needs to be stored somewhere
	(probably mergestate) so that it can be used at commit time.			(probably mergestate) so that it can be used at commit time.

	Return a tuple of (counts, filedata), where counts is a tuple			Return a tuple of (counts, filedata), where counts is a tuple
	(updated, merged, removed, unresolved) that describes how many			(updated, merged, removed, unresolved) that describes how many
	files were affected by the update, and filedata is as described in			files were affected by the update, and filedata is as described in
	batchget.			batchget.
	"""			"""

				actions = mresult.actionsdict
	_prefetchfiles(repo, mctx, actions)			_prefetchfiles(repo, mctx, actions)

	updated, merged, removed = 0, 0, 0			updated, merged, removed = 0, 0, 0
	ms = mergestatemod.mergestate.clean(			ms = mergestatemod.mergestate.clean(
	repo, wctx.p1().node(), mctx.node(), labels			repo, wctx.p1().node(), mctx.node(), labels
	)			)

	if commitinfo is None:			if commitinfo is None:
	merged += msmerged			merged += msmerged
	removed += msremoved			removed += msremoved

	extraactions = ms.actions()			extraactions = ms.actions()
	if extraactions:			if extraactions:
	mfiles = {a[0] for a in actions[mergestatemod.ACTION_MERGE]}			mfiles = {a[0] for a in actions[mergestatemod.ACTION_MERGE]}
	for k, acts in pycompat.iteritems(extraactions):			for k, acts in pycompat.iteritems(extraactions):
	actions[k].extend(acts)			actions[k].extend(acts)
				for a in acts:
				mresult.addfile(a[0], k, *a[1:])
	if k == mergestatemod.ACTION_GET and wantfiledata:			if k == mergestatemod.ACTION_GET and wantfiledata:
	# no filedata until mergestate is updated to provide it			# no filedata until mergestate is updated to provide it
	for a in acts:			for a in acts:
	getfiledata[a[0]] = None			getfiledata[a[0]] = None
	# Remove these files from actions[ACTION_MERGE] as well. This is			# Remove these files from actions[ACTION_MERGE] as well. This is
	# important because in recordupdates, files in actions[ACTION_MERGE]			# important because in recordupdates, files in actions[ACTION_MERGE]
	# are processed after files in other actions, and the merge driver			# are processed after files in other actions, and the merge driver
	# might add files to those actions via extraactions above. This can			# might add files to those actions via extraactions above. This can
	#			#
	# We don't need to do the same operation for 'dc' and 'cd' because			# We don't need to do the same operation for 'dc' and 'cd' because
	# those lists aren't consulted again.			# those lists aren't consulted again.
	mfiles.difference_update(a[0] for a in acts)			mfiles.difference_update(a[0] for a in acts)

	actions[mergestatemod.ACTION_MERGE] = [			actions[mergestatemod.ACTION_MERGE] = [
	a for a in actions[mergestatemod.ACTION_MERGE] if a[0] in mfiles			a for a in actions[mergestatemod.ACTION_MERGE] if a[0] in mfiles
	]			]
				for a in mresult.getactions([mergestatemod.ACTION_MERGE]):
				if a[0] not in mfiles:
				mresult.removefile(a[0])

	progress.complete()			progress.complete()
	assert len(getfiledata) == (			assert len(getfiledata) == (
	len(actions[mergestatemod.ACTION_GET]) if wantfiledata else 0			len(actions[mergestatemod.ACTION_GET]) if wantfiledata else 0
	)			)
	return updateresult(updated, merged, removed, unresolved), getfiledata			return updateresult(updated, merged, removed, unresolved), getfiledata



	_advertisefsmonitor(			_advertisefsmonitor(
	repo, len(actions[mergestatemod.ACTION_GET]), p1.node()			repo, len(actions[mergestatemod.ACTION_GET]), p1.node()
	)			)

	wantfiledata = updatedirstate and not branchmerge			wantfiledata = updatedirstate and not branchmerge
	stats, getfiledata = applyupdates(			stats, getfiledata = applyupdates(
	repo,			repo,
	actions,			mresult,
	wc,			wc,
	p2,			p2,
	overwrite,			overwrite,
	wantfiledata,			wantfiledata,
	labels=labels,			labels=labels,
	commitinfo=mresult.commitinfo,			commitinfo=mresult.commitinfo,
	)			)

	if updatedirstate:			if updatedirstate:
	with repo.dirstate.parentchange():			with repo.dirstate.parentchange():
	repo.setparents(fp1, fp2)			repo.setparents(fp1, fp2)
	mergestatemod.recordupdates(			mergestatemod.recordupdates(
	repo, actions, branchmerge, getfiledata			repo, mresult.actionsdict, branchmerge, getfiledata
	)			)
	# update completed, clear state			# update completed, clear state
	util.unlink(repo.vfs.join(b'updatestate'))			util.unlink(repo.vfs.join(b'updatestate'))

	if not branchmerge:			if not branchmerge:
	repo.dirstate.setbranch(p2.branch())			repo.dirstate.setbranch(p2.branch())

	# If we're updating to a location, clean up any stale temporary includes			# If we're updating to a location, clean up any stale temporary includes

mercurial/narrowspec.py

	# These two are extracted for extensions (specifically for Google's CitC file			# These two are extracted for extensions (specifically for Google's CitC file
	# system)			# system)
	def _deletecleanfiles(repo, files):			def _deletecleanfiles(repo, files):
	for f in files:			for f in files:
	repo.wvfs.unlinkpath(f)			repo.wvfs.unlinkpath(f)


	def _writeaddedfiles(repo, pctx, files):			def _writeaddedfiles(repo, pctx, files):
	actions = merge.emptyactions()			mresult = merge.mergeresult()
	addgaction = actions[mergestatemod.ACTION_GET].append
	mf = repo[b'.'].manifest()			mf = repo[b'.'].manifest()
	for f in files:			for f in files:
	if not repo.wvfs.exists(f):			if not repo.wvfs.exists(f):
	addgaction((f, (mf.flags(f), False), b"narrowspec updated"))			mresult.addfile(
				f,
				mergestatemod.ACTION_GET,
				(mf.flags(f), False),
				b"narrowspec updated",
				)
	merge.applyupdates(			merge.applyupdates(
	repo,			repo,
	actions,			mresult,
	wctx=repo[None],			wctx=repo[None],
	mctx=repo[b'.'],			mctx=repo[b'.'],
	overwrite=False,			overwrite=False,
	wantfiledata=False,			wantfiledata=False,
	)			)


	def checkworkingcopynarrowspec(repo):			def checkworkingcopynarrowspec(repo):

mercurial/sparse.py


	s = repo.status()			s = repo.status()
	if s.modified or s.added or s.removed or s.deleted:			if s.modified or s.added or s.removed or s.deleted:
	# Still have pending changes. Don't bother trying to prune.			# Still have pending changes. Don't bother trying to prune.
	return			return

	sparsematch = matcher(repo, includetemp=False)			sparsematch = matcher(repo, includetemp=False)
	dirstate = repo.dirstate			dirstate = repo.dirstate
	actions = []			mresult = mergemod.mergeresult()
	dropped = []			dropped = []
	tempincludes = readtemporaryincludes(repo)			tempincludes = readtemporaryincludes(repo)
	for file in tempincludes:			for file in tempincludes:
	if file in dirstate and not sparsematch(file):			if file in dirstate and not sparsematch(file):
	message = _(b'dropping temporarily included sparse files')			message = _(b'dropping temporarily included sparse files')
	actions.append((file, None, message))			mresult.addfile(file, b'r', None, message)
				indygregUnsubmitted Not Done Shouldn't we be using the constant for `r` here? But this appeared in the old code, so not a new issue. indygreg: Shouldn't we be using the constant for `r` here? But this appeared in the old code, so not a…
	dropped.append(file)			dropped.append(file)

	typeactions = mergemod.emptyactions()
	typeactions[b'r'] = actions
	mergemod.applyupdates(			mergemod.applyupdates(
	repo, typeactions, repo[None], repo[b'.'], False, wantfiledata=False			repo, mresult, repo[None], repo[b'.'], False, wantfiledata=False
	)			)

	# Fix dirstate			# Fix dirstate
	for file in dropped:			for file in dropped:
	dirstate.drop(file)			dirstate.drop(file)

	repo.vfs.unlink(b'tempsparse')			repo.vfs.unlink(b'tempsparse')
	repo._sparsesignaturecache.clear()			repo._sparsesignaturecache.clear()
	b'temporarily included %d file(s) in the sparse '			b'temporarily included %d file(s) in the sparse '
	b'checkout for merging\n'			b'checkout for merging\n'
	)			)
	% len(temporaryfiles)			% len(temporaryfiles)
	)			)
	addtemporaryincludes(repo, temporaryfiles)			addtemporaryincludes(repo, temporaryfiles)

	# Add the new files to the working copy so they can be merged, etc			# Add the new files to the working copy so they can be merged, etc
	actions = []			tmresult = mergemod.mergeresult()
	message = b'temporarily adding to sparse checkout'			message = b'temporarily adding to sparse checkout'
	wctxmanifest = repo[None].manifest()			wctxmanifest = repo[None].manifest()
	for file in temporaryfiles:			for file in temporaryfiles:
	if file in wctxmanifest:			if file in wctxmanifest:
	fctx = repo[None][file]			fctx = repo[None][file]
	actions.append((file, (fctx.flags(), False), message))			tmresult.addfile(
				file,
				mergestatemod.ACTION_GET,
				(fctx.flags(), False),
				message,
				)

	typeactions = mergemod.emptyactions()
	typeactions[mergestatemod.ACTION_GET] = actions
	mergemod.applyupdates(			mergemod.applyupdates(
	repo, typeactions, repo[None], repo[b'.'], False, wantfiledata=False			repo, tmresult, repo[None], repo[b'.'], False, wantfiledata=False
	)			)

	dirstate = repo.dirstate			dirstate = repo.dirstate
	for file, flags, msg in actions:			for file, flags, msg in tmresult.getactions([mergestatemod.ACTION_GET]):
	dirstate.normal(file)			dirstate.normal(file)

	profiles = activeconfig(repo)[2]			profiles = activeconfig(repo)[2]
	changedprofiles = profiles & files			changedprofiles = profiles & files
	# If an active profile changed during the update, refresh the checkout.			# If an active profile changed during the update, refresh the checkout.
	# Don't do this during a branch merge, since all incoming changes should			# Don't do this during a branch merge, since all incoming changes should
	# have been handled by the temporary includes above.			# have been handled by the temporary includes above.
	if changedprofiles and not branchmerge:			if changedprofiles and not branchmerge:
	repo.ui.warn(_(b"pending changes to '%s'\n") % f)			repo.ui.warn(_(b"pending changes to '%s'\n") % f)
	abort = not force			abort = not force

	if abort:			if abort:
	raise error.Abort(			raise error.Abort(
	_(b'could not update sparseness due to pending changes')			_(b'could not update sparseness due to pending changes')
	)			)

	# Calculate actions			# Calculate merge result
	dirstate = repo.dirstate			dirstate = repo.dirstate
	ctx = repo[b'.']			ctx = repo[b'.']
	added = []			added = []
	lookup = []			lookup = []
	dropped = []			dropped = []
	mf = ctx.manifest()			mf = ctx.manifest()
	files = set(mf)			files = set(mf)
				mresult = mergemod.mergeresult()
	actions = {}

	for file in files:			for file in files:
	old = origsparsematch(file)			old = origsparsematch(file)
	new = sparsematch(file)			new = sparsematch(file)
	# Add files that are newly included, or that don't exist in			# Add files that are newly included, or that don't exist in
	# the dirstate yet.			# the dirstate yet.
	if (new and not old) or (old and new and not file in dirstate):			if (new and not old) or (old and new and not file in dirstate):
	fl = mf.flags(file)			fl = mf.flags(file)
	if repo.wvfs.exists(file):			if repo.wvfs.exists(file):
	actions[file] = (b'e', (fl,), b'')			mresult.addfile(file, b'e', (fl,), b'')
				indygregUnsubmitted Not Done More opportunities for constants if you feel so inclined... indygreg: More opportunities for constants if you feel so inclined...
	lookup.append(file)			lookup.append(file)
	else:			else:
	actions[file] = (b'g', (fl, False), b'')			mresult.addfile(file, b'g', (fl, False), b'')
	added.append(file)			added.append(file)
	# Drop files that are newly excluded, or that still exist in			# Drop files that are newly excluded, or that still exist in
	# the dirstate.			# the dirstate.
	elif (old and not new) or (not old and not new and file in dirstate):			elif (old and not new) or (not old and not new and file in dirstate):
	dropped.append(file)			dropped.append(file)
	if file not in pending:			if file not in pending:
	actions[file] = (b'r', [], b'')			mresult.addfile(file, b'r', [], b'')

	# Verify there are no pending changes in newly included files			# Verify there are no pending changes in newly included files
	abort = False			abort = False
	for file in lookup:			for file in lookup:
	repo.ui.warn(_(b"pending changes to '%s'\n") % file)			repo.ui.warn(_(b"pending changes to '%s'\n") % file)
	abort = not force			abort = not force
	if abort:			if abort:
	raise error.Abort(			raise error.Abort(
	_(			_(
	b'cannot change sparseness due to pending '			b'cannot change sparseness due to pending '
	b'changes (delete the files or use '			b'changes (delete the files or use '
	b'--force to bring them back dirty)'			b'--force to bring them back dirty)'
	)			)
	)			)

	# Check for files that were only in the dirstate.			# Check for files that were only in the dirstate.
	for file, state in pycompat.iteritems(dirstate):			for file, state in pycompat.iteritems(dirstate):
	if not file in files:			if not file in files:
	old = origsparsematch(file)			old = origsparsematch(file)
	new = sparsematch(file)			new = sparsematch(file)
	if old and not new:			if old and not new:
	dropped.append(file)			dropped.append(file)

	# Apply changes to disk
	typeactions = mergemod.emptyactions()
	for f, (m, args, msg) in pycompat.iteritems(actions):
	typeactions[m].append((f, args, msg))

	mergemod.applyupdates(			mergemod.applyupdates(
	repo, typeactions, repo[None], repo[b'.'], False, wantfiledata=False			repo, mresult, repo[None], repo[b'.'], False, wantfiledata=False
	)			)

	# Fix dirstate			# Fix dirstate
	for file in added:			for file in added:
	dirstate.normal(file)			dirstate.normal(file)

	for file in dropped:			for file in dropped:
	dirstate.drop(file)			dirstate.drop(file)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	22257		Aug 5 2020, 4:51 AM	★	★
Diff 2	22334	rHGb9b055f150359b7970d434c43fc556c3b4cc72d0	Aug 3 2020, 4:42 AM	★	★