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

wrapProgram & friends depend on linux kernel shebang feature #11133

Closed
vizanto opened this issue Nov 18, 2015 · 16 comments
Closed

wrapProgram & friends depend on linux kernel shebang feature #11133

vizanto opened this issue Nov 18, 2015 · 16 comments
Labels
0.kind: enhancement Add something new 1.severity: mass-rebuild This PR causes a large number of packages to rebuild

Comments

@vizanto
Copy link
Contributor

vizanto commented Nov 18, 2015

While trying to package JRuby as a more ruby like package, I discovered that OSX does not support shebangs at the kernel level.

Running jruby (which is a bash script that starts java) works fine on any unix.
Trying to run gem which has jruby in it's shebang:

➭ /nix/store/5cxwjdd1izi7gk0i3csl1ifb2y6m69p1-jruby-9.0.4.0/bin/gem
zsh: exec format error: /nix/store/5cxwjdd1izi7gk0i3csl1ifb2y6m69p1-jruby-9.0.4.0/bin/gem

And apparantly bash has a fallback mode where it interprets the script as a bash script:

building path(s) ‘/nix/store/d64p6aq2hm1f8xz5smd4d7blzvygi2jc-tzinfo-1.2.2.gem’
unpacking sources
/nix/store/5cxwjdd1izi7gk0i3csl1ifb2y6m69p1-jruby-9.0.4.0/bin/gem: line 4: syntax error near unexpected token `('
/nix/store/5cxwjdd1izi7gk0i3csl1ifb2y6m69p1-jruby-9.0.4.0/bin/gem: line 4: `load File.join(File.dirname(__FILE__), 'jgem')'
builder for ‘/nix/store/nw9fwklp9jvp4bilvkhwcn45zgg51v1s-tzinfo-1.2.2.gem.drv’ failed with exit code 2
cannot build derivation ‘/nix/store/x99njf4xb10y21wbchj99667vfvwlyjh-fluentd-0.12.6.drv’: 1 dependencies couldn't be built
error: build of ‘/nix/store/x99njf4xb10y21wbchj99667vfvwlyjh-fluentd-0.12.6.drv’ failed

More details:

Suggested fix

If jruby were a binary, it would probably work fine. Maybe wrapProgram can generate and compile a tiny C program that runs the wrapped script.

@vizanto vizanto changed the title wrapProgram depends on linux kernel shebang feature wrapProgram & friends depend on linux kernel shebang feature Nov 18, 2015
@vizanto vizanto changed the title wrapProgram & friends depend on linux kernel shebang feature wrapProgram & friends depend on linux kernel shebang feature Nov 18, 2015
@lucabrunox
Copy link
Contributor

👍 I can work on this if we agree... @edolstra ? I know you don't like wrappers, but... :)

@vizanto
Copy link
Contributor Author

vizanto commented Nov 18, 2015

Related is patchShebang, because it removes /usr/bin/env which is a binary that makes the original scripts work.

In other words, originally gem starts with

#!/usr/bin/env jruby

After patchShebang

#!/nix/store/5cxwjdd1izi7gk0i3csl1ifb2y6m69p1-jruby-9.0.4.0/bin/jruby

@bjornfor
Copy link
Contributor

Related: #2146. I attempted to make patchShebangs change "/usr/bin/env prog" into "/nix/store/.../env /nix/store/.../prog", but that hit the default kernel shebang size limit. (See discussion in the mentioned issue.)

@vcunat
Copy link
Member

vcunat commented Nov 19, 2015

Yeah, it would solve multiple issues at once. Still, we probably want a simple way of inspecting what the wrapper does – options off the top of my head:

  • create also a text file next to the wrapper containing arguments to makeWrapper,
  • or even make the binary wrapper generic, i.e. make it just interpret that text file without hard-coding anything into the binary (perhaps do hardcode the location of the text file, as argv0 stuff is tricky).

@vcunat vcunat added 0.kind: enhancement Add something new 1.severity: mass-rebuild This PR causes a large number of packages to rebuild labels Nov 19, 2015
@lucabrunox
Copy link
Contributor

@vcunat which is basically exec bash and source that file :)

@vcunat
Copy link
Member

vcunat commented Nov 19, 2015

Yes, that wouldn't really change much from the current state. EDIT: ...except for fixing all those problems, of course.

@cstrahan
Copy link
Contributor

Thinking out loud:

makeWrapper could generate and build a simple C program, injecting the the flags as an array of strings. To recover the flags, we'd have another really small program that dlopens the binary, calls dlsym(handle, "<name-of-magic-symbol>"), and then prints the array of strings (could be formatted with a delimiter - null perhaps - or maybe as bash array syntax).

Now, we could just use getopt(3) on the array and handle the flag processing in the program, or we could instead generate code that does just the right thing. I don't have any strong feelings here.

How does that sound?

@cstrahan
Copy link
Contributor

Whatever we do, I agree with @vcunat that we ought to have a way to inspect the flags used for a given wrapper. I've had cases where I would extend an existing package with an additional fixupPhase, use wrapProgram to set an important env-var, only to clobber the existing .<prog>-wrapped file (because of mv $prog $hidden, where $prog is now the existing wrapper). Having a way to inspect the flags could allow us to make wrapProgram only regenerate <prog>, merging the two flags - which is, IMO, the correct behavior.

@ryanartecona
Copy link
Contributor

I just hit this issue as well.

OS X seems to not mind the longer /nix/store/...-coreutils/bin/env /nix/store/...-prog/bin/prog shebang interpreter line. Could we set patchShebangs up to conditionally include the nix-coreutils-env prefixed shebang only on OS X (or on BSDs) and only for interpreters detected to be shebang scripts themselves, and continue to use the current behavior (no env in shebang) on linux? That seems like it would also fix this problem.

The one downside I can imagine for that approach is any derivation which uses patchShebangs on a script which itself uses a script interpreter (so, if patchShebangs sees a wrapProgram'd executable script, etc.) would now become a platform-dependent derivation if it weren't already. That seems acceptable for my purposes, but not sure if it would be acceptable in general.

(commenting here rather than #2146 as this discussion is more recent, but they seem like duplicate issues to me)

@grahamc
Copy link
Member

grahamc commented Dec 13, 2016

Here is a (crappy) way to work around this issue:

{ stdenv, bundlerEnv}:
{
  osxWrap = { name, script, bundlerEnvResult }: stdenv.mkDerivation {
    inherit name;
    buildCommand = ''
      mkdir bin

      sed \
        -e "s#/bin/ruby #/bin/ruby ${script} #" \
        ${bundlerEnvResult.wrappedRuby}/bin/ruby > ./bin/${name}

      mkdir -p $out/bin
      install -D -m755 ./bin/${name} $out/bin/${name}
    '';
  };

  neato-script = osxWrap {
    name = "my-neato-script";
    script = ./my-neato-script.rb;
    bundlerEnvResult = bundlerEnv rec {
      name = "my-neato-script-env";

      inherit ruby;
      gemfile = ./Gemfile;
      lockfile = ./Gemfile.lock;
      gemset = ./gemset.nix;
    };
  };
}

Here is another crappy workaround, for not ruby scripts:

let 
rewrap = { name, script, interpreter }:
stdenv.mkDerivation {
  inherit name;

  # The normal bundler wrapper uses a script in a shebang. This works
  # fine on Linux, but OSX's kernel doesn't permit this.
  #
  # This wrapper copies the wrappedRuby script, which ends in:
  #
  #   exec /nix/store/.../bin/.foo-wrapped" "${extraFlagsArray[@]}" "$@"
  #
  # and adds the script between `bin/.foo-wrapped"` and `"${extraFlagsArray`:
  #
  #   exec /nix/store/.../bin/.foo-wrapped" /nix/store/...-my-script "${extraFlagsArray[@]}" "$@"

  # and since the wrapped script uses bash directly in the
  # shebang (and not another script) this works on macOS and Linux.
  buildCommand = ''
    sed \
      -e "s#-wrapped\" #-wrapped\" ${script} #" \
      ${interpreter} > ./working

    if ! grep -q "${script}" ./working; then
      echo " ABORTING:"
      echo "    sed appears to not have correctly added the script on"
      echo "    the exec line, which breaks this tool."
      exit 1
    fi

    mkdir -p $out/bin
    install -D -m755 ./working $out/bin/${name}
  '';
};
in
rewrap {
  name = "timeout";
  script = ./timeout.tcl;
  interpreter = "${expect}/bin/expect";
}

@wmertens
Copy link
Contributor

wmertens commented Apr 4, 2019

Just ran into the same issue, a Python script got patchShebang'd to point to the python2.7 wrapper. On Darwin and Solaris, shebang cannot point to a script.

On top of that, using custom shebangs adds changes to otherwise completely unchanged scripts, and it makes the store deduplication work less well.

How about making a __nixExec binary that knows about the nix store, and a shebang like #!/usr/bin/env __nixExec configName [...args] would make it search for /nix/store/package-where-real-scriptfile-is/.nixExec-configName to set the path and so on, and it also reads the first line of the script to get all the arguments so we're not limited to the 128 chars of Linux?

Then

  • shebang points to an executable so it works on Darwin
  • we have unlimited arguments
  • it will properly fail if __nixExec doesn't exist
  • __nixExec can do extra checks at runtime if needed, complaining about missing packages with the nix-store command to run etc
  • script content will stay mostly unchanged between versions and give slightly better deduplication rates

@FRidh
Copy link
Member

FRidh commented Jul 27, 2019

@wmertens see #23018

@stale
Copy link

stale bot commented Jun 2, 2020

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.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 2, 2020
adrian-gierakowski added a commit to adrian-gierakowski/nixpkgs that referenced this issue Jul 24, 2020
On Darwin a script cannot be used as an interpreter in a shebang line, which
causes scripts produced with makeScriptWriter (and its derivatives) to fail at
run time if the used interpreter was wrapped with makeWrapper (as in the case
of python3.withPackages).

This commit fixes the problem by detecting if the interpreter is a script
and prepending its shebang to the final interpreter line.

For example if used interpreter is;
```
/nix/store/ynwv137n2650qy39swcflxbcygk5jwv1-python3-3.8.3-env/bin/python
```

which is a script with following shebang:
```
#! /nix/store/knd85yc7iwli8344ghav3zli8d9gril0-bash-4.4-p23/bin/bash -e
```

then the shebang line in the produced script will be
```
#! /nix/store/knd85yc7iwli8344ghav3zli8d9gril0-bash-4.4-p23/bin/bash -e /nix/store/ynwv137n2650qy39swcflxbcygk5jwv1-python3-3.8.3-env/bin/python
```

This works on Darwin since there does not seem to be a limit to the length
of the shabang line and the shebang lines support multiple arguments to
the interpreters (as opposed to linux where the kernel imposes a strict limit
on shebang lengh and everything following the interpreter is passed to it
as a single string).

fixes; NixOS#93609
related to: NixOS#65351 NixOS#11133 (and probably a bunch of others)

NOTE: scripts produced on platforms other tha Darwin will remain unmodified
by this PR. However it might worth considering extending this fix to BSD systems
in general. I didn't do it since I have no way of testing it on systems other
than MacOS and linux.
Lassulus pushed a commit that referenced this issue Jul 25, 2020
* writers.makeScriptWriter: fix on Darwin\MacOS

On Darwin a script cannot be used as an interpreter in a shebang line, which
causes scripts produced with makeScriptWriter (and its derivatives) to fail at
run time if the used interpreter was wrapped with makeWrapper (as in the case
of python3.withPackages).

This commit fixes the problem by detecting if the interpreter is a script
and prepending its shebang to the final interpreter line.

For example if used interpreter is;
```
/nix/store/ynwv137n2650qy39swcflxbcygk5jwv1-python3-3.8.3-env/bin/python
```

which is a script with following shebang:
```
#! /nix/store/knd85yc7iwli8344ghav3zli8d9gril0-bash-4.4-p23/bin/bash -e
```

then the shebang line in the produced script will be
```
#! /nix/store/knd85yc7iwli8344ghav3zli8d9gril0-bash-4.4-p23/bin/bash -e /nix/store/ynwv137n2650qy39swcflxbcygk5jwv1-python3-3.8.3-env/bin/python
```

This works on Darwin since there does not seem to be a limit to the length
of the shabang line and the shebang lines support multiple arguments to
the interpreters (as opposed to linux where the kernel imposes a strict limit
on shebang lengh and everything following the interpreter is passed to it
as a single string).

fixes; #93609
related to: #65351 #11133 (and probably a bunch of others)

NOTE: scripts produced on platforms other than Darwin will remain unmodified
by this PR. However it might worth considering extending this fix to BSD systems
in general. I didn't do it since I have no way of testing it on systems other
than MacOS and linux.

* writers.makeScriptWriter: fix typo in comment

* writers.makeScriptWriter: fail build if interpreter of interpreter is a script
adrian-gierakowski added a commit to rhinofi/nixpkgs that referenced this issue Jul 26, 2020
* writers.makeScriptWriter: fix on Darwin\MacOS

On Darwin a script cannot be used as an interpreter in a shebang line, which
causes scripts produced with makeScriptWriter (and its derivatives) to fail at
run time if the used interpreter was wrapped with makeWrapper (as in the case
of python3.withPackages).

This commit fixes the problem by detecting if the interpreter is a script
and prepending its shebang to the final interpreter line.

For example if used interpreter is;
```
/nix/store/ynwv137n2650qy39swcflxbcygk5jwv1-python3-3.8.3-env/bin/python
```

which is a script with following shebang:
```
#! /nix/store/knd85yc7iwli8344ghav3zli8d9gril0-bash-4.4-p23/bin/bash -e
```

then the shebang line in the produced script will be
```
#! /nix/store/knd85yc7iwli8344ghav3zli8d9gril0-bash-4.4-p23/bin/bash -e /nix/store/ynwv137n2650qy39swcflxbcygk5jwv1-python3-3.8.3-env/bin/python
```

This works on Darwin since there does not seem to be a limit to the length
of the shabang line and the shebang lines support multiple arguments to
the interpreters (as opposed to linux where the kernel imposes a strict limit
on shebang lengh and everything following the interpreter is passed to it
as a single string).

fixes; NixOS#93609
related to: NixOS#65351 NixOS#11133 (and probably a bunch of others)

NOTE: scripts produced on platforms other than Darwin will remain unmodified
by this PR. However it might worth considering extending this fix to BSD systems
in general. I didn't do it since I have no way of testing it on systems other
than MacOS and linux.

* writers.makeScriptWriter: fix typo in comment

* writers.makeScriptWriter: fail build if interpreter of interpreter is a script
@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/650

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Dec 1, 2021
@Artturin
Copy link
Member

Fixed by #124556 ?

@doronbehar
Copy link
Contributor

Fixed by #124556 ?

Yes, and by #161298 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0.kind: enhancement Add something new 1.severity: mass-rebuild This PR causes a large number of packages to rebuild
Projects
None yet
Development

No branches or pull requests