Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fetchpatch silently strips binary sections #204320

Open
mweinelt opened this issue Dec 3, 2022 · 3 comments
Open

fetchpatch silently strips binary sections #204320

mweinelt opened this issue Dec 3, 2022 · 3 comments
Labels
0.kind: bug 5. scope: tracked Issue (or PR) is linked back to a `scope: tracking` issue 6.topic: fetch

Comments

@mweinelt
Copy link
Member

mweinelt commented Dec 3, 2022

Issue description

When applying a patch through fetchpatch that includes a Git binary hunk, that hunk is silently dropped. This can cause all kinds of funky behaviour, e.g. due to missing test artficacts (e.g. media, der certificates) and is not easily noticable.

The issue stems from the fact that patchutils does not deal with that kind of hunk and ideally we don't want to introduce a dependency on gitMinimal in fetchpatch.

Steps to reproduce

randombit/botan@c2faa88

❯ curl  https://github.com/randombit/botan/commit/c2faa88b0281e5017be72e1c85d0c41f686e1928.patch 2>/dev/null | lsdiff
b/src/tests/data/x509/ocsp/bdr-int.pem
b/src/tests/data/x509/ocsp/bdr-root.pem
b/src/tests/data/x509/ocsp/bdr.pem
a/src/tests/test_x509_path.cpp

What is missing after applying patchutils is the hunk for src/tests/data/x509/ocsp/bdr-int-ocsp-resp.der.

Proposed remediation

Simple and stupid:

grep -q "GIT binary patch" "$out"  && { echo "Patch includes Git binary hunk, which can't be supported in fetchpatch"; exit 1 }

possibly with an escape hatch, to ignore missing binaries.

Technical details

  • 23.05-pre-3056-gfae71ddc73a

cc @vcunat @risicle

@Artturin
Copy link
Member

Artturin commented Dec 3, 2022

How about fetchpatch2 which was just merged and uses a newer patchutils

@mweinelt
Copy link
Member Author

mweinelt commented Dec 3, 2022

No, patchutils 0.4.2 shows the same behaviour. I'm still thinking about whether it is a reasonable ask to support this upstream.

@amarshall
Copy link
Member

amarshall commented Oct 4, 2023

Upstream issue: twaugh/patchutils#57

However, note that GNU patch does not seem to support Git binary diff format.

@samueldr samueldr added the 5. scope: tracked Issue (or PR) is linked back to a `scope: tracking` issue label Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: bug 5. scope: tracked Issue (or PR) is linked back to a `scope: tracking` issue 6.topic: fetch
Projects
None yet
Development

No branches or pull requests

4 participants