NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
17.46k stars 13.66k forks source link

General fixupPhase patchShebangs strategy is in conflict with OS X (and BSD flavor) for wrapped interpreter #2146

Open wavewave opened 10 years ago

wavewave commented 10 years ago

A stdenv derivation replaces shebang line in shell scripts during fixupPhase. Current strategy is that when this meets #!/usr/bin/env (interpreter command), it replaces the whole line with actual command for the interpreter. Therefore, for example, the first line of a python script starting with

#!/usr/bin/env python

is changed to

#!/nix/store/....-python-2.7.6-wrapper/bin/python

This works in linux systems, but does not work on BSD flavored systems like OS X since they do not allow another shell script to play a role as an interpreter. A relevant link is here: http://www.in-ulm.de/~mascheck/various/shebang/#interpreter-script

I suggest to keep /usr/bin/env as /nix/store/...-coreutils-../bin/env when being replaced by patchShebangs during fixupPhase

bjornfor commented 10 years ago

Fix BSD! ;-)

(Being more serious from now on.)

If we keep using ".../bin/env" we must have wrappers that set $PATH correctly (unless we haven't really fixed the interpreter). I don't like that. But I understand breaking BSD/OS X isn't an option either.

How about we change "/usr/bin/env python" to "/nix/store/...coreutils/bin/env /nix/store/...python/bin/python"? (Who said that "env" had to do $PATH lookup at all?)

wavewave commented 10 years ago

Yes. That's what I implied. To be precise, From

#!/usr/bin/env python

To

#!/nix/store/yq1a2ajx4igi7sr8wv8scr36kvp8607f-coreutils-8.21/bin/env /nix/store/v3q7bpp2w9j9w7bvvc4ih877xc8v6mkz-python-2.7.6-wrapper/bin/python

(note that we still use python wrapper as an argument of env)

vizanto commented 10 years ago

Then why use env at all? No env vars are set and no -i option is used..?

bjornfor commented 10 years ago

@vizanto: Because sometimes they do use some flags or features of "env", and that causes breakage when patchShebangs replaces .../env foo with /path/to/foo.

peti commented 10 years ago

@bjornfor, patchShebangs detects additional arguments to env and fails with an error message in that case.

bjornfor commented 10 years ago

@peti: When I said "breakage" I meant that patchShebangs bails out and prints an error.

It seems there is a simple solution here, that stops patchShebangs from bailing out and makes it work on OSX (as per this issue). Simply patch /usr/bin/env python to /nix/store/.../env /nix/store/.../python.

peti commented 10 years ago

The way I read it, the issue has nothing to do with additional arguments to env. It's concerned with the fact that scripts cannot be used as an interpreter of other scripts on BSD.

That limitation shouldn't exist, btw, when those scripts are interpreted by bash.

vcunat commented 10 years ago

What about ${stdenv.shell} -c instead of /usr/bin/env? That way we only add a "lighter" dependency than coreutils.

Of course, the change would only be done on platforms that need it.

bjornfor commented 10 years ago

I don't see why we need to provide something "lighter" than env. If a script uses env then I think it's the easiest solution to just let it continue using env (and just fix it to use absolute paths). Then we have one way of patching shebangs on all systems that work and don't introduce any new problems, like the current situation we have where patchShebangs sometimes have to bail out.

vcunat commented 10 years ago

My meaning was that on Linux (at least) coreutils might be a new runtime dependency in some cases.

bjornfor commented 9 years ago

Some packages seem to have difficulties with the long (~128 byte) shebang that the above fix gives.

$ nix-build -A gnome3.vte
...
make[2]: Entering directory '/tmp/nix-build-vte-0.40.0.drv-0/vte-0.40.0/src'
  GEN      box_drawing.h
/nix/store/w91hsy6mznz3qf7w1cd9vyczq84vw695-coreutils-8.24/bin/env: /nix/store/n6mmyh62k6nz9vrcqsirxhqwrx2bl46g-bash-4.3-p39/b: No such file or directory
Makefile:2473: recipe for target 'box_drawing.h' failed

It snipped the path to bash :-(

bjornfor commented 9 years ago

Haha, no, it's coreutils / env that snips the path:

$ cat test.sh 
#!/nix/store/w91hsy6mznz3qf7w1cd9vyczq84vw695-coreutils-8.24/bin/env /nix/store/n6mmyh62k6nz9vrcqsirxhqwrx2bl46g-bash-4.3-p39/bin/bash 
echo "hello"
$ ./test.sh 
/nix/store/w91hsy6mznz3qf7w1cd9vyczq84vw695-coreutils-8.24/bin/env: /nix/store/n6mmyh62k6nz9vrcqsirxhqwrx2bl46g-bash-4.3-p39/b: No such file or directory
bjornfor commented 9 years ago

Or wait... could it be the linux kernel?

bjornfor commented 9 years ago

No, it's env:

$ strace -eexecve,write -s300 ./test.sh 
execve("./test.sh", ["./test.sh"], [/* 116 vars */]) = 0
execve("/nix/store/n6mmyh62k6nz9vrcqsirxhqwrx2bl46g-bash-4.3-p39/b", ["/nix/store/n6mmyh62k6nz9vrcqsirxhqwrx2bl46g-bash-4.3-p39/b", "./test.sh"], [/* 116 vars */]) = -1 ENOENT (No such file or directory)
write(2, "/nix/store/w91hsy6mznz3qf7w1cd9vyczq84vw695-coreutils-8.24/bin/env: ", 68/nix/store/w91hsy6mznz3qf7w1cd9vyczq84vw695-coreutils-8.24/bin/env: ) = 68
write(2, "/nix/store/n6mmyh62k6nz9vrcqsirxhqwrx2bl46g-bash-4.3-p39/b", 58/nix/store/n6mmyh62k6nz9vrcqsirxhqwrx2bl46g-bash-4.3-p39/b) = 58
write(2, ": No such file or directory", 27: No such file or directory) = 27
write(2, "\n", 1
)                       = 1
+++ exited with 127 +++
vcunat commented 9 years ago

I'd think it's the kernel, but it might be cut on multiple places at once. http://stackoverflow.com/questions/10813538/shebang-line-limit-in-bash-and-linux-kernel

lucabrunox commented 9 years ago

I believe /usr/bin/env bash should be replaced directly by nix-store/bash instead of nix-store/env nix-store/bash. Not a general solution, but solves the problem.

On Sat, Jul 18, 2015 at 12:59 PM, Vladimír Čunát notifications@github.com wrote:

I'd think it's the kernel, but it might be cut on multiple places at once. http://stackoverflow.com/questions/10813538/shebang-line-limit-in-bash-and-linux-kernel

— Reply to this email directly or view it on GitHub https://github.com/NixOS/nixpkgs/issues/2146#issuecomment-122530012.

NixOS Linux http://nixos.org

bjornfor commented 9 years ago

Ah, right.

bjornfor commented 9 years ago

@lethalman: That's what we have today. But according to this issue, it breaks on BSD (shebang interpreters cannot be shell scripts).

bjornfor commented 9 years ago

Well, yes, it'd probably work for the "bash" case, but not for e.g. python (which is a wrapper script).

jagajaga commented 8 years ago

ping

bjornfor commented 8 years ago

One option is to increase the kernel shebang buffer from 128 to 256 (or something) bytes. Then we can use #!.../env .../prog. But on the other hand, it would break Nix packages on non-NixOS, as long as typical Linux kernels on other distros still use 128 bytes buffer...

bjornfor commented 8 years ago

Anyone brave enough to suggest upstream linux increase shebang buffer from 128 to 256 bytes? :-)

gilligan commented 6 years ago

(triage) I assume the problem hasn't changed in any way?

matthewbauer commented 6 years ago

Anyone brave enough to suggest upstream linux increase shebang buffer from 128 to 256 bytes? :-)

