arc patch can't handle binary file correctly
Open, Needs TriagePublic
Actions

Assigned To

Authored By

	nickz
	Aug 5 2015, 12:27 AM

Description

In a revision, we upgraded a plugin like the following:

But when running arc patch, the following error occurred:
Checking patch hadoop-scala/lib/latest-jar.txt...
Checking patch hadoop-scala/lib/driven-plugin-1.1.1-eap-14-io.jar => hadoop-scala/lib/driven-plugin-1.3-eap-1.jar...
error: the patch applies to 'hadoop-scala/lib/driven-plugin-1.1.1-eap-14-io.jar' (e6fbf95cfa55b44d1e610c4dff35a15d7a65d6a0), which does not match the current contents.
error: hadoop-scala/lib/driven-plugin-1.1.1-eap-14-io.jar: patch does not apply

Is this a bug?

See PHI1999, where another install reports encountering this.

Related Objects
Search...

		Status	Assigned	Task
		Open	None	T12664 Update diff/patch parsing to extract more metadata and parse a wider range of formats
		Open	epriestley	T9069 arc patch can't handle binary file correctly

Event Timeline

nickz created this task.Aug 5 2015, 12:27 AM

nickz assigned this task to epriestley.

nickz raised the priority of this task from to Needs Triage.

nickz updated the task description. (Show Details)

nickz added a project: Arcanist.

nickz added a subscriber: nickz.

joshuaspence added a subscriber: joshuaspence.Aug 5 2015, 3:57 AM

cspeckmim added a subscriber: cspeckmim.Aug 6 2015, 7:09 AM

benjamin.cohen-solal added a subscriber: benjamin.cohen-solal.Oct 7 2015, 8:35 AM

It's a real problem when for example we store .mo files (binary translation files) or images, pdf, ...

Could you at least tell us if you confirm the bug and if you plan to solve it someday?

indeed it is problematic

Is there a workaround? We are evaluating phabricator and just stumbled across this issue. In our case, we got the error when an image file was deleted...

I can't reproduce this. Here's what I did.

I removed a file:

phabricator/ $ git rm webroot/rsrc/image/people/washington.png
rm 'webroot/rsrc/image/people/washington.png'

I made a commit:

phabricator/ $ git commit -am example
[master ea47dfc] example
 1 file changed, 0 insertions(+), 0 deletions(-)
 delete mode 100644 webroot/rsrc/image/people/washington.png

I created a diff from it:

$ arc diff HEAD^ --only
Uploaded binary data for "washington.png".
Upload complete.
 PUSH STAGING  Pushing changes to staging area...
Counting objects: 6, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 476 bytes | 0 bytes/s, done.
Total 6 (delta 5), reused 0 (delta 0)
set --hard To ssh://dweller@secure.phabricator.com/diffusion/STAGING/staging.git
 * [new tag]         ea47dfca52ef6d815047d99dc5217c0a7e791eec -> phabricator/diff/34789
 STAGING PUSHED  Pushed a copy of the changes to tag "phabricator/diff/34789" in the staging area.
Created a new Differential diff:
        Diff URI: https://secure.phabricator.com/differential/diff/34789/

Included changes:
  D (img) webroot/rsrc/image/people/washington.png

I reset my local working copy:

$ git reset --hard origin/master
HEAD is now at 39f8fea PHUIDocumentViewPro tweaks

I ran arc patch to apply the patch:

$ arc patch --diff 34789
Branch name arcpatch already exists; trying a new name.
Branch name arcpatch_1 already exists; trying a new name.
Created and checked out branch arcpatch_2.
Downloading binary data for 'washington.png'...
Checking patch webroot/rsrc/image/people/washington.png...
Applied patch webroot/rsrc/image/people/washington.png cleanly.
 OKAY  Successfully committed patch.

This deleted the file:

$ git show --raw
commit c37031e79566cb5dd1a459f1bda7f524362e0fb9
Author: epriestley <git@epriestley.com>
Date:   Wed Nov 4 05:06:48 2015 -0800

    Example apply.

:100644 000000 1cdf51b... 0000000... D  webroot/rsrc/image/people/washington.png

Do you have any ideas about what I might have done differently from you?

When reporting a bug, it's very important that you provide the best reproduction steps you can. We generally can not help you with problems we can't reproduce, and trying to reproduce issues given only a vague description of what you did is usually not a good use of our very limited resources. See:

https://secure.phabricator.com/book/phabcontrib/article/bug_reports/

A particularly useful way to generate a reproduction case here would be to create a diff on this install against the rGITTEST repository which does not apply cleanly with arc patch.

