This is an archive of the discontinued Mercurial Phabricator instance.

phabricator: add basectx arguments to file related `phabsend` utilities
ClosedPublic

Authored by mharbison72 on Mar 20 2020, 5:23 PM.

Download Raw Diff

Details

Reviewers

Alphare
pulkit

Group Reviewers

hg-reviewers

Commits

rHG53d75fdeaaaa: phabricator: add basectx arguments to file related `phabsend` utilities

Summary

This is in support of a future --fold option, that allows rolling up several
commits into a single review with a diff from the start to the end of the range.

There are no functional changes yet- the original ctx is also passed as the
new basectx, which represents the first commit in the review range (similar to
qbase in MQ parlance). Other functions will need the range of commits, but
these deal with status or the diffs, so they only need the end points.

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

mharbison72 created this revision.Mar 20 2020, 5:23 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptMar 20 2020, 5:23 PM

Herald added subscribers: mercurial-devel, Kwan. · View Herald Transcript

mharbison72 added a child revision: D8307: phabricator: account for `basectx != ctx` when calculating renames.Mar 20 2020, 5:23 PM

I am very curious as to how the UI will look on a folded series.

In D8306#124380, @Alphare wrote:

I am very curious as to how the UI will look on a folded series.

Thanks for the reviews!

See D8330. It basically looks the same, other than the Commits tab has multiple revisions.

pulkit accepted this revision.Mar 26 2020, 4:50 AM

This revision is now accepted and ready to land.Mar 26 2020, 4:50 AM

mharbison72 added a commit: rHG53d75fdeaaaa: phabricator: add basectx arguments to file related `phabsend` utilities.Mar 26 2020, 6:50 AM

Closed by commit rHG53d75fdeaaaa: phabricator: add basectx arguments to file related `phabsend` utilities (authored by mharbison72). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			hgext/phabricator.py (52 lines)

Status	Author	Revision
Closed	mharbison72	D8313 phabricator: extract logic to print the status when posting a commit
Closed	mharbison72	D8312 phabricator: extract the logic to amend diff properties to a function
Closed	mharbison72	D8311 phabricator: teach `getoldnodedrevmap()` to handle folded reviews
Closed	mharbison72	D8310 phabricator: teach createdifferentialrevision() to allow a folded commit range
Closed	mharbison72	D8309 phabricator: combine commit messages into the review when folding commits
Closed	mharbison72	D8308 phabricator: record all local commits used to create a Differential revision
Closed	mharbison72	D8307 phabricator: account for `basectx != ctx` when calculating renames
Closed	mharbison72	D8306 phabricator: add basectx arguments to file related `phabsend` utilities
Closed	mharbison72	D8305 phabricator: eliminate a couple of duplicate filectx lookups

Diff 20894

