Skip to content

patch-shebangs.sh: fix for macos sandbox#414448

Merged
Mic92 merged 1 commit intoNixOS:stagingfrom
DavHau:patchshebangs
Jun 27, 2025
Merged

patch-shebangs.sh: fix for macos sandbox#414448
Mic92 merged 1 commit intoNixOS:stagingfrom
DavHau:patchshebangs

Conversation

@DavHau
Copy link
Member

@DavHau DavHau commented Jun 6, 2025 •
edited
Loading

fixes #343576

To test this PR, execute:

nix-build -A tests.hooks.default-stdenv-hooks.patch-shebangs

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Nixpkgs 25.11 Release Notes (or backporting 24.11 and 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 24.11 and 25.05 NixOS Release notes)
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Jun 6, 2025
@nix-owners nix-owners bot requested a review from Ericson2314 June 6, 2025 07:42
@github-actions github-actions bot added 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Jun 6, 2025
drupol
drupol approved these changes Jun 7, 2025
View reviewed changes
Copy link
Contributor

@drupol drupol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diff LGTM

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jun 9, 2025
@DavHau
Copy link
Member Author

DavHau commented Jun 27, 2025

I ran the tests on aarch64-darwin and they succeeded. I think this is ready to merge

@Mic92 Mic92 merged commit 6039e03 into NixOS:staging Jun 27, 2025
20 of 21 checks passed
@trofi
Copy link
Contributor

trofi commented Jun 28, 2025

Bisect says c1cc6ff patch-shebangs.sh: fix for macos sandbox breaks various packages on x86_64-linux in staging. busybox example:

$ nix build nixpkgs/staging#busybox -L
...
busybox> patching script interpreter paths in /nix/store/wxsnmxdj6892ykhjmqs78nksfzamca6m-busybox-1.36.1/default.script
busybox> /nix/store/wxsnmxdj6892ykhjmqs78nksfzamca6m-busybox-1.36.1/default.script: interpreter directive changed from "#!/bin/sh" to "/nix/store/wxsnmxdj6892ykhjmqs78nksfzamca6m-busybox-1.36.1/bin/sh"
busybox> /nix/store/vhwhh4qfsf9ifh28gdf4925c9hphf2rv-patch-shebangs.sh: line 80: /nix/store/wxsnmxdj6892ykhjmqs78nksfzamca6m-busybox-1.36.1/default.script: Permission denied

@DavHau
Copy link
Member Author

DavHau commented Jun 30, 2025

Thanks for letting me know. I will try to fix it tomorrow.

@DavHau DavHau mentioned this pull request Jul 1, 2025
Merged
13 tasks
DavHau added a commit to DavHau/nixpkgs that referenced this pull request Jul 1, 2025
@DavHau
patchShebangs: add more tests
f2f55c5 
- Add test to ensure ability to patch read-only-files (see NixOS#414448 (comment))
- Add test to ensure the timestamp is preserved
- Add test to ensure read-only permissions are preserved
@al3xtjames al3xtjames mentioned this pull request Jul 4, 2025
Closed
13 tasks
@K900
Copy link
Contributor

K900 commented Jul 6, 2025

So I'm pretty sure this broke every single Erlang thing, because they are effectively zip files with a shebang, and going through bash strings ends up corrupting them. We should probably just use a temporary file in /tmp or something.

@DavHau
Copy link
Member Author

DavHau commented Jul 7, 2025

So I'm pretty sure this broke every single Erlang thing, because they are effectively zip files with a shebang, and going through bash strings ends up corrupting them. We should probably just use a temporary file in /tmp or something.

Working on a fix now.

@DavHau DavHau mentioned this pull request Jul 7, 2025
Merged
13 tasks
@DavHau
Copy link
Member Author

DavHau commented Jul 7, 2025

@K900 fixed in #423137

K900 pushed a commit that referenced this pull request Jul 7, 2025
@DavHau
patch-shebangs: fix binary data corrupt after patching
a1154d6 
This removes the recently introduced shell based implementation of `sponge` which wasn't capable of managing binary input.

Now, a tmpFile under $TMPDIR is created manually and later deleted

see: #414448 (comment)
@DavHau DavHau mentioned this pull request Jul 11, 2025
Closed
13 tasks
DavHau added a commit to DavHau/nixpkgs that referenced this pull request Jul 11, 2025
@DavHau
patch-shebangs: fix binary data corrupt after patching
12ade05 
This removes the recently introduced shell based implementation of `sponge` which wasn't capable of managing binary input.

Now, a tmpFile under $TMPDIR is created manually and later deleted

see: NixOS#414448 (comment)
@adamcstephens adamcstephens mentioned this pull request Jul 15, 2025
Merged
13 tasks
This was referenced Jul 20, 2025
Closed
Closed
terlar added a commit to terlar/nixpkgs that referenced this pull request Jul 30, 2025
@terlar
patch-shebangs: fix regression in patching of hard links
99d4c86 
This was discovered with issues related to pnpm that hard links on some
systems, see NixOS#426636.

This is a regression introduced by NixOS#414448 and NixOS#423137.

When `sed -i` is used on a file with hard links, then the file is seperated and only the file named in the command is changed.
@terlar terlar mentioned this pull request Jul 30, 2025
Closed
13 tasks
@gepbird gepbird mentioned this pull request Jul 30, 2025
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@drupol drupol drupol approved these changes

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

Assignees

No one assigned

Labels

6.topic: darwin Running or building packages on Darwin 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@DavHau @trofi @K900 @drupol @Mic92 @wegank