Thank you for a very speedy response. I suspect the issue is triggered by a very mangled GIT tree with too many merges and several hundred changed files.
We created the diff from windows, but the arc patch failure is consistent on Ubuntu, MingW and DOS, so I do not think that is relevant. I try to patch my D7, which seems to depend on D1. Patching D1 fails. Patching D7 with --skip-dependencies also fails, but on a different file. Both D1 and D7 are monster-diffs which probably should never have been pushed to phabricator. I suspect the issue is caused by the sum of these things...

I will see if I can reproduce on the rGITTEST - if not I will just bypass phabricator and push the diff directly to our repo.

Sorry for the lack of details in my bug report - I will try to improve in the future, I guess I was hoping you had a trivial solution since this looked like a known issue.
Will update this issue if able to reproduce on rGITTEST.

robwiss mentioned this in rGITTEST859399f024d9: added binary file to reproduce T9069.Nov 9 2015, 6:43 PM

I've managed to reproduce the issue on Ubuntu 15.10

I added a file to rGITTEST to allow this to be reproduced, T9069/Karl_Oskar_Lööw.jpg

Steps to reproduce

create a new folder to put everything else into

$ mkdir newroot

move contents of repo into this new folder:

$ git mv florida jane/ lines pasta T9069/ test

commit

$ git commit
[master 039db6c] add a new root
 6 files changed, 0 insertions(+), 0 deletions(-)
 rename "T9069/Karl_Oskar_L\303\266\303\266w.jpg" => "newroot/T9069/Karl_Oskar_L\303\266\303\266w.jpg" (100%)
 rename florida => newroot/florida (100%)
 rename {jane => newroot/jane}/test-T9628.txt (100%)
 rename lines => newroot/lines (100%)
 rename pasta => newroot/pasta (100%)
 rename test => newroot/test (100%)

create a diff from the newest commit

$ arc diff HEAD^ --only
[2015-11-09 18:36:19] ERROR 2: Invalid argument supplied for foreach() at [/home/robwiss/arc/arcanist/src/upload/ArcanistFileUploader.php:107]
arcanist(head=master, ref.master=baf5eb8a8795), phutil(head=master, ref.master=59f5a8d2bb82)
  #0 ArcanistFileUploader::uploadFiles() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:2558]
  #1 ArcanistDiffWorkflow::uploadFilesForChanges(array) called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:1104]
  #2 ArcanistDiffWorkflow::generateChanges() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:495]
  #3 ArcanistDiffWorkflow::run() called at [<arcanist>/scripts/arcanist.php:382]
[2015-11-09 18:36:19] ERROR 2: Invalid argument supplied for foreach() at [/home/robwiss/arc/arcanist/src/upload/ArcanistFileUploader.php:118]
arcanist(head=master, ref.master=baf5eb8a8795), phutil(head=master, ref.master=59f5a8d2bb82)
  #0 ArcanistFileUploader::uploadFiles() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:2558]
  #1 ArcanistDiffWorkflow::uploadFilesForChanges(array) called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:1104]
  #2 ArcanistDiffWorkflow::generateChanges() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:495]
  #3 ArcanistDiffWorkflow::run() called at [<arcanist>/scripts/arcanist.php:382]
[2015-11-09 18:36:19] ERROR 2: Invalid argument supplied for foreach() at [/home/robwiss/arc/arcanist/src/upload/ArcanistFileUploader.php:197]
arcanist(head=master, ref.master=baf5eb8a8795), phutil(head=master, ref.master=59f5a8d2bb82)
  #0 ArcanistFileUploader::uploadFiles() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:2558]
  #1 ArcanistDiffWorkflow::uploadFilesForChanges(array) called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:1104]
  #2 ArcanistDiffWorkflow::generateChanges() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:495]
  #3 ArcanistDiffWorkflow::run() called at [<arcanist>/scripts/arcanist.php:382]
[2015-11-09 18:36:19] ERROR 2: Invalid argument supplied for foreach() at [/home/robwiss/arc/arcanist/src/upload/ArcanistFileUploader.php:210]
arcanist(head=master, ref.master=baf5eb8a8795), phutil(head=master, ref.master=59f5a8d2bb82)
  #0 ArcanistFileUploader::uploadFiles() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:2558]
  #1 ArcanistDiffWorkflow::uploadFilesForChanges(array) called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:1104]
  #2 ArcanistDiffWorkflow::generateChanges() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:495]
  #3 ArcanistDiffWorkflow::run() called at [<arcanist>/scripts/arcanist.php:382]
