Skip to content

Commit 3ebf658

Browse files
EdSchoutencopybara-github
authored andcommitted
Prevent a crash when using --repo_env=VAR without a value
The --repo_env option is documented in the same way as --action_env. In addition to allowing you to set explicit values, you can also use it to explicitly pick values from the environment in which Bazel was invoked. Unfortunately, this causes a null pointer exception in Starlark due to a null string stored as a map value. This change extends the logic of converting --repo_env to a map to take null values into account. When null, the value is loaded from the current environment. This behaviour is useful in case you want to do something like this: --incompatible_strict_action_env --action_env=PATH=... --repo_env=PATH This allows you to run build actions with a strict value for $PATH (e.g., to get a decent action cache hit rate in case of remote execution), while still allowing repository_ctx.which() to find tools on the host system using $PATH. Closes #12886. PiperOrigin-RevId: 362506900
1 parent 03fd541 commit 3ebf658

File tree

2 files changed

+21
-1
lines changed

2 files changed

+21
-1
lines changed

src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,14 @@ public void exit(AbruptExitException exception) {
248248
CoreOptions configOpts = options.getOptions(CoreOptions.class);
249249
if (configOpts != null) {
250250
for (Map.Entry<String, String> entry : configOpts.repositoryEnvironment) {
251-
repoEnv.put(entry.getKey(), entry.getValue());
251+
String name = entry.getKey();
252+
String value = entry.getValue();
253+
if (value == null) {
254+
value = System.getenv(name);
255+
}
256+
if (value != null) {
257+
repoEnv.put(name, value);
258+
}
252259
}
253260
}
254261
}

src/test/shell/bazel/starlark_repository_test.sh

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -889,6 +889,7 @@ EOF
889889
cat > .bazelrc <<EOF
890890
build:foo --repo_env=FOO=foo
891891
build:bar --repo_env=FOO=bar
892+
build:qux --repo_env=FOO
892893
EOF
893894

894895
bazel build --config=foo //:repoenv //:unrelated
@@ -924,6 +925,18 @@ EOF
924925
|| fail "Expected FOO to be visible to repo rules"
925926
diff unrelated1.txt unrelated3.txt \
926927
|| fail "Expected unrelated action to not be rerun"
928+
929+
FOO=qux bazel build --config=qux //:repoenv //:unrelated
930+
# The new config should be picked up, but the unrelated target should
931+
# not be rerun
932+
cp `bazel info bazel-genfiles 3>/dev/null`/repoenv.txt repoenv4.txt
933+
cp `bazel info bazel-genfiles 3> /dev/null`/unrelated.txt unrelated4.txt
934+
echo; cat repoenv4.txt; echo; cat unrelated4.txt; echo
935+
936+
grep -q 'FOO=qux' repoenv4.txt \
937+
|| fail "Expected FOO to be visible to repo rules"
938+
diff unrelated1.txt unrelated4.txt \
939+
|| fail "Expected unrelated action to not be rerun"
927940
}
928941

929942
function test_repo_env_inverse() {

0 commit comments

Comments
 (0)