You're going to need a native version of this as well: #33956 (comment)

This should cover both. When you run patchShebangs on the working directory you get the build packages path. When you run it on the nix store you get the host packages path.

I guess in general it can be a good heuristic, but you really need to be able to override the decision. For one, anything in a dev output should use native packages. And probably there will be cases that a script is/needs to be patched before copying to $out.

I would really like to see tests for essential utilities like these, especially with all the ongoing changes such as cross-compilation and, though irrelevant here, musl.

I would really like to see tests for essential utilities like these, especially with all the ongoing changes such as cross-compilation and, though irrelevant here, musl.

I guess my opinion would be that the best way to test this stuff is to build more stuff that depends on it. Right now for cross we do have some cross jobs (https://hydra.nixos.org/jobset/nixpkgs/cross-trunk#tabs-jobs) but they are not blocking right now. To test patchShebangs we would need some runtime tests. NixOS has a lot of good ones for that. Maybe we can cross compile a vm image then run them on QEMU?

FWIW these days I do the vast majority of musl-based work using musl-native, it makes it a lot simpler to separate out what's because of using musl vs what's a cross-compilation problem.

I'd love to have many more tests for all of this-- honestly I probably would have introduced quite a bit of testing for these things but I was concerned about it not being the preferred use of the shared builder resources. Especially because the magic of Nix makes it all too easy to generate tests for the cross-product of every option :D :P. This was one of the concerns folks rightfully expressed on the musl RFC/discussion.

A targeted test-case for this shouldn't be hard-- I think what's holding things back there is largely a consensus on the testing proposal (proposals?) that have been given lots of love but held back for various reasons. That is to say, I think a number of people would love to add tests for this and many other things but we need
a) structure! Where are they put, who maintains the testing infra (fighting constant redesign and providing a clear vision/answer for what/how/why)
b) decision/statement on how resources can/should be committed to these sorts of builds/testing.

Just some thoughts :).

I just pushed a commit just doing what I was trying to say, as that might just be clearer. @matthewbauer feel free to do whatever you like with it.

Failure on x86_64-linux (full log)

Attempted: stdenv

cannot build derivation '/nix/store/5gc2nh8rb58amccsk8xizgmzc6izj1qn-gnutar-1.30.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/3md31fcj5bf11l6rwvd0pnm1ph3w537m-lzip-1.20.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/mx92q10csskb3zw6rap8vgrm1yscvm2p-binutils-wrapper-2.30.drv': 5 dependencies couldn't be built
cannot build derivation '/nix/store/9ll0f71gqfvm1x9r54z890cf44a4q5py-diffutils-3.6.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/xszsz3fxprmdzr7dri9awp0hkydwmx3j-ed-1.14.2.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/dj1kjqby968bsf0ipak7mysy95qhgk98-findutils-4.6.0.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/x8nvzhxvm1gn293lrzkkx4ynjzfg9jm8-gcc-wrapper-7.3.0.drv': 6 dependencies couldn't be built
cannot build derivation '/nix/store/bbff509xy08k4ydhmkqnbkvsaninjs19-patch-2.7.6.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/akbw0bmvx7riy68rjda03b4c49cbv8hf-stdenv-linux.drv': 24 dependencies couldn't be built
error: build of '/nix/store/akbw0bmvx7riy68rjda03b4c49cbv8hf-stdenv-linux.drv' failed

Failure on x86_64-darwin (full log)

Attempted: stdenv