[2015-11-09 18:36:19] ERROR 2: Invalid argument supplied for foreach() at [/home/robwiss/arc/arcanist/src/workflow/ArcanistDiffWorkflow.php:2561]
arcanist(head=master, ref.master=baf5eb8a8795), phutil(head=master, ref.master=59f5a8d2bb82)
  #0 ArcanistDiffWorkflow::uploadFilesForChanges(array) called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:1104]
  #1 ArcanistDiffWorkflow::generateChanges() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:495]
  #2 ArcanistDiffWorkflow::run() called at [<arcanist>/scripts/arcanist.php:382]
[2015-11-09 18:36:19] ERROR 2: Invalid argument supplied for foreach() at [/home/robwiss/arc/arcanist/src/workflow/ArcanistDiffWorkflow.php:2583]
arcanist(head=master, ref.master=baf5eb8a8795), phutil(head=master, ref.master=59f5a8d2bb82)
  #0 ArcanistDiffWorkflow::uploadFilesForChanges(array) called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:1104]
  #1 ArcanistDiffWorkflow::generateChanges() called at [<arcanist>/src/workflow/ArcanistDiffWorkflow.php:495]
  #2 ArcanistDiffWorkflow::run() called at [<arcanist>/scripts/arcanist.php:382]
Upload complete.
 PUSH STAGING  Pushing changes to staging area...
Counting objects: 193, done.
Delta compression using up to 8 threads.
Compressing objects: 100% (52/52), done.
Writing objects: 100% (193/193), 343.60 KiB | 0 bytes/s, done.
Total 193 (delta 124), reused 185 (delta 122)
To ssh://dweller@secure.phabricator.com/diffusion/STAGING/staging.git
 * [new tag]         039db6c4a6929932a2bbe1c859d3873bb13bceb9 -> phabricator/diff/34919
 STAGING PUSHED  Pushed a copy of the changes to tag "phabricator/diff/34919" in the staging area.
Created a new Differential diff:
        Diff URI: https://secure.phabricator.com/differential/diff/34919/

Included changes:
  V (bin) newroot/T9069/Karl_Oskar_Lööw.jpg
             from: T9069/Karl_Oskar_Lööw.jpg
  V (bin) T9069/Karl_Oskar_Lööw.jpg
             to: newroot/T9069/Karl_Oskar_Lööw.jpg
  V       newroot/florida
             from: florida
  V       florida
             to: newroot/florida
  V       newroot/jane/test-T9628.txt
             from: jane/test-T9628.txt
  V       jane/test-T9628.txt
             to: newroot/jane/test-T9628.txt
  V       newroot/lines
             from: lines
  V       lines
             to: newroot/lines
  V       newroot/pasta
             from: pasta
  V       pasta
             to: newroot/pasta
  V       newroot/test
             from: test
  V       test
             to: newroot/test

reset the branch to the state before the commit

$ git reset --hard 872940d58fe653f1a9040d4e3a942eea93a50a8e
HEAD is now at 872940d added binary file

apply the patch

$ arc patch --diff 34919
Created and checked out branch arcpatch.
Checking patch test => newroot/test...
Checking patch pasta => newroot/pasta...
Checking patch lines => newroot/lines...
Checking patch jane/test-T9628.txt => newroot/jane/test-T9628.txt...
Checking patch florida => newroot/florida...
Checking patch "T9069/Karl_Oskar_L\303\266\303\266w.jpg" => "newroot/T9069/Karl_Oskar_L\303\266\303\266w.jpg"...
error: the patch applies to 'T9069/Karl_Oskar_Lööw.jpg' (57fbf2b3f1973d34edcc80a560b2221b85d8ed02), which does not match the current contents.
error: T9069/Karl_Oskar_Lööw.jpg: patch does not apply
Applied patch test => newroot/test cleanly.
Applied patch pasta => newroot/pasta cleanly.
Applied patch lines => newroot/lines cleanly.
Applied patch jane/test-T9628.txt => newroot/jane/test-T9628.txt cleanly.
Applied patch florida => newroot/florida cleanly.

 Patch Failed! 
Usage Exception: Unable to apply patch!

epriestley moved this task from Backlog to arc patch on the Arcanist board.Dec 10 2015, 3:03 PM

I just removed a binary file (a .jpg file) and the command arc patch failed as following :

$ /home/jenkins/app/arcanist/bin/arc patch --diff 19499 --nocommit --nobranch ********
Checking patch rybon.png...
error: the patch applies to 'rybon.png' (4a23bc330dac12d53c08938e97020ac4a43554e1), which does not match the current contents.
error: rybon.png: patch does not apply

