Skip to content

Commit

Permalink
Flip --experimental_strict_action_env on by default.
Browse files Browse the repository at this point in the history
See bazelbuild#2574. For a while, many actions did not respect the --action_env command line option, but those have been fixed now. So, I think it's time to flip on this flag for greater hermeticity by default.
  • Loading branch information
benjaminp committed Sep 27, 2018
1 parent 3e0f520 commit bc730be
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 31 deletions.
1 change: 1 addition & 0 deletions compile.sh
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ log "Building output/bazel"
# We set host and target platform directly since the defaults in @bazel_tools
# have not yet been generated.
bazel_build "src:bazel_nojdk${EXE_EXT}" \
--action_env=PATH \
--host_platform=@bazel_tools//platforms:host_platform \
--platforms=@bazel_tools//platforms:target_platform \
|| fail "Could not build Bazel"
Expand Down
23 changes: 8 additions & 15 deletions site/docs/remote-caching.md
Original file line number Diff line number Diff line change
Expand Up @@ -333,12 +333,6 @@ You can pass a user-specific path to the `--disk_cache` flag using the `~` alias
when enabling the disk cache for all developers of a project via the project's
checked in `.bazelrc` file.

To enable cache hits across different workspaces, use the following flag:

```
build --experimental_strict_action_env
```

## Known issues

**Input file modification during a build**
Expand All @@ -353,15 +347,14 @@ build.

**Environment variables leaking into an action**

An action definition contains environment variables. This can be a problem
for sharing remote cache hits across machines. For example, environments
with different `$PATH` variables won't share cache hits. You can specify
`--experimental_strict_action_env` to ensure that that's not the case and
that only environment variables explicitly whitelisted via `--action_env`
are included in an action definition. Bazel's Debian/Ubuntu package used
to install `/etc/bazel.bazelrc` with a whitelist of environment variables
including `$PATH`. If you are getting fewer cache hits than expected, check
that your environment doesn't have an old `/etc/bazel.bazelrc` file.
An action definition contains environment variables. This can be a problem for
sharing remote cache hits across machines. For example, environments with
different `$PATH` variables won't share cache hits. Only environment variables
explicitly whitelisted via `--action_env` are included in an action
definition. Bazel's Debian/Ubuntu package used to install `/etc/bazel.bazelrc`
with a whitelist of environment variables including `$PATH`. If you are getting
fewer cache hits than expected, check that your environment doesn't have an old
`/etc/bazel.bazelrc` file.


**Bazel does not track tools outside a workspace**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public class BazelRuleClassProvider {
public static class StrictActionEnvOptions extends FragmentOptions {
@Option(
name = "experimental_strict_action_env",
defaultValue = "false",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
help =
Expand Down
1 change: 0 additions & 1 deletion src/test/py/bazel/windows_remote_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ def _RunRemoteBazel(self, args, env_remove=None, env_add=None):
'--define=EXECUTOR=remote',
'--remote_executor=localhost:' + str(self._worker_port),
'--remote_cache=localhost:' + str(self._worker_port),
'--experimental_strict_action_env=true',
'--remote_timeout=3600',
'--auth_enabled=false',
'--remote_accept_cached=false',
Expand Down
18 changes: 4 additions & 14 deletions src/test/shell/bazel/bazel_rules_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -287,29 +287,19 @@ EOF
local new_tmpdir="$(mktemp -d "${TEST_TMPDIR}/newfancytmpdirXXXXXX")"
[ -d "${new_tmpdir}" ] || \
fail "Could not create new temporary directory ${new_tmpdir}"
export PATH="$PATH_TO_BAZEL_WRAPPER:/bin:/usr/bin:/random/path"
if is_windows; then
export PATH="$PATH_TO_BAZEL_WRAPPER;/bin;/usr/bin;/random/path"
local old_tmpdir="${TMP:-}"
export TMP="${new_tmpdir}"
else
export PATH="$PATH_TO_BAZEL_WRAPPER:/bin:/usr/bin:/random/path"
local old_tmpdir="${TMPDIR:-}"
export TMPDIR="${new_tmpdir}"
fi
# shut down to force reload of the environment
bazel shutdown
bazel build //pkg:test --spawn_strategy=standalone \
bazel build //pkg:test --spawn_strategy=standalone --action_env=PATH \
|| fail "Failed to build //pkg:test"
if is_windows; then
# As of 2018-07-10, Bazel on Windows sets the PATH to
# "/usr/bin:/bin:" + $PATH of the Bazel server process.
#
# MSYS appears to convert path entries in PATH to Windows style when running
# a native Windows process such as Bazel, but "cygpath -w /bin" returns
# MSYS_ROOT + "\usr\bin".
# The point is, the PATH will be quite different from what we expect on
# Linux. Therefore only assert that the PATH contains
# "$PATH_TO_BAZEL_WRAPPER" and "/random/path", ignore the rest.
local -r EXPECTED_PATH=".*:$PATH_TO_BAZEL_WRAPPER:.*:/random/path"
local -r EXPECTED_PATH="$PATH_TO_BAZEL_WRAPPER;/bin;/usr/bin;/random/path"
# new_tmpdir is based on $TEST_TMPDIR which is not Unix-style -- convert it.
local -r EXPECTED_TMP="$(cygpath -u "$new_tmpdir")"
else
Expand Down

0 comments on commit bc730be

Please sign in to comment.