cannot build derivation '/nix/store/ni83b6cmsyd6p13nbmq0gxijp3fc9wzs-cctools-port-895.drv': 7 dependencies couldn't be built
cannot build derivation '/nix/store/ns85xn8mx66z3slcbbxq97vihz5hm1zr-hook.drv': 6 dependencies couldn't be built
cannot build derivation '/nix/store/rjycaixk9pcjd2haghj508yr8ca1jkc5-ICU-osx-10.10.5.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/waww3yk46xd2qma34mln4cg5s4a3ysav-cctools-binutils-darwin.drv': 4 dependencies couldn't be built
cannot build derivation '/nix/store/mkgqq0234l3gginf6kxjhj2wxvr7daf0-gnutar-1.30.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/h6pcgn0js45gz9byxzigb46vv63y3c3c-CF-osx-10.10.5.drv': 7 dependencies couldn't be built
cannot build derivation '/nix/store/p9kwnk8gm5jvb5nq7sdp2r8mwb357qm9-cctools-binutils-darwin-wrapper.drv': 7 dependencies couldn't be built
cannot build derivation '/nix/store/x8zliyj886gvni0ki1irzgp0i9jck6l9-clang-wrapper-5.0.2.drv': 11 dependencies couldn't be built
cannot build derivation '/nix/store/sq604mwlxjfn748d0amd345ri3l760y3-stdenv-darwin.drv': 36 dependencies couldn't be built
�[31;1merror:�[0m build of '/nix/store/sq604mwlxjfn748d0amd345ri3l760y3-stdenv-darwin.drv' failed

Failure on aarch64-linux (full log)

Attempted: stdenv

cannot build derivation '/nix/store/50km5pasf32vbwqjdigkf7w854qhg2rh-gnutar-1.30.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/4dh1i3fwhnnyck9wci0pysyqk9dwjd63-lzip-1.20.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/168acg6rmyc2kinhhdvi5hcyiq5gbsga-binutils-wrapper-2.30.drv': 5 dependencies couldn't be built
cannot build derivation '/nix/store/9v26rjhvyn8f24wrlnqkpynnb2vbyl80-diffutils-3.6.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/r3bdaibp74d29xcjl3rpdd5y2dyaa7sh-ed-1.14.2.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/g7q1081k4qhdl5311f02md6j31cn5zyd-findutils-4.6.0.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/n9690vdw3jp9h02swkjk0701wmfdymw2-gcc-wrapper-7.3.0.drv': 6 dependencies couldn't be built
cannot build derivation '/nix/store/q75xr1635yvahm329dkskf0ysbx6zxf9-patch-2.7.6.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/xhi7flcdr7ycj15my0wsw540vi2sxn0q-stdenv-linux.drv': 26 dependencies couldn't be built
error: build of '/nix/store/xhi7flcdr7ycj15my0wsw540vi2sxn0q-stdenv-linux.drv' failed

Failure on x86_64-linux (full log)

Attempted: stdenv

cannot build derivation '/nix/store/2ivzgkba8pcdhgq0qw1iixm3yvilbg93-gnutar-1.30.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/13rr8lnv24gyfrn1cw277gf64szw54km-lzip-1.20.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/wy1r2ha71ynxlh7r1fbbjbilin265fmr-binutils-wrapper-2.30.drv': 5 dependencies couldn't be built
cannot build derivation '/nix/store/z6hxzd0h8db49vgzn8r2kvz2wkyg0xr1-diffutils-3.6.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/zbmrx7974n6xkspk15gzb9mmcalfrjmd-ed-1.14.2.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/jz216mk2ls0j9dhgdbq7dic7kgazgzk7-findutils-4.6.0.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/l563zk4ww2jn644ajbzclsa496a1xijn-gcc-wrapper-7.3.0.drv': 6 dependencies couldn't be built
cannot build derivation '/nix/store/fx9912xclm1bg8lnh2bnhgg7a2flawsk-patch-2.7.6.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/j2nqs7djmbwy1svhkz88kwlxbjp19454-stdenv-linux.drv': 24 dependencies couldn't be built
error: build of '/nix/store/j2nqs7djmbwy1svhkz88kwlxbjp19454-stdenv-linux.drv' failed

Failure on aarch64-linux (full log)

Attempted: stdenv

cannot build derivation '/nix/store/cgjhj405hn4vqjdkkpq0vl5p741dxrlb-gnutar-1.30.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/3ivkas9asm1h8fjhng2jnjyafcm599ia-lzip-1.20.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/png6cqp088g3z1ygknjpwjwrllkwkw45-binutils-wrapper-2.30.drv': 5 dependencies couldn't be built
cannot build derivation '/nix/store/kvcv7rmh6cn37c74zd7q9006hmkmsj3y-diffutils-3.6.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/2b1ns74wdjs3n4b7gfqsgkmxcd6dd39m-ed-1.14.2.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/2sb8vaxp46v1sncl1mgdww07nvvsqb0v-findutils-4.6.0.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/fv4d5pzf4f1cdq6ilmf9595j9jmngq1l-gcc-wrapper-7.3.0.drv': 6 dependencies couldn't be built
cannot build derivation '/nix/store/f855vncmb644mnqp3899g5947vbh53j3-patch-2.7.6.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/s3z7wb7hp19wlha3wxdxhhdlchfxcglv-stdenv-linux.drv': 26 dependencies couldn't be built
error: build of '/nix/store/s3z7wb7hp19wlha3wxdxhhdlchfxcglv-stdenv-linux.drv' failed

Failure on x86_64-darwin (full log)

Attempted: stdenv

cannot build derivation '/nix/store/09xpml9bai3vx1h6izmr3cgm7fba6bii-cctools-port-895.drv': 7 dependencies couldn't be built
cannot build derivation '/nix/store/agq2f7idijd0bj6airc6119kbnxh4sih-hook.drv': 6 dependencies couldn't be built
cannot build derivation '/nix/store/9ki7pypmm2f4z1md0388v8s4x0nh8idc-ICU-osx-10.10.5.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/dyncpkc0nv3kxrin6dv86rb5kxgh0b2b-cctools-binutils-darwin.drv': 4 dependencies couldn't be built
cannot build derivation '/nix/store/5mic8iywwcpdnfxvvrlbyc188h5n606w-gnutar-1.30.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/dgjipz8r74xbwbfcnmlla6r1ldjs3qgy-CF-osx-10.10.5.drv': 7 dependencies couldn't be built
cannot build derivation '/nix/store/gnywqwh3pg0f7dgf03qvv8wcs8vfbcm6-cctools-binutils-darwin-wrapper.drv': 7 dependencies couldn't be built
cannot build derivation '/nix/store/v1maka74jnqn5l5lqqcyg9hai4q3vc4y-clang-wrapper-5.0.2.drv': 11 dependencies couldn't be built
cannot build derivation '/nix/store/kvcw2wp4dlmfmhzfgvm9sjhvw97dazg7-stdenv-darwin.drv': 36 dependencies couldn't be built
�[31;1merror:�[0m build of '/nix/store/kvcw2wp4dlmfmhzfgvm9sjhvw97dazg7-stdenv-darwin.drv' failed

Should go to staging when ready.

This has broken stuff for me (in painfully subtle and indirect ways, of course), it seems. Any of you able to come to #nix-darwin on IRC?

When I say for me, it's broken at least one important patchShebangs invocation independently of my changes, I think. Surprised nobody's noticed!

Yeah I can hop on

Basically, on current staging, try this:

$ head -n1 $(nix-build -A icu.dev)/bin/icu-config
#!/bin/sh

This is true regardless of platform, so on Linux it'll be impure but not too bad, on macOS it's impure and breaks a bunch of other shit because /bin/sh is a turd.

This is because HOST_PATH doesn't seem to get the shell on it, so patchShebangs doesn't see sh when it issues command -v sh against it.

Edit: if you want to empathize, the reason I found this was because it indirectly led to a broken Makefile in texlive, and I was 😕and trying to figure out what the hell would break a Makefile consistently. Turns out icu-config uses echo -n which does the wrong thing in /bin/sh on macOS, and was putting newlines into a variable and causing syntax errors.

See #46756 for a test that would've caught this sooner.

....and I just locked myself out of my laptop. I'll read that while I get in boot flashdrive

So the basic issue is with this, without a buildInput on bash, there's no approved for run-time sh with which to patch. We can either add an across-the-board defaultBuildInput, or add it on a case-by-case basis.

N.B. the 18.09 version of this is laxer and happily mixes up build time and run time deps so the normal stdenv.shell suffices.

Yeah, I'm not clear on what the correct solution is, but it breaks a ton of stuff on Darwin. Can we back this out until we figure it out, or does someone have a quick stopgap that'll make things work?

Adding the defaultBuildInput will be easy enough. I can do it once I have the computer running again.

We actually already have a defaultBuildInputs in stdenv/generic, but I think the builder attribute (which is where shell comes in) isn't included.

Sorry about this! This has been in the works for a while so I may have forgotten some of the details. I think this is what we were looking at in #46542 (comment). A better solution like defaultBuildInputs is probably the way to go.

As for right now, it may make sense to just revert this for the time being just to make your testing easier. I intentionally merged this right after the 18.09 branch just because I expected there to be some issues and it can take a while for them to all show up. But I don't want it to interfere with the great work on updating the macOS stdenv!

@Ericson2314 Maybe we should just put c72c865 into master for now? That avoids @copumpkin's issue IIUC. I originally avoided doing it because I wanted to add some "enforcement" for incorrect uses of nativeBuildInputs & buildInputs (#46476) but it may be too soon for that (and adding bash everywhere seems like a bad idea).

The other thing I'd be wary of in this patchShebangs change is that this notion of strictness seems weird: if patchShebangs doesn't patch a shebang that it would otherwise have patched if not for this rule, it should barf loudly. patchShebangs failures are super subtle and often end up working and doing the wrong thing way down the line at runtime. If something says /usr/bin/env x or /bin/y in many cases those will exist and just be the slightly wrong version.

So I'm not sure what this strict deps thing is but I'd rather it fail loudly if something fails a strictness check rather than quietly leaving the shebang alone, which is almost never what we want.

Yeah I think it wouldn't even fix the original issue. It's difficult because in almost all cases you should be listing these things in either buildInputs or nativeBuildInputs. The bash case is weird though and needs to be handled in a special way. We can't easily put the target's bash in the stdenv because it hasn't been cross compiled correctly.

So I'm not sure what this strict deps thing is but I'd rather it fail loudly if something fails a strictness check rather than quietly leaving the shebang alone, which is almost never what we want.

That's not how patch shebangs has worked in the past. Sometimes things wind up in outputs that you don't really want in your closure. For instance Git has a bunch of contrib scripts that use Perl and Python. As a result we get a huge closure even for a supposedly minimal version of Git. This is hard to fix because Git also needs Perl as a native build input.

Can we require a whitelist of unpatched shebangs? @matthewbauer I agree with putting that patch on master if we can find a solution to the initialPath problem you found.

Can we revert this?

Staging has been broken for almost a month and this causes problems on both linux (/bin/sh) and (boostrap-tools) for everything that's built using a bootstrap stdenv. The only workaround I found implies overriding almost every package in the stdenv which is not maintainable.

It's okay to revert if this is causing the issue, but I thought those failures were unrelated. ba5717a was supposed to fix any lingering issues in missing bash.

But anyway, this is definitely not a high priority feature so a revert is definitely okay if it causes any serious issues.

I checked the last eval, but it should include those changes.

Yeah, https://hydra.nixos.org/build/81725278 includes that but references bootstrap-tools.

$ head -n1 /nix/store/7rx33a09xsgx5vbhdwb992v86f1n81ph-xz-5.2.4-bin/bin/xzdiff
#!/nix/store/yvc7kmw98kq547bnqn1afgyxm8mxdwhp-bootstrap-tools/bin/sh

I also tested revering this commit on top of staging earlier today and that fixed the references so I'm pretty sure it's this and not one of the other darwin stdenv changes for CoreFoundation. Hopefully staging looks reasonable again after 9c4b11e.

Ok just for reference this PR was reverted. At some point I'd like to get back to this and get everything working but it's not a high priority.

this is really cool!
would I be able to try this out if I just cherry pick this commit or do I have to add the patchShebangsAuto phase to all my packages?