michaeljs1990 added a subscriber: michaeljs1990.Feb 19 2016, 2:57 AM

I have the same problem, I installed the following 's jenkins plugin for phabricator :
https://wiki.jenkins-ci.org/display/JENKINS/Phabricator+Differential+Plugin
But this plugin is unusabled because of this issue. every time I try to run a build on a branch that contains a binary file modification (rhe most often .mo file)
th arc patch command crashes. Are you going to fix this issue soon, please?

Yeah, we still run into the same problem from time to time:
error: the patch applies to 'hadoop-scala/lib/driven-plugin-1.3.6-hosted-ga-11.jar' (49f50831fe1109b164f56c8e823132aa5f5171c9), which does not match the current contents.

It looks like git auto-detected it as a renamed + modified file.
If it's a delete and add, there is no such an issue.

20after4 added a subscriber: 20after4.Apr 6 2016, 4:04 PM

greggrossmeier added a subscriber: greggrossmeier.Apr 7 2016, 9:14 PM

joaovrb added a subscriber: joaovrb.Apr 29 2016, 11:46 AM

jnwng added a subscriber: jnwng.May 25 2016, 6:04 AM

I've re-created 2 sub-cases of "arc patch with binary files":

deleting files with --skip-binaries D15972: When removing large binary files, specifying --skip-binaries, the patch fails
rename + move D16070: When git detects a file as "renamed and moved", the patch fails. If the modification is huge, it's considered "delete and add", which does work.

(I'm using patch in my T182 flow, so both of these bit me IRL).

Herald added a subscriber: faulconbridge. · View Herald TranscriptJun 7 2016, 5:24 PM

avivey moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jun 7 2016, 6:34 PM

I think it's probably correct to fail with --skip-binaries, but it would be nice to preserve that metadata through the process so arc patch can warn you about it, something like:

This change can not be applied completely because it was generated with --skip-binaries, ...

Ideally we'd apply everything but the binaries, I guess, although I'm not sure how often this is useful. Probably fine if the binaries are, like, photography for a blog, but likely not useful at all if the binaries are code-related. And not useful in a "Land Revision" flow.

The (2) case is probably (?) just an actual bug somewhere.

T11091 is also a way to cheat around this in some cases, possibly.

avivey moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.Jul 12 2016, 9:54 PM

avivey moved this task from Restricted Project Column to Restricted Project Column on the Restricted Project board.

epriestley added projects: Prioritized, Differential.Mar 30 2017, 3:13 PM

Herald added a subscriber: eadler. · View Herald TranscriptMar 30 2017, 3:13 PM

A SAAS install is hitting the "move + rename binary" case.

epriestley added a project: Phacility.Mar 30 2017, 3:15 PM

epriestley edited projects, added Customer Impact; removed Prioritized.Apr 12 2017, 3:12 PM

epriestley removed a project: Phacility.

epriestley moved this task from Backlog to Active Impact on the Customer Impact board.Apr 12 2017, 3:29 PM

• liyufu moved this task from Active Impact to Requests on the Customer Impact board.Apr 15 2017, 9:33 AM

epriestley moved this task from Requests to Active Impact on the Customer Impact board.Apr 15 2017, 11:19 AM

epriestley added a parent task: T12664: Update diff/patch parsing to extract more metadata and parse a wider range of formats.May 2 2017, 5:01 PM

epriestley mentioned this in T12664: Update diff/patch parsing to extract more metadata and parse a wider range of formats.May 2 2017, 5:12 PM

• GoogleLegacy added a subscriber: • GoogleLegacy.Feb 19 2018, 4:37 AM

It seems that newer versions of git have become more aggressive at detecting binary changes as moves + renames, because my install is now hitting this on a very frequent basis

kuba-orlik added a subscriber: kuba-orlik.Dec 2 2019, 8:22 PM

Herald added a subscriber: amckinley. · View Herald TranscriptDec 2 2019, 8:22 PM

epriestley updated the task description. (Show Details)Feb 16 2021, 7:11 PM

epriestley removed a project: Customer Impact.

joaovrb removed a subscriber: joaovrb.Feb 17 2021, 4:01 PM

arc patch can't handle binary file correctlyOpen, Needs TriagePublicActions

Description

Related ObjectsSearch...

Event Timeline

Steps to reproduce

arc patch can't handle binary file correctly
Open, Needs TriagePublic
Actions

Related Objects
Search...