bazelbuild / bazelbuild/bazel

Caching behavior related to PATH and use_default_shell_env are problematic

Open
#18,809 0 comments 0 reactions 0 assignees View on GitHub
help wanted P3 team-Rules-CPP type: bug
Dominant language
Java
Stars
25.8k
Forks
4.6k
Avg merge
2d 20h
Merged PRs (30d)
72

Description

Currently:

- The Bazel action environment (which also affects its cache key) inherit `PATH` and `LD_LIBRARY_PATH` by default. If `--incompatible_strict_action_env` is specified, then a default `PATH` is used and `LD_LIBRARY_PATH` is not inherited. (Or these environment variables can be explicitly overridden via `--action_env`.)
- The `use_default_shell_env` option defaults to `False`, which means by default commands are run with no `PATH`. If `use_default_shell_env=True` are specified, then the normal `PATH` and `LD_LIBRARY_PATH` values for the `action_env` are used.

This leads to the following problems:

- When invoking Bazel from a Python setup.py script in order to build a Python extension with Bazel, as is done by [tensorstore](https://github.com/google/tensorstore/blob/e23e45e75dc73ff824afdad6e2d57df99bb44d14/setup.py#L183), the `PATH` may include temporary virtualenv directories created by pip. Even when using `pip install -e .` to install for local development purposes, pip will by default use an isolated build environment (which results in these temporary directories being added to the `PATH` in order to isolate build dependencies. This means that neither local nor remote caching works.
- If we specify `--incompatible_strict_action_env`, then `PATH` is no longer passed to the action, and no longer affects the cache key, but this breaks building in environments/with toolchains that require `PATH` and/or `LD_LIBRARY_PATH` be set. For example, on Windows we may wish to use a self-contained version of MSVC or mingw where we haven't copied the various runtime DLLs to the Windows system directory. As a result, any binaries built will only run if the `PATH` is preserved so that they can locate the DLLs that they require (Windows does not have the equivalent of rpath). (See https://github.com/bazelbuild/bazel/issues/15059)
- Even if we don't specify `--incompatible_strict_action_env`, a lot of rules still don't work with toolchains that require `PATH` to be set, because they invoke executables without setting `use_default_shell_env=True`. Since the rule author doesn't know the details of the toolchain or system environment, they cannot know that the default `use_default_shell_env=False` will be problematic.

I propose the following changes:

- Introduce `--exclude_action_env_from_cache=X` to exclude a given action environment variable (if inherited or specified on the command-line, but not if specified in the rule directly) from the cache key. (Previously proposed here some years ago https://groups.google.com/g/bazel-discuss/c/AZek-OR1t7A)
- Introduce `--incompatible_exclude_path_from_action_cache_key`. If set, this is equivalent to specifying `--exclude_action_env_from_cache={PATH,LD_LIBRARY_PATH}`.
- Introduce `--incompatible_use_default_shell_env` which sets `use_default_shell_env=True` by default. This would require https://github.com/bazelbuild/bazel/pull/18235 to be practical.

As noted in https://groups.google.com/g/bazel-discuss/c/AZek-OR1t7A, including `PATH` in the action cache key does not ensure hermeticity anyway except in special cases such as when using nix, where the `PATH` is guaranteed to change if any of the executables change. In this rare case where including `PATH` in the cache key is helpful, `--incompatible_exclude_path_from_action_cache_key=false` can be specified by the user.

In https://github.com/bazelbuild/bazel/issues/15059 an alternative to defaulting `use_default_shell_env=True` to solve the issues with mingw is proposed, that the toolchain should somehow add all of the runtime DLLs as runfiles. Note that the issue is not specific to mingw, though --- we may wish to build with a self-contained copy of MSVC (in fact we do this in the tensorstore CI build) and the same issue applies there, so this would basically be needed for all toolchains. This may work, but in general by breaking normal expectations of how the `PATH` should be handled, creates additional trouble for users. For example, if any other binaries used by actions need DLLs, those DLLs have to be copied to the same directory as the binary on Windows if `PATH` cannot be relied upon.

Contributor guide

Open the contributing guide

Research direction

Start by reading the proposed behavior and the tensorstore setup.py reference, especially the Bazel invocation at line 183. Review issues 15059 and the prerequisite pull request 18235 for related constraints. Done means implementing and documenting the proposed action-environment cache controls and default shell-environment behavior, with validation of their effects on caching and toolchains.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
build-system, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.