hgext/phabricator.py

	m = _differentialrevisiontagre.match(tag)			m = _differentialrevisiontagre.match(tag)
	if m:			if m:
	result[rev] = int(m.group(1))			result[rev] = int(m.group(1))
	break			break

	return result			return result


	def getdiff(ctx, diffopts):			def getdiff(basectx, ctx, diffopts):
	"""plain-text diff without header (user, commit message, etc)"""			"""plain-text diff without header (user, commit message, etc)"""
	output = util.stringio()			output = util.stringio()
	for chunk, _label in patch.diffui(			for chunk, _label in patch.diffui(
	ctx.repo(), ctx.p1().node(), ctx.node(), None, opts=diffopts			ctx.repo(), basectx.p1().node(), ctx.node(), None, opts=diffopts
	):			):
	output.write(chunk)			output.write(chunk)
	return output.getvalue()			return output.getvalue()


	class DiffChangeType(object):			class DiffChangeType(object):
	ADD = 1			ADD = 1
	CHANGE = 2			CHANGE = 2
	def addchange(self, change):			def addchange(self, change):
	if not isinstance(change, phabchange):			if not isinstance(change, phabchange):
	raise error.Abort(b'phabdiff.addchange only takes phabchanges')			raise error.Abort(b'phabdiff.addchange only takes phabchanges')
	self.changes[change.currentPath] = pycompat.byteskwargs(			self.changes[change.currentPath] = pycompat.byteskwargs(
	attr.asdict(change)			attr.asdict(change)
	)			)


	def maketext(pchange, ctx, fname):			def maketext(pchange, basectx, ctx, fname):
	"""populate the phabchange for a text file"""			"""populate the phabchange for a text file"""
	repo = ctx.repo()			repo = ctx.repo()
	fmatcher = match.exact([fname])			fmatcher = match.exact([fname])
	diffopts = mdiff.diffopts(git=True, context=32767)			diffopts = mdiff.diffopts(git=True, context=32767)
	_pfctx, _fctx, header, fhunks = next(			_pfctx, _fctx, header, fhunks = next(
	patch.diffhunks(repo, ctx.p1(), ctx, fmatcher, opts=diffopts)			patch.diffhunks(repo, basectx.p1(), ctx, fmatcher, opts=diffopts)
	)			)

	for fhunk in fhunks:			for fhunk in fhunks:
	(oldOffset, oldLength, newOffset, newLength), lines = fhunk			(oldOffset, oldLength, newOffset, newLength), lines = fhunk
	corpus = b''.join(lines[1:])			corpus = b''.join(lines[1:])
	shunk = list(header)			shunk = list(header)
	shunk.extend(lines)			shunk.extend(lines)
	_mf, _mt, addLines, delLines, _hb = patch.diffstatsum(			_mf, _mt, addLines, delLines, _hb = patch.diffstatsum(
	except UnicodeDecodeError:			except UnicodeDecodeError:
	fctx.repo().ui.write(			fctx.repo().ui.write(
	_(b'file %s detected as non-UTF-8, marked as binary\n')			_(b'file %s detected as non-UTF-8, marked as binary\n')
	% fctx.path()			% fctx.path()
	)			)
	return True			return True


	def addremoved(pdiff, ctx, removed):			def addremoved(pdiff, basectx, ctx, removed):
	"""add removed files to the phabdiff. Shouldn't include moves"""			"""add removed files to the phabdiff. Shouldn't include moves"""
	for fname in removed:			for fname in removed:
	pchange = phabchange(			pchange = phabchange(
	currentPath=fname, oldPath=fname, type=DiffChangeType.DELETE			currentPath=fname, oldPath=fname, type=DiffChangeType.DELETE
	)			)
	oldfctx = ctx.p1()[fname]			oldfctx = basectx.p1()[fname]
	pchange.addoldmode(gitmode[oldfctx.flags()])			pchange.addoldmode(gitmode[oldfctx.flags()])
	if not (oldfctx.isbinary() or notutf8(oldfctx)):			if not (oldfctx.isbinary() or notutf8(oldfctx)):
	maketext(pchange, ctx, fname)			maketext(pchange, basectx, ctx, fname)

	pdiff.addchange(pchange)			pdiff.addchange(pchange)


	def addmodified(pdiff, ctx, modified):			def addmodified(pdiff, basectx, ctx, modified):
	"""add modified files to the phabdiff"""			"""add modified files to the phabdiff"""
	for fname in modified:			for fname in modified:
	fctx = ctx[fname]			fctx = ctx[fname]
	oldfctx = ctx.p1()[fname]			oldfctx = basectx.p1()[fname]
	pchange = phabchange(currentPath=fname, oldPath=fname)			pchange = phabchange(currentPath=fname, oldPath=fname)
	filemode = gitmode[fctx.flags()]			filemode = gitmode[fctx.flags()]
	originalmode = gitmode[oldfctx.flags()]			originalmode = gitmode[oldfctx.flags()]
	if filemode != originalmode:			if filemode != originalmode:
	pchange.addoldmode(originalmode)			pchange.addoldmode(originalmode)
	pchange.addnewmode(filemode)			pchange.addnewmode(filemode)

	if (			if (
	fctx.isbinary()			fctx.isbinary()
	or notutf8(fctx)			or notutf8(fctx)
	or oldfctx.isbinary()			or oldfctx.isbinary()
	or notutf8(oldfctx)			or notutf8(oldfctx)
	):			):
	makebinary(pchange, fctx)			makebinary(pchange, fctx)
	addoldbinary(pchange, oldfctx, fctx)			addoldbinary(pchange, oldfctx, fctx)
	else:			else:
	maketext(pchange, ctx, fname)			maketext(pchange, basectx, ctx, fname)

	pdiff.addchange(pchange)			pdiff.addchange(pchange)


	def addadded(pdiff, ctx, added, removed):			def addadded(pdiff, basectx, ctx, added, removed):
	"""add file adds to the phabdiff, both new files and copies/moves"""			"""add file adds to the phabdiff, both new files and copies/moves"""
	# Keep track of files that've been recorded as moved/copied, so if there are			# Keep track of files that've been recorded as moved/copied, so if there are
	# additional copies we can mark them (moves get removed from removed)			# additional copies we can mark them (moves get removed from removed)
	copiedchanges = {}			copiedchanges = {}
	movedchanges = {}			movedchanges = {}
	for fname in added:			for fname in added:
	fctx = ctx[fname]			fctx = ctx[fname]
	oldfctx = None			oldfctx = None
	pchange = phabchange(currentPath=fname)			pchange = phabchange(currentPath=fname)

	filemode = gitmode[fctx.flags()]			filemode = gitmode[fctx.flags()]
	renamed = fctx.renamed()			renamed = fctx.renamed()

	if renamed:			if renamed:
	originalfname = renamed[0]			originalfname = renamed[0]
	oldfctx = ctx.p1()[originalfname]			oldfctx = basectx.p1()[originalfname]
	originalmode = gitmode[oldfctx.flags()]			originalmode = gitmode[oldfctx.flags()]
	pchange.oldPath = originalfname			pchange.oldPath = originalfname

	if originalfname in removed:			if originalfname in removed:
	origpchange = phabchange(			origpchange = phabchange(
	currentPath=originalfname,			currentPath=originalfname,
	oldPath=originalfname,			oldPath=originalfname,
	type=DiffChangeType.MOVE_AWAY,			type=DiffChangeType.MOVE_AWAY,
	fctx.isbinary()			fctx.isbinary()
	or notutf8(fctx)			or notutf8(fctx)
	or (oldfctx and (oldfctx.isbinary() or notutf8(oldfctx)))			or (oldfctx and (oldfctx.isbinary() or notutf8(oldfctx)))
	):			):
	makebinary(pchange, fctx)			makebinary(pchange, fctx)
	if renamed:			if renamed:
	addoldbinary(pchange, oldfctx, fctx)			addoldbinary(pchange, oldfctx, fctx)
	else:			else:
	maketext(pchange, ctx, fname)			maketext(pchange, basectx, ctx, fname)

	pdiff.addchange(pchange)			pdiff.addchange(pchange)

	for _path, copiedchange in copiedchanges.items():			for _path, copiedchange in copiedchanges.items():
	pdiff.addchange(copiedchange)			pdiff.addchange(copiedchange)
	for _path, movedchange in movedchanges.items():			for _path, movedchange in movedchanges.items():
	pdiff.addchange(movedchange)			pdiff.addchange(movedchange)


	def creatediff(ctx):			def creatediff(basectx, ctx):
	"""create a Differential Diff"""			"""create a Differential Diff"""
	repo = ctx.repo()			repo = ctx.repo()
	repophid = getrepophid(repo)			repophid = getrepophid(repo)
	# Create a "Differential Diff" via "differential.creatediff" API			# Create a "Differential Diff" via "differential.creatediff" API
	pdiff = phabdiff(			pdiff = phabdiff(
	sourceControlBaseRevision=b'%s' % ctx.p1().hex(),			sourceControlBaseRevision=b'%s' % basectx.p1().hex(),
	branch=b'%s' % ctx.branch(),			branch=b'%s' % ctx.branch(),
	)			)
	modified, added, removed, _d, _u, _i, _c = ctx.p1().status(ctx)			modified, added, removed, _d, _u, _i, _c = basectx.p1().status(ctx)
	# addadded will remove moved files from removed, so addremoved won't get			# addadded will remove moved files from removed, so addremoved won't get
	# them			# them
	addadded(pdiff, ctx, added, removed)			addadded(pdiff, basectx, ctx, added, removed)
	addmodified(pdiff, ctx, modified)			addmodified(pdiff, basectx, ctx, modified)
	addremoved(pdiff, ctx, removed)			addremoved(pdiff, basectx, ctx, removed)
	if repophid:			if repophid:
	pdiff.repositoryPHID = repophid			pdiff.repositoryPHID = repophid
	diff = callconduit(			diff = callconduit(
	repo.ui,			repo.ui,
	b'differential.creatediff',			b'differential.creatediff',
	pycompat.byteskwargs(attr.asdict(pdiff)),			pycompat.byteskwargs(attr.asdict(pdiff)),
	)			)
	if not diff:			if not diff:
	raise error.Abort(_(b'cannot create diff for %s') % ctx)			if basectx != ctx:
				msg = _(b'cannot create diff for %s::%s') % (basectx, ctx)
				else:
				msg = _(b'cannot create diff for %s') % ctx
				raise error.Abort(msg)
	return diff			return diff


	def writediffproperties(ctx, diff):			def writediffproperties(ctx, diff):
	"""write metadata to diff so patches could be applied losslessly"""			"""write metadata to diff so patches could be applied losslessly"""
	# creatediff returns with a diffid but query returns with an id			# creatediff returns with a diffid but query returns with an id
	diffid = diff.get(b'diffid', diff.get(b'id'))			diffid = diff.get(b'diffid', diff.get(b'id'))
	params = {			params = {
	If revid is None, create a new Differential Revision, otherwise update			If revid is None, create a new Differential Revision, otherwise update
	revid. If parentrevphid is not None, set it as a dependency.			revid. If parentrevphid is not None, set it as a dependency.

	If oldnode is not None, check if the patch content (without commit message			If oldnode is not None, check if the patch content (without commit message
	and metadata) has changed before creating another diff.			and metadata) has changed before creating another diff.

	If actions is not None, they will be appended to the transaction.			If actions is not None, they will be appended to the transaction.
	"""			"""
				basectx = ctx
	repo = ctx.repo()			repo = ctx.repo()
	if oldnode:			if oldnode:
	diffopts = mdiff.diffopts(git=True, context=32767)			diffopts = mdiff.diffopts(git=True, context=32767)
	oldctx = repo.unfiltered()[oldnode]			oldctx = repo.unfiltered()[oldnode]
	neednewdiff = getdiff(ctx, diffopts) != getdiff(oldctx, diffopts)			oldbasectx = oldctx
				neednewdiff = getdiff(basectx, ctx, diffopts) != getdiff(
				oldbasectx, oldctx, diffopts
				)
	else:			else:
	neednewdiff = True			neednewdiff = True

	transactions = []			transactions = []
	if neednewdiff:			if neednewdiff:
	diff = creatediff(ctx)			diff = creatediff(basectx, ctx)
	transactions.append({b'type': b'update', b'value': diff[b'phid']})			transactions.append({b'type': b'update', b'value': diff[b'phid']})
	if comment:			if comment:
	transactions.append({b'type': b'comment', b'value': comment})			transactions.append({b'type': b'comment', b'value': comment})
	else:			else:
	# Even if we don't need to upload a new diff because the patch content			# Even if we don't need to upload a new diff because the patch content
	# does not change. We might still need to update its metadata so			# does not change. We might still need to update its metadata so
	# pushers could know the correct node metadata.			# pushers could know the correct node metadata.
	assert olddiff			assert olddiff

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	20846		Mar 20 2020, 5:23 PM	★	★
Diff 2	20894	rHG53d75fdeaaaa84307efbfbe48740805304a0a0ff	Feb 24 2020, 12:06 PM	★	★