Only if we can convince BSD people too!

I have been trying to figure out a solution for this. I have potential two solutions, neither one very ideal:

  1. Rewrite makeWrapper to build a C program with all of the relevant information patched in.
  2. Make patchShebangs prefix everything with #!/bin/sh -c instead of the actual path. May need some testing but I think /bin/sh works everywhere.
dezgeg commented 6 years ago

You can't use more than 1 argument in a #! line, so #!/bin/sh -c won't work.

dezgeg commented 6 years ago

What's the behaviour of the BSD/Linux/Darwin kernels when the shebang line is too long? Does it silently truncate it or refuse to execute it? If it just truncates it we could write a small C program that ignores the actual argument in the shebang passed by the kernel and just reads it from the file itself (by opening argv[0] or whatever mechanism there is) and do:

#! /nix/store/...-shebang-wrapper/bin/shebang /nix/store/....-python-2.7.6-wrapper/bin/python
sorbits commented 5 years ago

I ran into this issue on macOS and now I manually patch some of the stuff to go via /usr/bin/env.

From all of the above comments, I do not understand why rewriting #!/usr/bin/env python to #!/usr/bin/env /nix/store/<hash>/bin/python is not an option.

This should still be less than 128 characters, and /usr/bin/env is available on NixOS (and all other systems), so there is no need to have it call env with a store path.

Since this is only an issue when the interpreter being called via the shebang line is itself a script, the patchShebangs function could keep its current behavior when the interpreter is not a script.

I can submit a PR for this change, unless there is a reason why this solution does not work.

bjornfor commented 5 years ago

AFAIK, #!/usr/bin/env /nix/store/<hash>/bin/python would fail in sandboxed builds (out of the box). Ref. https://github.com/NixOS/nix/issues/1205.

sorbits commented 5 years ago

AFAIK, #!/usr/bin/env /nix/store/<hash>/bin/python would fail in sandboxed builds (out of the box). Ref. NixOS/nix#1205

Does the sandboxed building execute any of the scripts that goes through the patchShebangs function?

Because if not, it would still be able to “build” them, they just wouldn’t run in the sandbox, but unless it’s related to running tests, I don’t see why the builder would run them.

bjornfor commented 5 years ago

Right, the scripts themself would be built successfully in the sandbox (patchShebangs does not execute them). But if those scripts get used inside another build, then there is a problem.

stale[bot] commented 4 years ago

Thank you for your contributions. This has been automatically marked as stale because it has had no activity for 180 days. If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity. Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse. 3. Ask on the #nixos channel on irc.freenode.net.
balsoft commented 2 years ago

Still an issue