This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8218

phabricator: pass old `fctx` to `addoldbinary()` instead of inferring it
ClosedPublic

Authored by mharbison72 on Mar 4 2020, 11:56 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG98f7b9cf7bfc: phabricator: pass old `fctx` to `addoldbinary()` instead of inferring it
Summary

Currently, removed binaries aren't marked as binaries on the left side, which
sends the raw file view to a bad URL in the web interface. (See D8009) In order
to handle marking the file as binary in the removed case, both contexts need to
be provided by the caller, since there is no current fctx in the removed case.
Having an explicit old fctx will also be useful to support a --no-stack option
that rolls up the commit stack into a single review.

The bug isn't fixed with this change- there's a missing call to it in
addremoved() as well. But instead of spamming the list with a bunch of test
diffs, all of the missing binary issues will be fixed at once later.

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 4 2020, 11:56 AM
mharbison72 added a commit: rHG98f7b9cf7bfc: phabricator: pass old `fctx` to `addoldbinary()` instead of inferring it.Mar 4 2020, 2:07 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG98f7b9cf7bfc: phabricator: pass old `fctx` to `addoldbinary()` instead of inferring it (authored by mharbison72). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/phabricator.py (12 lines)

Diff 20480

hgext/phabricator.py

ui.debug(b'server already has %s\n' % bytes(fctx)) ui.debug(b'server already has %s\n' % bytes(fctx))
if not fphid: if not fphid:
raise error.Abort(b'Upload of %s failed.' % bytes(fctx)) raise error.Abort(b'Upload of %s failed.' % bytes(fctx))
return fphid return fphid
def addoldbinary(pchange, fctx): def addoldbinary(pchange, oldfctx, fctx):
"""add the metadata for the previous version of a binary file to the """add the metadata for the previous version of a binary file to the
phabchange for the new version phabchange for the new version
``oldfctx`` is the previous version of the file; ``fctx`` is the new
version of the file, or None if the file is being removed.
""" """
oldfctx = fctx.p1() if not fctx or fctx.cmp(oldfctx):
if fctx.cmp(oldfctx):
# Files differ, add the old one # Files differ, add the old one
pchange.metadata[b'old:file:size'] = oldfctx.size() pchange.metadata[b'old:file:size'] = oldfctx.size()
mimeguess, _enc = mimetypes.guess_type( mimeguess, _enc = mimetypes.guess_type(
encoding.unifromlocal(oldfctx.path()) encoding.unifromlocal(oldfctx.path())
) )
if mimeguess: if mimeguess:
pchange.metadata[b'old:file:mime-type'] = pycompat.bytestr( pchange.metadata[b'old:file:mime-type'] = pycompat.bytestr(
mimeguess mimeguess
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() or notutf8(fctx): if fctx.isbinary() or notutf8(fctx):
makebinary(pchange, fctx) makebinary(pchange, fctx)
addoldbinary(pchange, fctx) addoldbinary(pchange, fctx.p1(), fctx)
else: else:
maketext(pchange, ctx, fname) maketext(pchange, ctx, fname)
pdiff.addchange(pchange) pdiff.addchange(pchange)
def addadded(pdiff, ctx, added, removed): def addadded(pdiff, 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"""
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() or notutf8(fctx): if fctx.isbinary() or notutf8(fctx):
makebinary(pchange, fctx) makebinary(pchange, fctx)
if renamed: if renamed:
addoldbinary(pchange, fctx) addoldbinary(pchange, fctx.p1(), fctx)
else: else:
maketext(pchange, ctx, fname) maketext(pchange, 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():