This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7054

phabricator: treat non-utf-8 text files as binary as phabricator requires
ClosedPublic

Authored by Kwan on Oct 10 2019, 5:53 PM.

Download Raw Diff

Details

Reviewers

None

Group Reviewers

hg-reviewers

Commits

rHG06a33a501aa2: phabricator: treat non-utf-8 text files as binary as phabricator requires
rHGa852a1ef8c8a: phabricator: treat non-utf-8 text files as binary as phabricator requires

Summary

Phabricator can't cope with text files that are not UTF-8, so requires them to
be submitted as binary files instead. This has the unfortunate effect of
making them practically unreviewable in Phabricator since it will only display
the separate versions of the file in other views, not a diff. phabreading
such submissions are similar, since it will just output the binary patch, but
hg import copes with it fine and hg diff afterwards will show the actual
changes. It is still a marked improvement over trying to submit them as text,
which just leads to corruption (Phabricator will either output ? or HTML
entities for non-UTF-8 characters, depending on context).

Running decode on the whole file like this seems slightly unfortunate, but I'm
not aware of a better way.
Needs to be done to p1() version as well to detect conversions to UTF-8.

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

Kwan created this revision.Oct 10 2019, 5:53 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptOct 10 2019, 5:53 PM

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

Kwan added a child revision: D7055: phabricator: update hgmatcher to cope with the new data format.Oct 10 2019, 5:53 PM

Fix some test-check-code issues, and one test-check-pyflakes unused local.

I'd missed out the attrs key conversion needed on py3.

Kwan added a commit: rHGa852a1ef8c8a: phabricator: treat non-utf-8 text files as binary as phabricator requires.Oct 16 2019, 4:51 PM

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

Closed by commit rHGa852a1ef8c8a: phabricator: treat non-utf-8 text files as binary as phabricator requires (authored by Kwan). · Explain Why

This revision was automatically updated to reflect the committed changes.

Kwan added a commit: rHG06a33a501aa2: phabricator: treat non-utf-8 text files as binary as phabricator requires.Oct 16 2019, 10:38 PM

Revision Contents
Changeset List

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

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	17060		Oct 10 2019, 5:52 PM	★	★
Diff 2	17084		Oct 11 2019, 1:55 PM	★	★
Diff 3	17104		Oct 12 2019, 9:45 AM	★	★
Diff 4	17260	rHGa852a1ef8c8a8e54b11224ae3a6202522be2e775	Oct 10 2019, 5:05 PM	★	★

Status	Author	Revision
Abandoned	Kwan	D7056 phabricator: include bookmark in diff metadata
Closed	Kwan	D7055 phabricator: update hgmatcher to cope with the new data format
Closed	Kwan	D7054 phabricator: treat non-utf-8 text files as binary as phabricator requires
Closed	Kwan	D7053 phabricator: switch to the creatediff endpoint
Closed	Kwan	D7052 phabricator: change conduit data format to match arcanist
Closed	Kwan	D7051 phabricator: remove tests and all recordings
Closed	Kwan	D7066 phabricator: convert phabhunk and phabchange keys to bytes when finalising
Closed	Kwan	D7050 phabricator: add addadded function
Closed	Kwan	D7049 phabricator: add addremoved and addmodified functions
Closed	Kwan	D7048 phabricator: add makebinary and addoldbinary functions
Closed	Kwan	D7047 phabricator: add the uploadfile function
Closed	Kwan	D7046 phabricator: add the uploadchunks function
Closed	Kwan	D7045 phabricator: add the maketext function
Closed	Kwan	D7044 phabricator: add the phabdiff data structure
Closed	Kwan	D7043 phabricator: add the phabchange data structure
Closed	Kwan	D7042 phabricator: add the phabhunk data structure
Closed	Kwan	D7041 phabricator: add the DiffChangeType and DiffFileType constants

Diff 17260

hgext/phabricator.py

	if mimeguess.startswith(b'image/'):			if mimeguess.startswith(b'image/'):
	pchange.fileType = DiffFileType.IMAGE			pchange.fileType = DiffFileType.IMAGE


	# Copied from mercurial/patch.py			# Copied from mercurial/patch.py
	gitmode = {b'l': b'120000', b'x': b'100755', b'': b'100644'}			gitmode = {b'l': b'120000', b'x': b'100755', b'': b'100644'}


				def notutf8(fctx):
				"""detect non-UTF-8 text files since Phabricator requires them to be marked
				as binary
				"""
				try:
				fctx.data().decode('utf-8')
				if fctx.parents():
				fctx.p1().data().decode('utf-8')
				return False
				except UnicodeDecodeError:
				fctx.repo().ui.write(
				_(b'file %s detected as non-UTF-8, marked as binary\n')
				% fctx.path()
				)
				return True


	def addremoved(pdiff, ctx, removed):			def addremoved(pdiff, 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
	)			)
	pchange.addoldmode(gitmode[ctx.p1()[fname].flags()])			pchange.addoldmode(gitmode[ctx.p1()[fname].flags()])
	fctx = ctx.p1()[fname]			fctx = ctx.p1()[fname]
	if not fctx.isbinary():			if not (fctx.isbinary() or notutf8(fctx)):
	maketext(pchange, ctx, fname)			maketext(pchange, ctx, fname)

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


	def addmodified(pdiff, ctx, modified):			def addmodified(pdiff, 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]
	pchange = phabchange(currentPath=fname, oldPath=fname)			pchange = phabchange(currentPath=fname, oldPath=fname)
	filemode = gitmode[ctx[fname].flags()]			filemode = gitmode[ctx[fname].flags()]
	originalmode = gitmode[ctx.p1()[fname].flags()]			originalmode = gitmode[ctx.p1()[fname].flags()]
	if filemode != originalmode:			if filemode != originalmode:
	pchange.addoldmode(originalmode)			pchange.addoldmode(originalmode)
	pchange.addnewmode(filemode)			pchange.addnewmode(filemode)

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

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



	if filemode != originalmode:			if filemode != originalmode:
	pchange.addoldmode(originalmode)			pchange.addoldmode(originalmode)
	pchange.addnewmode(filemode)			pchange.addnewmode(filemode)
	else: # Brand-new file			else: # Brand-new file
	pchange.addnewmode(gitmode[fctx.flags()])			pchange.addnewmode(gitmode[fctx.flags()])
	pchange.type = DiffChangeType.ADD			pchange.type = DiffChangeType.ADD

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

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