Page MenuHomePhabricator

Arc Patch does not patch images
Open, Needs TriagePublic

Description

Arc patch does not download images in the diff. The patch command completes successfully but image size is 0.

andrew@andrew:~$ arc patch D2495
A webroot/dsn.png
property 'svn:executable' set on 'webroot/dsn.png'
property 'svn:mime-type' set on 'webroot/dsn.png'
OKAY Successfully applied patch to the working copy.

andrew@andrew:~$ ls -l webroot/dsn.png
-rwxr-xr-x 1 andrew users 0 Jun 2 11:38 webroot/dsn.png

Event Timeline

neoone raised the priority of this task from to Needs Triage.
neoone updated the task description. (Show Details)
neoone added a project: Arcanist.
neoone added a subscriber: neoone.

I believe this is related to storage issues (i.e., the image file is actually missing on the server), but we should at least warn the user about it.

Avivey. The image can be seen in differential, that means there is no problem in server storage.

Also having this issue on the Subversion repository.

The native svn patching doesn't support images at all. There is probably some custom code in Phabricator to make this happen, but it fails obviously.

I've checked, that Differential revision is storing images as regular file and that's why it works in Differential itself. But https://secure.phabricator.com/diffusion/ARC/browse/master/src/workflow/ArcanistPatchWorkflow.php;083127c4cc5aaca09cb2cd829a6a58bc7dc74738$605-627 code that applies changes in revision just ignores images instead of attempting to download corresponding file from Phabricator server.

The https://github.com/aik099/arcanist/commit/8fbffd1adf4bb9a6efb43fa046592c5a595ebe12 change (made myself) solved @neoone described problem. You can apply mentioned commit to your Arcanist install and all should work.

@aik099 is that a change you think might be accepted upstream in phabricator? Just thinking your change might be better suited as a revision here instead of a change/pull request on github. It would also allow for individual sites who need the fix to pull it more easily by doing an arc patch of it.

@cspeckmim, an issue is several month old, but still has Priority set to Needs Triage. If and when issue will be confirmed by Phabricator team (that usually results in Priority field value change), then I would be glad to send differential revision for it.