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

{erlang,elixir}/generic-builder: /bin/sh fixes #37946

Closed
wants to merge 2 commits into from
Closed

{erlang,elixir}/generic-builder: /bin/sh fixes #37946

wants to merge 2 commits into from

Conversation

pbogdan
Copy link
Member

@pbogdan pbogdan commented Mar 27, 2018

Motivation for this change

See commit messages.

I would propose backporting to 18.03.


  • ran nox-review with nixStable2 obtained from nixpkgs @
    c5f141f (from before backports of /bin/sh fixes).

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@@ -66,6 +66,8 @@ in stdenv.mkDerivation ({

postPatch = ''
patchShebangs make
substituteInPlace erts/etc/unix/Install.src \
--replace "#!/bin/sh" "${stdenv.shell}"
Copy link
Member

Choose a reason for hiding this comment

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

is the #! dropped here intentional?

Copy link
Member Author

@pbogdan pbogdan Mar 27, 2018

Choose a reason for hiding this comment

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

Oh my.. Changed the match to be more precise and didn't update the replacement.. Thank you for catching this!

@dezgeg
Copy link
Contributor

dezgeg commented Mar 27, 2018

If there still are hydra builders with the crap /bin/sh, the right thing is to complain more to get the hydra fixed.

Otherwise this is still broken on 17.09 for instance.

Certain Nix versions used to provide a very feature limited shell as the sandbox
/bin/sh. When built in such an environment erlang build would succeed however
the resulting package would lack few .boot files. This would result in build
failures of erlang projects when built with such a package.

The boot files are supposed to be included in the erlang package by being
installed with the Install script, however the usage of certain glob patterns
not supported by the sandbox /bin/sh and not aborting on errors would cause them
to be missing.

As such Nix versions may still be out in the wild, including Hydra builders, we
patch the source of the Install script to use stdenv's shell rather than relying
on any particular feature set of the sandbox /bin/sh.

For an example build of erlang where the issue can be seen visit
https://hydra.nixos.org/build/70845721/log -> CTRL-F "cp: cannot stat".

See also:

- #36853
- #36823
- #37638
Certain Nix versions used to provide a very feature limited shell as the sandbox
/bin/sh. When built in such an environment elixir build would fail due to usage
of certain glob patterns not supported by the sandbox /bin/sh in bin/elixir.

As such Nix versions may still be out in the wild, including Hydra builders, we
patch bin/elixir to avoid reliance on any particular feature set of the sandbox
/bin/sh.
@pbogdan
Copy link
Member Author

pbogdan commented Apr 9, 2018

As far as I can tell this is no longer needed.

@pbogdan pbogdan closed this Apr 9, 2018
@pbogdan pbogdan deleted the erlang-shebangs branch December 3, 2019 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants