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

GNU patch < 2.7.6 doesn't apply executable bit correctly #3782

Open
dra27 opened this issue Mar 13, 2019 · 7 comments · Fixed by #5400
Open

GNU patch < 2.7.6 doesn't apply executable bit correctly #3782

dra27 opened this issue Mar 13, 2019 · 7 comments · Fixed by #5400
Assignees

Comments

@dra27
Copy link
Member

dra27 commented Mar 13, 2019

Found while investigating build failures ocaml/opam-repository#12674

Versions of patch prior to 2.7.6 (includes Debian 9) don't set the executable bit on new files when reading a git patch (see http://git.savannah.gnu.org/cgit/patch.git/commit/?id=592e1f9163d6261359aa87c2c99d411c0dacf6f3)

This causes new executable files to lose the +x bit when copied from the Git clone to .opam/repo/...

2.7.6. is the very latest release of GNU patch - I expect the solution to this will be to post-process and check the permissions.

dra27 added a commit to dra27/opam-repository that referenced this issue Mar 13, 2019
@AltGr
Copy link
Member

AltGr commented Mar 25, 2019

One more reason to reimplement our own patch handling :)

@dra27
Copy link
Member Author

dra27 commented Mar 27, 2019

Indeed!

@rjbou
Copy link
Collaborator

rjbou commented Mar 28, 2019

@dra27
Copy link
Member Author

dra27 commented Mar 29, 2019

Cool - it would need extending to parse Git headers... to be honest, it would probably want extending line-by-line against GNU patch 🤦‍♂️

@hannesm
Copy link
Member

hannesm commented Mar 29, 2019

the patch is atm mainly code extracted from conex (where, due to dependencies, and being able to cope with non-unix systems, I don't call out to patch). for that, I don't have the need for "parse git headers" or similar features (which does not mean that that should not be added, just that I won't spend much time on it). the scope is also only to handle unified diffs (atm even ignoring merge conflicts) -- if the diff does not apply cleanly, this should result in an error state (but iirc atm its accepted).

@dra27 I don't understand what you mean by "it would probably want extending line-by-line against GNU patch"

@dra27
Copy link
Member Author

dra27 commented Mar 29, 2019

@hannesm - I mean that patch is so well-specified that it’ll probably need someone to go through its code and reverse-engineer the precise behaviours which want adding

dra27 added a commit to dra27/opam-repository that referenced this issue Aug 1, 2019
Expressly set the execute bit on the script
dra27 added a commit to dra27/opam-repository that referenced this issue Aug 1, 2019
Expressly set the execute bit on the script
dra27 added a commit to dra27/opam-repository that referenced this issue Aug 1, 2019
Expressly set the execute bit on the script
kit-ty-kate added a commit to kit-ty-kate/opam-repository that referenced this issue Feb 20, 2021
@kit-ty-kate
Copy link
Member

I got bitten by this again in ocaml/opam-repository#18064
I'll make the opam-repo-ci linter fail if this case is encountered (ocurrent/opam-repo-ci#74) but this is still quite annoying

kit-ty-kate added a commit to kit-ty-kate/opam-repo-ci that referenced this issue Feb 21, 2021
kit-ty-kate added a commit to ocaml/opam-repository that referenced this issue Feb 21, 2021
Remove any files with permissions different from 644 (mitigates ocaml/opam#3782)
kit-ty-kate added a commit to ocurrent/opam-repo-ci that referenced this issue Feb 21, 2021
mitigate ocaml/opam#3782 + call opam-2.1 in verbose mode
fdopen added a commit to fdopen/opam-repository-mingw that referenced this issue Feb 22, 2021
Remove any files with permissions different from 644 (mitigates ocaml/opam#3782)
@dra27 dra27 self-assigned this Jul 13, 2021
@rjbou rjbou reopened this Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants