Skip to content

Commit

Permalink
Merge pull request #146389 from ylecornec/ylecornec/bazel_bash_fix
Browse files Browse the repository at this point in the history
Consistent PATH between ctx.actions.run and ctx.actions.run_shell
  • Loading branch information
uri-canva authored Dec 19, 2021
2 parents 69a2c62 + cd842a9 commit d2a93c1
Show file tree
Hide file tree
Showing 5 changed files with 91 additions and 50 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
diff --git a/src/main/java/com/google/devtools/build/lib/exec/local/PosixLocalEnvProvider.java b/src/main/java/com/google/devtools/build/lib/exec/local/PosixLocalEnvProvider.java
index 6fff2af..7e2877e 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/local/PosixLocalEnvProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/local/PosixLocalEnvProvider.java
@@ -47,6 +47,16 @@ public final class PosixLocalEnvProvider implements LocalEnvProvider {
Map<String, String> env, BinTools binTools, String fallbackTmpDir) {
ImmutableMap.Builder<String, String> result = ImmutableMap.builder();
result.putAll(Maps.filterKeys(env, k -> !k.equals("TMPDIR")));
+
+ // In case we are running on NixOS.
+ // If bash is called with an unset PATH on this platform,
+ // it will set it to /no-such-path and default tools will be missings.
+ // See, https://github.com/NixOS/nixpkgs/issues/94222
+ // So we ensure that minimal dependencies are present.
+ if (!env.containsKey("PATH")){
+ result.put("PATH", "@actionsPathPatch@");
+ }
+
String p = clientEnv.get("TMPDIR");
if (Strings.isNullOrEmpty(p)) {
// Do not use `fallbackTmpDir`, use `/tmp` instead. This way if the user didn't export TMPDIR
index 95642767c6..39d3c62461 100644
--- a/src/main/java/com/google/devtools/build/lib/exec/local/XcodeLocalEnvProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/exec/local/XcodeLocalEnvProvider.java
@@ -74,6 +74,16 @@ public final class XcodeLocalEnvProvider implements LocalEnvProvider {

ImmutableMap.Builder<String, String> newEnvBuilder = ImmutableMap.builder();
newEnvBuilder.putAll(Maps.filterKeys(env, k -> !k.equals("TMPDIR")));
+
+ // In case we are running on NixOS.
+ // If bash is called with an unset PATH on this platform,
+ // it will set it to /no-such-path and default tools will be missings.
+ // See, https://github.com/NixOS/nixpkgs/issues/94222
+ // So we ensure that minimal dependencies are present.
+ if (!env.containsKey("PATH")){
+ newEnvBuilder.put("PATH", "@actionsPathPatch@");
+ }
+
String p = clientEnv.get("TMPDIR");
if (Strings.isNullOrEmpty(p)) {
// Do not use `fallbackTmpDir`, use `/tmp` instead. This way if the user didn't export TMPDIR
94 changes: 44 additions & 50 deletions pkgs/development/tools/build-managers/bazel/bazel_4/default.nix
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ let
for i in ${builtins.toString srcDeps}; do cp $i $out/$(stripHash $i); done
'';

defaultShellPath = lib.makeBinPath
defaultShellUtils =
# Keep this list conservative. For more exotic tools, prefer to use
# @rules_nixpkgs to pull in tools from the nix repository. Example:
#
Expand Down Expand Up @@ -103,7 +103,27 @@ let
# ],
# )
#
[ bash coreutils findutils gawk gnugrep gnutar gnused gzip which unzip file zip python27 python3 ];
# Some of the scripts explicitly depend on Python 2.7. Otherwise, we
# default to using python3. Therefore, both python27 and python3 are
# runtime dependencies.
[
bash
coreutils
file
findutils
gawk
gnugrep
gnused
gnutar
gzip
python27
python3
unzip
which
zip
];

defaultShellPath = lib.makeBinPath defaultShellUtils;

# Java toolchain used for the build and tests
javaToolchain = "@bazel_tools//tools/jdk:toolchain_${buildJdkName}";
Expand Down Expand Up @@ -208,6 +228,11 @@ stdenv.mkDerivation rec {
strictActionEnvPatch = defaultShellPath;
})

(substituteAll {
src = ./actions_path.patch;
actionsPathPatch = defaultShellPath;
})

# bazel reads its system bazelrc in /etc
# override this path to a builtin one
(substituteAll {
Expand Down Expand Up @@ -235,7 +260,7 @@ stdenv.mkDerivation rec {
runLocal = name: attrs: script:
let
attrs' = removeAttrs attrs [ "buildInputs" ];
buildInputs = [ python3 ] ++ (attrs.buildInputs or []);
buildInputs = [ python3 which ] ++ (attrs.buildInputs or []);
in
runCommandCC name ({
inherit buildInputs;
Expand Down Expand Up @@ -358,32 +383,6 @@ stdenv.mkDerivation rec {
# Bazel starts a local server and needs to bind a local address.
__darwinAllowLocalNetworking = true;

# Bazel expects several utils to be available in Bash even without PATH. Hence this hack.
customBash = writeCBin "bash" ''
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
extern char **environ;
int main(int argc, char *argv[]) {
char *path = getenv("PATH");
char *pathToAppend = "${defaultShellPath}";
char *newPath;
if (path != NULL) {
int length = strlen(path) + 1 + strlen(pathToAppend) + 1;
newPath = malloc(length * sizeof(char));
snprintf(newPath, length, "%s:%s", path, pathToAppend);
} else {
newPath = pathToAppend;
}
setenv("PATH", newPath, 1);
execve("${bash}/bin/bash", argv, environ);
return 0;
}
'';

postPatch = let

darwinPatches = ''
Expand Down Expand Up @@ -456,26 +455,26 @@ stdenv.mkDerivation rec {
# We default to python3 where possible. See also `postFixup` where
# python3 is added to $out/nix-support
substituteInPlace "$path" \
--replace /bin/bash ${customBash}/bin/bash \
--replace "/usr/bin/env bash" ${customBash}/bin/bash \
--replace /bin/bash ${bash}/bin/bash \
--replace "/usr/bin/env bash" ${bash}/bin/bash \
--replace "/usr/bin/env python" ${python3}/bin/python \
--replace /usr/bin/env ${coreutils}/bin/env \
--replace /bin/true ${coreutils}/bin/true
done
# bazel test runner include references to /bin/bash
substituteInPlace tools/build_rules/test_rules.bzl \
--replace /bin/bash ${customBash}/bin/bash
--replace /bin/bash ${bash}/bin/bash
for i in $(find tools/cpp/ -type f)
do
substituteInPlace $i \
--replace /bin/bash ${customBash}/bin/bash
--replace /bin/bash ${bash}/bin/bash
done
# Fixup scripts that generate scripts. Not fixed up by patchShebangs below.
substituteInPlace scripts/bootstrap/compile.sh \
--replace /bin/bash ${customBash}/bin/bash
--replace /bin/bash ${bash}/bin/bash
# add nix environment vars to .bazelrc
cat >> .bazelrc <<EOF
Expand All @@ -496,6 +495,8 @@ stdenv.mkDerivation rec {
build --host_javabase='@local_jdk//:jdk'
build --host_java_toolchain='${javaToolchain}'
build --verbose_failures
build --curses=no
build --sandbox_debug
EOF
# add the same environment vars to compile.sh
Expand All @@ -508,6 +509,8 @@ stdenv.mkDerivation rec {
-e "/\$command \\\\$/a --host_javabase='@local_jdk//:jdk' \\\\" \
-e "/\$command \\\\$/a --host_java_toolchain='${javaToolchain}' \\\\" \
-e "/\$command \\\\$/a --verbose_failures \\\\" \
-e "/\$command \\\\$/a --curses=no \\\\" \
-e "/\$command \\\\$/a --sandbox_debug \\\\" \
-i scripts/bootstrap/compile.sh
# This is necessary to avoid:
Expand All @@ -529,21 +532,18 @@ stdenv.mkDerivation rec {
in lib.optionalString stdenv.hostPlatform.isDarwin darwinPatches
+ genericPatches;

buildInputs = [
buildJdk
python3
];
buildInputs = [buildJdk] ++ defaultShellUtils;

# when a command can’t be found in a bazel build, you might also
# need to add it to `defaultShellPath`.
nativeBuildInputs = [
coreutils
installShellFiles
zip
makeWrapper
python3
unzip
makeWrapper
which
customBash
zip
] ++ lib.optionals (stdenv.isDarwin) [ cctools libcxx CoreFoundation CoreServices Foundation ];

# Bazel makes extensive use of symlinks in the WORKSPACE.
Expand Down Expand Up @@ -573,7 +573,7 @@ stdenv.mkDerivation rec {
# Note that .bazelversion is always correct and is based on bazel-*
# executable name, version checks should work fine
export EMBED_LABEL="${version}- (@non-git)"
${customBash}/bin/bash ./bazel_src/compile.sh
${bash}/bin/bash ./bazel_src/compile.sh
./bazel_src/scripts/generate_bash_completion.sh \
--bazel=./bazel_src/output/bazel \
--output=./bazel_src/output/bazel-complete.bash \
Expand Down Expand Up @@ -665,16 +665,10 @@ stdenv.mkDerivation rec {
'';

# Save paths to hardcoded dependencies so Nix can detect them.
# This is needed because the templates get tar’d up into a .jar.
postFixup = ''
mkdir -p $out/nix-support
echo "${customBash} ${defaultShellPath}" >> $out/nix-support/depends
# The templates get tar’d up into a .jar,
# so nix can’t detect python is needed in the runtime closure
# Some of the scripts explicitly depend on Python 2.7. Otherwise, we
# default to using python3. Therefore, both python27 and python3 are
# runtime dependencies.
echo "${python27}" >> $out/nix-support/depends
echo "${python3}" >> $out/nix-support/depends
echo "${defaultShellPath}" >> $out/nix-support/depends
'' + lib.optionalString stdenv.isDarwin ''
echo "${cctools}" >> $out/nix-support/depends
'';
Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/tools/build-managers/bazel/cpp-test.nix
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ let
${bazel}/bin/bazel \
build --verbose_failures \
--distdir=${distDir} \
--curses=no \
--sandbox_debug \
//...
'';
};
Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/tools/build-managers/bazel/java-test.nix
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ let
--java_toolchain='@bazel_tools//tools/jdk:toolchain_hostjdk8' \
--javabase='@local_jdk//:jdk' \
--verbose_failures \
--curses=no \
--sandbox_debug \
//:ProjectRunner
'';
};
Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/tools/build-managers/bazel/protobuf-test.nix
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ let
--java_toolchain='@bazel_tools//tools/jdk:toolchain_hostjdk8' \
--javabase='@local_jdk//:jdk' \
--verbose_failures \
--curses=no \
--sandbox_debug \
//...
'';
};
Expand Down

0 comments on commit d2a93c1

Please sign in to comment.