Merge pull request #146389 from ylecornec/ylecornec/bazel_bash_fix

Consistent PATH between ctx.actions.run and ctx.actions.run_shell
2021-12-20 10:00:16 +11:00 · 2021-12-20 10:00:16 +11:00 · d2a93c1fc1
commit d2a93c1fc1
parent 69a2c62cf7 cd842a9e42
5 changed files with 91 additions and 50 deletions
--- a/pkgs/development/tools/build-managers/bazel/bazel_4/actions_path.patch
+++ b/pkgs/development/tools/build-managers/bazel/bazel_4/actions_path.patch
@ -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
--- a/pkgs/development/tools/build-managers/bazel/bazel_4/default.nix
+++ b/pkgs/development/tools/build-managers/bazel/bazel_4/default.nix
@ -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:
    #
@ -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}";
@ -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 {
@ -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;
@ -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 = ''
@ -456,8 +455,8 @@ 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
@ -465,17 +464,17 @@ stdenv.mkDerivation rec {

      # 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
@ -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
@ -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:
@ -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.
@ -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 \
@ -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
  '';
--- a/pkgs/development/tools/build-managers/bazel/cpp-test.nix
+++ b/pkgs/development/tools/build-managers/bazel/cpp-test.nix
@ -44,6 +44,8 @@ let
      ${bazel}/bin/bazel \
        build --verbose_failures \
        --distdir=${distDir} \
+        --curses=no \
+        --sandbox_debug \
          //...
    '';
  };
--- a/pkgs/development/tools/build-managers/bazel/java-test.nix
+++ b/pkgs/development/tools/build-managers/bazel/java-test.nix
@ -50,6 +50,8 @@ let
          --java_toolchain='@bazel_tools//tools/jdk:toolchain_hostjdk8' \
          --javabase='@local_jdk//:jdk' \
          --verbose_failures \
+          --curses=no \
+          --sandbox_debug \
          //:ProjectRunner
    '';
  };
--- a/pkgs/development/tools/build-managers/bazel/protobuf-test.nix
+++ b/pkgs/development/tools/build-managers/bazel/protobuf-test.nix
@ -169,6 +169,8 @@ let
          --java_toolchain='@bazel_tools//tools/jdk:toolchain_hostjdk8' \
          --javabase='@local_jdk//:jdk' \
          --verbose_failures \
+          --curses=no \
+          --sandbox_debug \
          //...
    '';
  };