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

bazel: use bash with fallback $PATH consisting of basic shell utils #266847

Merged
merged 1 commit into from
Nov 12, 2023

Conversation

malt3
Copy link
Contributor

@malt3 malt3 commented Nov 11, 2023

Description of changes

Fixes #265040. Ensure any shell action has a minimal set of tools available (even for remote builds).

This is the second attempt at implementing this. The first attempt (#265041) did not ensure the bash wrapper bashWithDefaultShellUtils is available at runtime.

Please also let me know if this should target staging instead of master.

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/)
  • 23.11 Release Notes (or backporting 23.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.

Second iteration of this change.
First attempt did not ensure `bashWithDefaultShellUtils` is available at runtime.
@malt3
Copy link
Contributor Author

malt3 commented Nov 11, 2023

Currently building jaxlib. Seems to work fine @mweinelt @bjornfor.
I'll test this in more packages that depend on Bazel to be sure!

@malt3
Copy link
Contributor Author

malt3 commented Nov 12, 2023

Result of nixpkgs-review pr 266847 run on x86_64-linux 1

28 packages marked as broken and skipped:
  • python310Packages.distrax
  • python310Packages.distrax.dist
  • python310Packages.dm-sonnet
  • python310Packages.dm-sonnet.dist
  • python310Packages.elegy
  • python310Packages.elegy.dist
  • python310Packages.rlax
  • python310Packages.rlax.dist
  • python310Packages.tensorflow-datasets
  • python310Packages.tensorflow-datasets.dist
  • python310Packages.treeo
  • python310Packages.treeo.dist
  • python310Packages.treex
  • python310Packages.treex.dist
  • python311Packages.distrax
  • python311Packages.distrax.dist
  • python311Packages.dm-sonnet
  • python311Packages.dm-sonnet.dist
  • python311Packages.elegy
  • python311Packages.elegy.dist
  • python311Packages.rlax
  • python311Packages.rlax.dist
  • python311Packages.tensorflow-datasets
  • python311Packages.tensorflow-datasets.dist
  • python311Packages.treeo
  • python311Packages.treeo.dist
  • python311Packages.treex
  • python311Packages.treex.dist
173 packages built:
  • bazel (bazel_6)
  • envoy
  • libretranslate (python311Packages.libretranslate)
  • libretranslate.dist (python311Packages.libretranslate.dist)
  • pomerium
  • python310Packages.aeppl
  • python310Packages.aeppl.dist
  • python310Packages.aesara
  • python310Packages.aesara.dist
  • python310Packages.argos-translate-files
  • python310Packages.argos-translate-files.dist
  • python310Packages.argostranslate
  • python310Packages.argostranslate.dist
  • python310Packages.arviz
  • python310Packages.arviz.dist
  • python310Packages.augmax
  • python310Packages.augmax.dist
  • python310Packages.awkward
  • python310Packages.awkward.dist
  • python310Packages.bambi
  • python310Packages.bambi.dist
  • python310Packages.blackjax
  • python310Packages.blackjax.dist
  • python310Packages.chex
  • python310Packages.chex.dist
  • python310Packages.coffea
  • python310Packages.coffea.dist
  • python310Packages.correctionlib
  • python310Packages.correctionlib.dist
  • python310Packages.ctranslate2
  • python310Packages.ctranslate2.dist
  • python310Packages.dalle-mini
  • python310Packages.dalle-mini.dist
  • python310Packages.dask-awkward
  • python310Packages.dask-awkward.dist
  • python310Packages.dm-haiku
  • python310Packages.dm-haiku.dist
  • python310Packages.dm-haiku.testsout
  • python310Packages.equinox
  • python310Packages.equinox.dist
  • python310Packages.faster-whisper
  • python310Packages.faster-whisper.dist
  • python310Packages.flax
  • python310Packages.flax.dist
  • python310Packages.gymnasium
  • python310Packages.gymnasium.dist
  • python310Packages.jax
  • python310Packages.jax.dist
  • python310Packages.jaxlib (python310Packages.jaxlib-build ,python310Packages.jaxlibWithoutCuda)
  • python310Packages.jaxlib.dist (python310Packages.jaxlib-build.dist ,python310Packages.jaxlibWithoutCuda.dist)
  • python310Packages.jaxlibWithCuda
  • python310Packages.jaxlibWithCuda.dist
  • python310Packages.jaxopt
  • python310Packages.jaxopt.dist
  • python310Packages.jmp
  • python310Packages.jmp.dist
  • python310Packages.libretranslate
  • python310Packages.libretranslate.dist
  • python310Packages.mplhep
  • python310Packages.mplhep.dist
  • python310Packages.numpyro
  • python310Packages.numpyro.dist
  • python310Packages.objax
  • python310Packages.objax.dist
  • python310Packages.optax
  • python310Packages.optax.dist
  • python310Packages.optax.testsout
  • python310Packages.pymc
  • python310Packages.pymc.dist
  • python310Packages.pytensor
  • python310Packages.pytensor.dist
  • python310Packages.skrl
  • python310Packages.skrl.dist
  • python310Packages.tensorflow-bin
  • python310Packages.tensorflow-bin.dist
  • python310Packages.tensorflow-probability
  • python310Packages.tensorflow-probability.dist
  • python310Packages.translatehtml
  • python310Packages.translatehtml.dist
  • python310Packages.trfl
  • python310Packages.trfl.dist
  • python310Packages.uproot
  • python310Packages.uproot.dist
  • python310Packages.vector
  • python310Packages.vector.dist
  • python310Packages.vqgan-jax
  • python310Packages.vqgan-jax.dist
  • python311Packages.aeppl
  • python311Packages.aeppl.dist
  • python311Packages.aesara
  • python311Packages.aesara.dist
  • python311Packages.argos-translate-files
  • python311Packages.argos-translate-files.dist
  • python311Packages.argostranslate
  • python311Packages.argostranslate.dist
  • python311Packages.arviz
  • python311Packages.arviz.dist
  • python311Packages.augmax
  • python311Packages.augmax.dist
  • python311Packages.awkward
  • python311Packages.awkward.dist
  • python311Packages.bambi
  • python311Packages.bambi.dist
  • python311Packages.blackjax
  • python311Packages.blackjax.dist
  • python311Packages.chex
  • python311Packages.chex.dist
  • python311Packages.coffea
  • python311Packages.coffea.dist
  • python311Packages.correctionlib
  • python311Packages.correctionlib.dist
  • python311Packages.ctranslate2
  • python311Packages.ctranslate2.dist
  • python311Packages.dalle-mini
  • python311Packages.dalle-mini.dist
  • python311Packages.dask-awkward
  • python311Packages.dask-awkward.dist
  • python311Packages.dm-haiku
  • python311Packages.dm-haiku.dist
  • python311Packages.dm-haiku.testsout
  • python311Packages.equinox
  • python311Packages.equinox.dist
  • python311Packages.faster-whisper
  • python311Packages.faster-whisper.dist
  • python311Packages.flax
  • python311Packages.flax.dist
  • python311Packages.gymnasium
  • python311Packages.gymnasium.dist
  • python311Packages.jax
  • python311Packages.jax.dist
  • python311Packages.jaxlib (python311Packages.jaxlib-build ,python311Packages.jaxlibWithoutCuda)
  • python311Packages.jaxlib.dist (python311Packages.jaxlib-build.dist ,python311Packages.jaxlibWithoutCuda.dist)
  • python311Packages.jaxlibWithCuda
  • python311Packages.jaxlibWithCuda.dist
  • python311Packages.jaxopt
  • python311Packages.jaxopt.dist
  • python311Packages.jmp
  • python311Packages.jmp.dist
  • python311Packages.mplhep
  • python311Packages.mplhep.dist
  • python311Packages.numpyro
  • python311Packages.numpyro.dist
  • python311Packages.objax
  • python311Packages.objax.dist
  • python311Packages.optax
  • python311Packages.optax.dist
  • python311Packages.optax.testsout
  • python311Packages.pymc
  • python311Packages.pymc.dist
  • python311Packages.pytensor
  • python311Packages.pytensor.dist
  • python311Packages.skrl
  • python311Packages.skrl.dist
  • python311Packages.tensorflow-bin
  • python311Packages.tensorflow-bin.dist
  • python311Packages.tensorflow-probability
  • python311Packages.tensorflow-probability.dist
  • python311Packages.translatehtml
  • python311Packages.translatehtml.dist
  • python311Packages.trfl
  • python311Packages.trfl.dist
  • python311Packages.uproot
  • python311Packages.uproot.dist
  • python311Packages.vector
  • python311Packages.vector.dist
  • python311Packages.vqgan-jax
  • python311Packages.vqgan-jax.dist
  • tts
  • tts.dist
  • whisper-ctranslate2
  • whisper-ctranslate2.dist
  • wyoming-faster-whisper
  • wyoming-faster-whisper.dist

@malt3 malt3 marked this pull request as ready for review November 12, 2023 13:16
@malt3 malt3 requested a review from Profpatsch as a code owner November 12, 2023 13:16
@delroth delroth added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Nov 12, 2023
@mweinelt mweinelt merged commit e94bcce into NixOS:master Nov 12, 2023
18 checks passed
@malt3 malt3 deleted the fix/bazel/bashWithUtils branch November 12, 2023 17:20
boltzmannrain added a commit to boltzmannrain/nixpkgs that referenced this pull request Nov 20, 2023
https://hydra.nixos.org/build/240805256/nixlog/1
https://hydra.nixos.org/build/240805170/nixlog/2
Failure is a bit obscured but long story short, a script within
bazel gets custom nixpkgs shebang which in turn makes shell run
in POSIX-compatible mode. Bazel expects bash in non-POSIX mode
and osx-specific script starts to fail due to `set -e` and subshell
interaction differences in those modes (sub-shells and functions
suddently start inheriting `set -e` and fail to produce desired
output). More debug info is available in NixOS#267670

Shell scripts aren't guaranteed to work as interpreters in shebang.
In particular thin shell wrappers aren't shebang-ready on MacOS.
It may work sometimes depending on what exactly would try to execute
a script with such shebang, but generally it's not guaranteed to work.
See NixOS#124556

Bash wrapper was introduced in NixOS#266847 and so far seems like the
issue only affects darwin builds: hydra failure is in osx-specific
script, also shebang issue is usually darwin-specific.

Let's wrap it as a native binary to make it shebang-compatible.

The wrapper is only currently added to `bazel_6` so no need for
changes in other versions.

ZHF: NixOS#265948
github-actions bot pushed a commit that referenced this pull request Dec 7, 2023
https://hydra.nixos.org/build/240805256/nixlog/1
https://hydra.nixos.org/build/240805170/nixlog/2
Failure is a bit obscured but long story short, a script within
bazel gets custom nixpkgs shebang which in turn makes shell run
in POSIX-compatible mode. Bazel expects bash in non-POSIX mode
and osx-specific script starts to fail due to `set -e` and subshell
interaction differences in those modes (sub-shells and functions
suddently start inheriting `set -e` and fail to produce desired
output). More debug info is available in #267670

Shell scripts aren't guaranteed to work as interpreters in shebang.
In particular thin shell wrappers aren't shebang-ready on MacOS.
It may work sometimes depending on what exactly would try to execute
a script with such shebang, but generally it's not guaranteed to work.
See #124556

Bash wrapper was introduced in #266847 and so far seems like the
issue only affects darwin builds: hydra failure is in osx-specific
script, also shebang issue is usually darwin-specific.

Let's wrap it as a native binary to make it shebang-compatible.

The wrapper is only currently added to `bazel_6` so no need for
changes in other versions.

ZHF: #265948
(cherry picked from commit 7377bba)
bjornfor pushed a commit that referenced this pull request Dec 7, 2023
https://hydra.nixos.org/build/240805256/nixlog/1
https://hydra.nixos.org/build/240805170/nixlog/2
Failure is a bit obscured but long story short, a script within
bazel gets custom nixpkgs shebang which in turn makes shell run
in POSIX-compatible mode. Bazel expects bash in non-POSIX mode
and osx-specific script starts to fail due to `set -e` and subshell
interaction differences in those modes (sub-shells and functions
suddently start inheriting `set -e` and fail to produce desired
output). More debug info is available in #267670

Shell scripts aren't guaranteed to work as interpreters in shebang.
In particular thin shell wrappers aren't shebang-ready on MacOS.
It may work sometimes depending on what exactly would try to execute
a script with such shebang, but generally it's not guaranteed to work.
See #124556

Bash wrapper was introduced in #266847 and so far seems like the
issue only affects darwin builds: hydra failure is in osx-specific
script, also shebang issue is usually darwin-specific.

Let's wrap it as a native binary to make it shebang-compatible.

The wrapper is only currently added to `bazel_6` so no need for
changes in other versions.

ZHF: #265948
(cherry picked from commit 7377bba)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 101-500 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bazel cannot be used for remote builds
4 participants