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.

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
Kwan updated this revision to Diff 17084.Oct 11 2019, 1:55 PM

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

Kwan updated this revision to Diff 17104.Oct 12 2019, 9:45 AM

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.

Revision Contents
Changeset List

PathPackages
Mhgext/phabricator.py (23 lines)

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)