Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Campaign patches with binaries don't apply #10815

Closed
eseliger opened this issue May 19, 2020 · 2 comments · Fixed by #11020
Closed

Campaign patches with binaries don't apply #10815

eseliger opened this issue May 19, 2020 · 2 comments · Fixed by #11020
Assignees
Labels
batch-changes Issues related to Batch Changes bug An error, flaw or fault that produces an incorrect or unexpected result, or behavior. estimate/0.5d planned/3.17
Milestone

Comments

@eseliger
Copy link
Member

  • Sourcegraph version: master
  • Platform information: macOS 10.15.4, Chrome 81

Steps to reproduce:

  1. Create an action that touches a binary file (ie. compress images in docs/ folder)
  2. Create campaign

Expected behavior:

It

  • works
  • or warned me that it can't work
  • or skips binaries so it doesn't fail

Actual behavior:

It fails with errors like

creating commit from patch for repository "X": gitserver: applying patch: exit status 1

$ git apply --cached -p0 --unidiff-zero
error: cannot apply binary patch to 'binary.ttf' without full index line
error: binary.ttf: patch does not apply

Corresponding diff (see that binary content is not inlined):

diff --git binary.ttf binary.ttf
index 563872c..3d12770 100644
Binary files binary.ttf and binary.ttf differ
@eseliger eseliger added bug An error, flaw or fault that produces an incorrect or unexpected result, or behavior. campaigns estimate/1.5d planned/3.17 batch-changes Issues related to Batch Changes labels May 19, 2020
@eseliger eseliger added this to the 3.17 milestone May 19, 2020
@eseliger
Copy link
Member Author

eseliger commented May 19, 2020

Findings:

  • adding --binary to git diff in src-cli inlines the file, which should make it applicable.
  • our go-diff parser doesn't understand that and skips that file. It should also account for a GIT binary patch header. In that case, the whole binary content needs to be decoded and set on the diff, so we can use it in the newFileContent function in patch_sets.go, making it correctly return binary: true in the resolver.

Seems to be git encodes the binary file contents using base85: https://github.com/git/git/blob/master/base85.c
Seems like the ascii85 package in Go could decode it (?)

@mrnugget mrnugget removed this from the 3.17 milestone May 26, 2020
@eseliger eseliger self-assigned this May 26, 2020
@eseliger eseliger added this to the 3.17 milestone May 26, 2020
@eseliger
Copy link
Member Author

eseliger commented May 26, 2020

Tasks

For binary patches to work, these need to be done:

Follow-up issue making the integration of binary files even smoother (but the list above fixes the filed issue from this ticket):
https://github.com/sourcegraph/sourcegraph/issues/10998

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
batch-changes Issues related to Batch Changes bug An error, flaw or fault that produces an incorrect or unexpected result, or behavior. estimate/0.5d planned/3.17
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants