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

Add usrBinEnv setup hook for running scripts with #!/usr/bin/env shebang during build without patching them #319420

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bergkvist
Copy link
Member

@bergkvist bergkvist commented Jun 13, 2024

Description of changes

Adds pkgs.usrBinEnv (setup hook) for providing /usr/bin/env in build environments.

Related issues:

Motivation

Consider that you want to build a third party project with build-scripts written in bash, and you see the following line at the top:

#!/usr/bin/env bash
...

Now, if you want to run this script in your build sandbox, you have a problem, since /usr/bin/env doesn't exist in your build sandbox. You will typically find advice telling you that you need to patch the shebang line of the scripts.

There is an alternative approach that doesn't require patching the third party source code though. You can create /usr/bin/env in your build sandbox before running the script as part of your build. This can be done with a setup hook.

{ pkgs ? import <nixpkgs> {}
}:
let
  usrBinEnv = pkgs.makeSetupHook { name = "create-usr-bin-env"; } (pkgs.writeText "create-usr-bin-env.sh" ''
    mkdir -m 0755 -p /usr/bin
    ln -sfn "${pkgs.coreutils}/bin/env" /usr/bin/env
  '');
in
pkgs.runCommand "example" { buildInputs = [ usrBinEnv ]; } ''
  mkdir -p $out && cd $out
  ${./some-script-that-uses-usr-bin-env.sh}
''

This feels like something "common enough" that it maybe should be part of nixpkgs. Then the above example could be written as

{ pkgs ? import <nixpkgs> {}
}:
pkgs.runCommand "example" { buildInputs = [ pkgs.usrBinEnv ]; } ''
  mkdir -p $out && cd $out
  ${./some-script-that-uses-usr-bin-env.sh}
''

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/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (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 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Jun 13, 2024
@eclairevoyant
Copy link
Contributor

Setup hooks would go in nativeBuildInputs not buildInputs, and I'm not sure I'd call this common personally

Copy link
Contributor

@nbraud nbraud left a comment

Choose a reason for hiding this comment

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

mkdir -m 0755 -p /usr/bin fails here:

mkdir: cannot create directory '/usr': Permission denied

If this actually works on your system, please add a test demonstrating that.

@nbraud
Copy link
Contributor

nbraud commented Aug 7, 2024

I'm not sure I'd call this common personally

For what it's worth, I ran into a fair few cases of things which expect /usr/bin/env to be present during build, which seems fair-enough given that POSIX requires it to be available IIRC.

patchShebangs is a workable solution if the scripts are present in the source tree, though it gets tedious when there are many scripts and it's not obvious whether they are run at build- or run-time; worse, errors won't be apparent until someone cross-compiles the drv and tries to use it.

However, it gets downright painful when the scripts are generated during the build, and even more error-prone: one must patch whatever is generating the scripts (or somehow run part of the build, then patchShebangs then resume the build) and it's even more difficult to ensure the scripts are only used during the build and not shipped in the output.

All in all, that seems to me like extra tedium for packagers, and a source of subtle cross-compilation bugs.

@bergkvist
Copy link
Member Author

bergkvist commented Aug 8, 2024

mkdir -m 0755 -p /usr/bin fails here:

mkdir: cannot create directory '/usr': Permission denied

If this actually works on your system, please add a test demonstrating that.

Running nix-shell in a folder containing the following files seems to work fine for me

./shell.nix

{ pkgs ? import (fetchTarball {
    name = "nixpkgs-24.05";
    url = "https://github.com/NixOS/nixpkgs/archive/refs/tags/24.05.tar.gz";
    sha256 = "sha256:1lr1h35prqkd1mkmzriwlpvxcb34kmhc9dnr48gkm8hh089hifmx";
  }) {}
}:
let
  usrBinEnv = pkgs.makeSetupHook { name = "create-usr-bin-env"; } (pkgs.writeText "create-usr-bin-env.sh" ''
    mkdir -m 0755 -p /usr/bin
    ln -sfn "${pkgs.coreutils}/bin/env" /usr/bin/env
  '');
  sayhello = pkgs.runCommand "sayhello" { nativeBuildInputs = [ usrBinEnv ]; } ''
    mkdir -p $out && cd $out
    ${./makesayhello.sh}
  '';
in
pkgs.mkShell {
  buildInputs = [ sayhello ];
  shellHook = ''sayhello'';
}

./makesayhello.sh

#!/usr/bin/env bash
mkdir -p bin
echo 'echo "Hello world"' > bin/sayhello
chmod +x bin/sayhello

@nbraud
Copy link
Contributor

nbraud commented Aug 8, 2024

Running nix-shell in a folder containing the following files seems to work fine for me

That fails here, both with nixpkgs-24.05 and a recent version of nixpkgs-unstable, with and without remote builds.
In any case, adding a test to the PR would make things less ambiguous, as it would ensure we are building the exact same thing, and show whether the test passes on @ofborg

@FliegendeWurst FliegendeWurst added the awaiting_changes (old Marvin label, do not use) label Dec 4, 2024
@@ -1448,6 +1448,12 @@ with pkgs;
#package writers
writers = callPackage ../build-support/writers { };

# Useful for creating /usr/bin/env in a build environment to run build scripts with #!/usr/bin/env shebang
usrBinEnv = makeSetupHook { name = "create-usr-bin-env"; } (pkgs.writeText "create-usr-bin-env.sh" ''
mkdir -m 0755 -p /usr/bin
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm getting

mkdir: cannot create directory '/usr': Permission denied

and ls -la / && id from that hook looks like so

total 0
drwxr-x---   1 nobody nixbld   48 Dec  7 13:51 .
drwxr-x---   1 nobody nixbld   48 Dec  7 13:51 ..
drwxr-xr-x   1 nobody nogroup   4 Dec  7 13:51 bin
drwx------   1 nixbld nixbld    0 Dec  7 13:51 build
drwxr-xr-x   1 nobody nogroup 120 Dec  7 13:51 dev
dr-xr-xr-x   1 nobody nogroup  32 Dec  7 13:51 etc
drwxr-xr-x   1 nobody nogroup  10 Dec  7 13:51 nix
dr-xr-xr-x 418 nobody nogroup   0 Dec  7 13:51 proc
drwxrwxrwt   1 nobody nogroup   0 Dec  7 13:51 tmp

uid=1000(nixbld) gid=100(nixbld) groups=100(nixbld)

@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 2, 2025
@SigmaSquadron SigmaSquadron removed the awaiting_changes (old Marvin label, do not use) label Jan 5, 2025
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants