do not leak PATH, LD_LIBRARY_PATH, and TMPDIR into the shell environment by default

[benjaminp]

BazelConfiguration.setupShellEnvironement makes the default shell environment inherit the PATH, LD_LIBRARY_PATH, and TMPDIR environmental variables from the server environment (i.e., what they are at server startup time). I can see why this might be useful, but I don't think it should be the default behavior. I observed that this behavior was a source of indeterminism when building the same code on different machines; the build products were the same, but as environment affects the action key, remote caching across the machines didn't work. Why not be hermetic by default and have users who need to change these variables pass, e.g., --action_env=PATH?

If the behavior can't be changed, I would at least like a way to unset those environmental variables in the shell environment from the command line. AFAICT, with --action_env currently, the best you can do is set them to an empty string.

[iirina]

@gregestren can you take a look at this? Thanks.

[ulfjack]

This is definitely on our todo list, but I'm not sure when we'll get to it. Adding 1.0 as the expected milestone, because it needs to be fixed by then (although earlier would be better).

[AustinSchuh]

There's a pretty easy workaround.

Modify //tools/bazel in your repo to set the environment when it executes Bazel. We do something like:

ENVIRONMENT_VARIABLES=()
ENVIRONMENT_VARIABLES+=(HOSTNAME="${HOSTNAME}")
ENVIRONMENT_VARIABLES+=(SHELL="${SHELL}")
ENVIRONMENT_VARIABLES+=(USER="${USER}")
ENVIRONMENT_VARIABLES+=(PATH="${PATH}")
ENVIRONMENT_VARIABLES+=(LANG="${LANG}")
ENVIRONMENT_VARIABLES+=(HOME="${HOME}")
ENVIRONMENT_VARIABLES+=(LOGNAME="${LOGNAME}")
ENVIRONMENT_VARIABLES+=(TERM="${TERM}")

if [[ ! -z "${DISPLAY+x}" ]]; then
  ENVIRONMENT_VARIABLES+=(DISPLAY="${DISPLAY}")
fi

exec -a "${VERSION_BAZEL}" env -i \
    "${ENVIRONMENT_VARIABLES[@]}" \
    "${VERSION_BAZEL}-real" "$@"

That lets us control which version of bazel is in use for each revision of the repository, and the environment. That makes it easy to handle compatibility.

[ulfjack]

Ugh, this could be a problem...

[ittaiz]

@ulfjack Why would this be a problem?

[ulfjack]

We have some users who are very eager to use remote caching, and we've been trying to get all the necessary changes into 0.5.2, so this bug could be a problem for them.

[ulfjack]

I confirm that I see cache misses on changes to PATH in combination with bazel server restarts. :-(

[ulfjack]

I've started on adding a flag for migration. We can't just remove the env variables, since that is a breaking change.

[ulfjack]

I'd suggest setting --experimental_strict_action_env. It requires Bazel 0.5.3 or higher.

[benjaminp]

Yep, that option seems do precisely the right thing for me. Thanks.

Why does the option description mention $LANG, though? It doesn't actually seem to get set ever.

[ulfjack]

Sorry about that. That was from an earlier version of the change where I set LANG, which I decided not to do after all.

[laszlocsomor]

@mboes : No, because Bazel uses different output directories for these workspaces, plus the workspace-relative file names (of the output files) are different.

[ulfjack]

There are multiple caches here. You can get a cache hit on the local or remote spawn cache, but not on the action cache, which is workspace-local, or on the in-memory cache, which is also workspace-local. If the two rules are in different locations (say //foo and //bar, then the names of the output files are likely different, in which case you can't get a cache hit either.

[mboes]

@ulfjack thanks. Is there any place you can point me to, to understand the difference between the spawn cache and the action cache? Does the (local or remote) spawn cache cache action outputs, or is that the action cache?

Being able to cache action outputs is pretty important for our use case. Haskell packages are notoriously slow to build. Many of them have source code (but not binary artifacts) published in a central location (called Hackage). We're considering running a remote cache that obviates the need for developers to have to rebuild these centrally published open source package every time they create a new project that uses them.

[ulfjack]

The spawn caches cache action outputs, the action cache works with outputs in the output tree (it doesn't cache files separately).

[mboes]

Gotcha. So IIUC, in the use case envisioned above, we should be able to avoid package rebuilds even across multiple (differently named) workspaces.

[ulfjack]

Yes, absolutely.

[ittaiz]

Ulf, Any chance for a writeup (blog post?) about how and when the shell environment and its paths effect the cache?

…

On Mon, Jan 22, 2018 at 12:15 PM Ulf Adams ***@***.***> wrote: Yes, absolutely. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2574 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABUIF7zh_-cBC5EjIBDD7x6V9Xz1ool0ks5tNF_PgaJpZM4MJl2z> .

[danny-kuminov]

qq: is there an ETA for when --experimental_strict_action_env=true will be the default ?

[ulfjack]

I've just fixed a number of cases where --action_env did not properly apply. I think we might want to wait for another release before we flip the default.

[danny-kuminov]

I see - makes sense, thank you!

[mboes]

I think we might want to wait for another release before we flip the default.

Will it become the default after 0.16.0? I too am pretty keen on this. :)

[buchgr]

This has been enabled in Bazel 0.22