Skip to content

Comments

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

Merged
mweinelt merged 1 commit intoNixOS:masterfrom
malt3:fix/bazel/bashWithUtils
Nov 12, 2023
Merged

bazel: use bash with fallback $PATH consisting of basic shell utils#266847
mweinelt merged 1 commit intoNixOS:masterfrom
malt3:fix/bazel/bashWithUtils

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!

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. labels Nov 11, 2023
@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 person. label Nov 12, 2023
@mweinelt mweinelt merged commit e94bcce into NixOS:master Nov 12, 2023
@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 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 101-500 This PR causes between 101 and 500 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bazel cannot be used for remote builds

4 participants