commit fec7962b0ea19572382578a1e5e3088754292ead
author ajurkowski <ajurkowski@google.com> Tue May 03 09:05:14 2022 -0700
committer Copybara-Service <copybara-worker@google.com> Tue May 03 09:06:26 2022 -0700
tree 51f7e249558cc2d1c7db47aeccb508b91875a8d0
parent a8014266482def3ebf87dfccb50d1b42efcc91a3

Do not crash tests when one of the `{JAVA,PYTHON}_RUNFILES` variables is overridden.

Environment collection uses `ImmutableMap.Builder::buildOrThrow` for the variables map which will crash when user specifies only one of the runfiles variables on the command line. Change the logic to pick up the values specified by the user instead.

PiperOrigin-RevId: 446207370

src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java

diff --git a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
index 0c8dbc3..526ffd7 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
@@ -85,12 +85,12 @@
       return environment;
     } else {
       ImmutableMap.Builder<String, String> env = ImmutableMap.builder();
-      env.putAll(environment);
       // TODO(bazel-team): Unify these into a single env variable.
       String runfilesRootString = runfilesRoot.getPathString();
       env.put("JAVA_RUNFILES", runfilesRootString);
       env.put("PYTHON_RUNFILES", runfilesRootString);
-      return env.buildOrThrow();
+      env.putAll(environment);
+      return env.buildKeepingLast();
     }
   }
 

src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java

diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java b/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java
index 7e5a44a..02b63d8 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/util/BuildIntegrationTestCase.java
@@ -613,7 +613,7 @@
    *
    * @param target the label of the target whose artifacts are requested.
    */
-  protected Iterable<Artifact> getArtifacts(String target)
+  protected ImmutableList<Artifact> getArtifacts(String target)
       throws LabelSyntaxException, NoSuchPackageException, NoSuchTargetException,
           InterruptedException, TransitionException, InvalidConfigurationException {
     return getFilesToBuild(getConfiguredTarget(target)).toList();

src/test/shell/bazel/bazel_test_test.sh

diff --git a/src/test/shell/bazel/bazel_test_test.sh b/src/test/shell/bazel/bazel_test_test.sh
index 1399b52..709530a 100755
--- a/src/test/shell/bazel/bazel_test_test.sh
+++ b/src/test/shell/bazel/bazel_test_test.sh
@@ -144,6 +144,38 @@
   expect_log "ws: bar$"
 }
 
+function test_runfiles_java_runfiles_merges_env_vars() {
+  runfiles_merges_runfiles_env_vars JAVA_RUNFILES PYTHON_RUNFILES
+}
+
+function test_runfiles_python_runfiles_merges_env_vars() {
+  runfiles_merges_runfiles_env_vars PYTHON_RUNFILES JAVA_RUNFILES
+}
+
+# Usage: runfiles_merges_runfiles_env_vars overridden unchanged
+function runfiles_merges_runfiles_env_vars() {
+  local -r overridden=$1
+  local -r unchanged=$2
+  cat > WORKSPACE <<EOF
+workspace(name = "bar")
+EOF
+  add_rules_cc_to_workspace WORKSPACE
+  mkdir -p foo
+  cat > foo/foo.sh <<'EOF'
+#!/bin/sh
+echo "JAVA_RUNFILES: ${JAVA_RUNFILES}"
+echo "PYTHON_RUNFILES: ${PYTHON_RUNFILES}"
+EOF
+  chmod +x foo/foo.sh
+  echo 'sh_test(name = "foo", srcs = ["foo.sh"])' > foo/BUILD
+
+  bazel test --test_env="${overridden}"=override --test_output=all \
+      //foo >& "${TEST_log}" || fail "Test failed"
+
+  expect_log "${overridden}: /.*/execroot/bar/override"
+  expect_log "${unchanged}: /.*/execroot/bar/bazel-out/[^/]\+-fastbuild/bin/foo/foo.runfiles"
+}
+
 function test_run_under_external_label_with_options() {
   mkdir -p testing run || fail "mkdir testing run failed"
   cat <<EOF > run/BUILD