PRs ready for review _

bmillwood · November 29, 2025, 4:29pm

This one’s tricky because it’s in a low-level build hook, but I think the actual bug / fix is pretty straightforward actually

github.com/NixOS/nixpkgs

fix patchShebangs in the presence of readonly scripts

staging ← bmillwood:bmillwood/fix-patch-shebangs-chmod-w

opened 06:35PM - 16 Nov 25 UTC

bmillwood

+32 -14

- Fixes #462298 - Replaces `chmod -w` with `chmod u-w` (and likewise `+w`) in `…patch-shebangs.sh`, so that the only permission we touch is the one that we tested (and the one that we need). Restoring the mode just wasn't working correctly before, because `-w` isn't an inverse of `+w` (except in special cases). As documented in the issue, this is necessary to fix working on scripts with mode 575 (though who uses those?) - Added a test case that catches this. - Fixes `restoreReadOnly` stale value by resetting it to an empty string on every loop iteration. This was a much more serious problem, leading the hook to break on scripts with mode 777 as long as you'd already seen a readonly script earlier this invocation. - Testing this requires a single invocation of `patchShebangs` that visits a readonly file before a read-write one. In general the order of `patchShebangs` is the order of the embedded `find` operation, which is nondeterministic, but I think `find` processes its command line arguments in order, so you could do something like `patchShebangs test-555 test-777` and I think that would do it. This would feel very specific to this one bug, though, and unlikely to be useful in future. Testing this comprehensively is made more tricky by the fact that modifying this script forces a rebuild of everything. I worked around this by copying the fixed script to another location and replacing the line in the test that sources the real script with one that sources my fixed script (and cherry-picking all my test changes). The test passes in this setup (and fails on the original script). ## Things done - Built on platform: - [x] x86_64-linux - [ ] aarch64-linux - [ ] x86_64-darwin - [ ] aarch64-darwin - Tested, as applicable: - [ ] [NixOS tests] in [nixos/tests]. - [ ] [Package tests] at `passthru.tests`. - [x] Tests in [lib/tests] or [pkgs/test] for functions and "core" functionality. - [ ] Ran `nixpkgs-review` on this PR. See [nixpkgs-review usage]. - [ ] Tested basic functionality of all binary files, usually in `./result/bin/`. - Nixpkgs Release Notes - [ ] Package update: when the change is major or breaking. - NixOS Release Notes - [ ] Module addition: when adding a new NixOS module. - [ ] Module update: when the change is significant. - [x] Fits [CONTRIBUTING.md], [pkgs/README.md], [maintainers/README.md] and other READMEs. [NixOS tests]: https://nixos.org/manual/nixos/unstable/index.html#sec-nixos-tests [Package tests]: https://github.com/NixOS/nixpkgs/blob/master/pkgs/README.md#package-tests [nixpkgs-review usage]: https://github.com/Mic92/nixpkgs-review#usage [CONTRIBUTING.md]: https://github.com/NixOS/nixpkgs/blob/master/CONTRIBUTING.md [lib/tests]: https://github.com/NixOS/nixpkgs/blob/master/lib/tests [maintainers/README.md]: https://github.com/NixOS/nixpkgs/blob/master/maintainers/README.md [nixos/tests]: https://github.com/NixOS/nixpkgs/blob/master/nixos/tests [pkgs/README.md]: https://github.com/NixOS/nixpkgs/blob/master/pkgs/README.md [pkgs/test]: https://github.com/NixOS/nixpkgs/blob/master/pkgs/test --- Add a :+1: [reaction] to [pull requests you find important]. [reaction]: https://github.blog/2016-03-10-add-reactions-to-pull-requests-issues-and-comments/ [pull requests you find important]: https://github.com/NixOS/nixpkgs/pulls?q=is%3Aopen+sort%3Areactions-%2B1-desc