Skip to content

Commit

Permalink
Prevent a crash when using --repo_env=VAR without a value
Browse files Browse the repository at this point in the history
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 bazelbuild#12886.

PiperOrigin-RevId: 362506900
  • Loading branch information
EdSchouten authored and Copybara-Service committed Mar 12, 2021
1 parent 03fd541 commit 3ebf658
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 1 deletion.
Expand Up @@ -248,7 +248,14 @@ public void exit(AbruptExitException exception) {
CoreOptions configOpts = options.getOptions(CoreOptions.class);
if (configOpts != null) {
for (Map.Entry<String, String> entry : configOpts.repositoryEnvironment) {
repoEnv.put(entry.getKey(), entry.getValue());
String name = entry.getKey();
String value = entry.getValue();
if (value == null) {
value = System.getenv(name);
}
if (value != null) {
repoEnv.put(name, value);
}
}
}
}
Expand Down
13 changes: 13 additions & 0 deletions src/test/shell/bazel/starlark_repository_test.sh
Expand Up @@ -889,6 +889,7 @@ EOF
cat > .bazelrc <<EOF
build:foo --repo_env=FOO=foo
build:bar --repo_env=FOO=bar
build:qux --repo_env=FOO
EOF

bazel build --config=foo //:repoenv //:unrelated
Expand Down Expand Up @@ -924,6 +925,18 @@ EOF
|| fail "Expected FOO to be visible to repo rules"
diff unrelated1.txt unrelated3.txt \
|| fail "Expected unrelated action to not be rerun"

FOO=qux bazel build --config=qux //:repoenv //:unrelated
# The new config should be picked up, but the unrelated target should
# not be rerun
cp `bazel info bazel-genfiles 3>/dev/null`/repoenv.txt repoenv4.txt
cp `bazel info bazel-genfiles 3> /dev/null`/unrelated.txt unrelated4.txt
echo; cat repoenv4.txt; echo; cat unrelated4.txt; echo

grep -q 'FOO=qux' repoenv4.txt \
|| fail "Expected FOO to be visible to repo rules"
diff unrelated1.txt unrelated4.txt \
|| fail "Expected unrelated action to not be rerun"
}

function test_repo_env_inverse() {
Expand Down

0 comments on commit 3ebf658

Please sign in to comment.