Don't include PATH in the Action definition by default

[Jakob Buchgraber]

Hi all,

actions in Bazel inherit the PATH environment variable from the host environment (the shell environment the bazel command is invoked in). This has always been problematic for remote caching and execution users as two builds with different PATHs do not share cache hits. For this very reason we introduced the concept of a strict action environment which when enabled would no longer inherit the host PATH to actions but set the PATH to a fixed predefined value (i.e. PATH=/bin/:/usr/bin/:/usr/lib/bin). In the latest Bazel release (0.21) we enabled the strict action environment by default and it really hurt local execution users and so we are rolling this change back again for Bazel 0.22.

That means that we need to come up with better defaults that work for both local and remote. The current best proposal is to have the following default behavior:

• Locally executed actions inherit the PATH and LD_LIBRARY_PATH variable.
• The action message for remote caching and execution does not include the PATH and LD_LIBRARY_PATH in the action definition. This implies that they aren't included in the cache key computation and more importantly that it would now be for the remote execution system to provide a PATH and LD_LIBRARY_PATH variable to the action its executing. In remote caching the locally executed action would also inherit the host environment but also not include the PATH and LD_LIBRARY_PATH environment variables in the action message.
• If a user overrides the PATH/LD_LIBRARY_PATH environment variable via --action_env=PATH=/custom/path then these would be included in the remote action message.

One might say that this proposal makes actions less hermetic, but I'd argue that the problem there is not the PATH but actions relying on PATH lookup as Bazel simply can't track all files made available via PATH.

Please note that this proposal is about the default behavior and I think its very much possible that in the future we might extend i.e. platforms and toolchains to also make it possible to specify environment variables (no discussions about this have happened though and @katre might have good arguments against it).

I'd be interested to learn about your thoughts and concerns about this proposal as well as alternative ideas.

Thanks,

Jakob

Jakob Buchgraber

Software Engineer

Google Germany GmbH

Erika-Mann-Straße 33

80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls sie diese fälschlicherweise erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen, dass die E-Mail an die falsche Person gesendet wurde.

    

This e-mail is confidential. If you received this communication by mistake, please don't forward it to anyone else, please erase all copies and attachments, and please let me know that it has gone to the wrong person.

[George Gensure]

We gave up long ago and forced all users to have the same PATH via a wrapper to compensate for this. Solid proposal, would recommend all of these become controllable by workspace .bazelrc (and that an option exist to unset them).

-George

--
You received this message because you are subscribed to the Google Groups "Bazel remote execution and caching system developer list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-buildfa...@googlegroups.com.
To post to this group, send email to bazel-b...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-buildfarm/CAGQ4vn2GRtfyqna%3DfgXc%3Dp0uM2dHX%2BNV%3DqtrD%2B7bMhKphVHL7w%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[Théophane Hufschmitt]

That’s a great news for us as we almost systematically use nix-shell (think python’s virtualenv, but more isolated and for the whole dependency chain instead of just the python packages) to make our builds more isolated and strict_action_env was getting in the way of that:
One of the ways nix-shell ensures the isolation is by replacing the PATH by a carefully crafted one containing only the executables for the packages specified in the configuration of the environment. This worked quite well with remote caching (and probably remote building too, though I personally didn’t try it) because the PATH (along with other related environment variables like LD_LIBRARY_PATH) was uniquely specifying our environment − which kinda solve the “Bazel simply can’t track all files made available via PATH” because the only way to change what’s available in the PATH is by modifying the PATH itself. So strict_action_env was in fact making our builds less reproducible instead of more.

For our use-case (which tbh I’d like to see more widespread as it’s a great way to ensure isolated builds at a relatively low cost) having the PATH used in the remote cache key was I think a rather good solution as it gave us a really strong confidence in the remote cache at a reasonable price.

​

--

Théophane

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CAGQ4vn2GRtfyqna%3DfgXc%3Dp0uM2dHX%2BNV%3DqtrD%2B7bMhKphVHL7w%40mail.gmail.com.

[Alexandra Goultiaeva]

I think I don't understand what the problem is.

For the users broken by this change, would setting "--action_env=PATH" fix it, or is it harder than that?

I think the issue is that relying on PATH in the actions is unsafe and non-hermetic.

We don't want users to do that inadvertently - that's why it's problematic to allow it by default.

Users who know what they are doing and who have taken steps to make it more safe and hermetic (and/or those who accept the risks) can set the flag.

Or am I mistaken/does the flag not fix the problem?

[Philipp Wollermann]

On Mon, Jan 21, 2019 at 12:42 AM 'Alexandra Goultiaeva' via bazel-discuss <bazel-...@googlegroups.com> wrote:

I think I don't understand what the problem is.

For the users broken by this change, would setting "--action_env=PATH" fix it, or is it harder than that?

I think the issue is that relying on PATH in the actions is unsafe and non-hermetic.

We don't want users to do that inadvertently - that's why it's problematic to allow it by default.

Users who know what they are doing and who have taken steps to make it more safe and hermetic (and/or those who accept the risks) can set the flag.

Or am I mistaken/does the flag not fix the problem?

The problem is that this breaks users without even giving them any benefit. It makes usability worse for people building software on their local machine and it can even make hermeticity worse, too (as Théophane pointed out). Telling these users to "just pass a flag" is not the right solution, IMHO and will lead to people being frustrated with Bazel and everyone just cargo-culting that flag. These users can rightfully ask: Why don't the *other* users who want Bazel to turn have this behavior have to pass a flag to get their stuff working?

First, for remote execution users, the whole discussion and this feature shouldn't matter, because Bazel should not use the PATH, LD_LIBRARY_PATH, and other system-specific environment variables from the local system anyway when it uses sends an action for remote execution (neither in the action cache key, nor send it to the remote execution system to actually be used). I hope we already agree on this and this is how it's implemented, otherwise I'm happy to explain my thoughts here. For remote execution, Bazel should barely send any environment variables that come from the local system, probably even none unless the user explicitly specifies one with --action_env=. Which environment variables are set on a remote execution platform should be the matter of the remote execution platform and its configuration (these can e.g. come from a Docker container that is used as an execution environment).

For local execution users, the "strict action env" feature makes things just worse, as explained above. It operates on the assumption that there is something like a "default environment" that can be used no matter the current system, basically a "one size fits all Linux / macOS / Windows installations" environment. That is just not the case and breaks lots of stuff. It breaks people who install tools in /usr/local or in their $HOME/homebrew, it breaks people who need a special PATH and LD_LIBRARY_PATH for their embedded toolchain, it breaks tools who inspect the system they're running on (we even broke Python's platform detection on Windows, because we filter out the PROCESSOR_ARCHITECTURE environment variable, which causes Python to no longer be able to tell you on which CPU architecture it's running). We broke our own tests (Bazel's own CI is currently running with this feature disabled and with --test_env=PATH at least on Windows, because otherwise *nothing* there would work), we broke nixOS users.

The only case where this feature helps (and Jakob told me that this was also the thought behind the whole strict action env feature, as it's not about increasing hermeticity) is when you use local execution, but want to upload the resulting artifacts to a shared remote cache and then allow other users to get cache hits from there. This obviously doesn't work if everyone has their own individual values for environment variables that are put into the action cache key, so you have to filter or normalize them somehow. How to fix this is still open for debate, but there are already proven solutions outside of Bazel, like using wrappers that are tuned by their users to normalize their environment to something that works / matches their CI environment (George talked about this above and I know that Austin also uses this approach).

Cheers,

Philipp

[Jakob Buchgraber]

On Mon, Jan 21, 2019 at 9:55 AM 'Philipp Wollermann' via bazel-discuss <bazel-...@googlegroups.com> wrote:

On Mon, Jan 21, 2019 at 12:42 AM 'Alexandra Goultiaeva' via bazel-discuss <bazel-...@googlegroups.com> wrote:

I think I don't understand what the problem is.

For the users broken by this change, would setting "--action_env=PATH" fix it, or is it harder than that?

It's as simple as that but we don't want to improve the usability of remote execution at the expense of local execution. Also, as philwo@ pointed out,

using PATH from the host environment for the remote execution environment doesn't seem sound.

First, for remote execution users, the whole discussion and this feature shouldn't matter, because Bazel should not use the PATH, LD_LIBRARY_PATH, and other system-specific environment variables from the local system anyway when it uses sends an action for remote execution (neither in the action cache key, nor send it to the remote execution system to actually be used). I hope we already agree on this and this is how it's implemented, otherwise I'm happy to explain my thoughts here.

For the record, this is not how it's implemented currently. This is what this proposal is partly about.

 

The only case where this feature helps (and Jakob told me that this was also the thought behind the whole strict action env feature, as it's not about increasing hermeticity) is when you use local execution, but want to upload the resulting artifacts to a shared remote cache and then allow other users to get cache hits from there.

In a nutshell, for remote caching it has the same limitations as for local execution. So I think overall this feature is just really broken.

[Andrew Suffield]

My experience of trying to get PATH to behave sensibly so far has led me to stop trying to set it via action_env for any but the most trivial of workspaces, because there is no correct setting for "all actions". Anything using ctx.actions.run_shell needs a fairly large path to get anything done, while my crosstool-managed compiler wants a path that does not include /usr/bin to make sure it doesn't try to use the system compiler (which I would love to do in starlark, but since cc rules are still native I'm doing it with wrapper scripts).

I am increasingly leaning towards managing PATH explicitly in the rules. If a command-line override is beneficial, I suspect it would serve us better if it worked like --strategy and selected specific mnemonics.

--

You received this message because you are subscribed to the Google Groups "bazel-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CAGQ4vn3vyxacVii-jVO3mFgTdE3d0KGnYfQT6Sjjayi4k9wVLA%40mail.gmail.com.

[Philipp Wollermann]

On Mon, Jan 21, 2019 at 10:21 AM Andrew Suffield <asuf...@gmail.com> wrote:
> I am increasingly leaning towards managing PATH explicitly in the rules. If a command-line override is beneficial, I suspect it would serve us better if it worked like --strategy and selected specific mnemonics.

How are you considering managing the PATH variable? I can only think
of three reasonable approaches that won't break on uncommon systems:

- Pass it through as the user has it set in their shell.
- Set it to an empty string.
- Set it to (or prefix it with) a directory that your rule maintains
control over.

I believe that anything else (like what we did in Bazel so far,
override it with a hardcoded string that contains a couple of common
directories like /usr/bin and /bin) will break users.

[Andrew Suffield]

Yes, those are precisely the scenarios that I'm working with.

For things I don't have any better solution for, like sh_* rules, I pass it from the shell. For crosstool-managed gcc, where I deliver the compiler and library with a repo rule and want to precisely control the search paths, I set it to an empty string. And for other toolchains vendored through starlark rules, I set it to precisely the things that toolchain supplies.

I have been toying with the idea of rewriting the sh_* rules in starlark and creating a "shell toolchain" that vendors and builds bash, coreutils, etc, to remove the need to use system paths for that. But I have not yet been sufficiently motivated to take this seriously, and it would require thinking of a good solution to managing dependencies between toolchains (since half the rules in the world are going to need a working shell). 

[Nicolas Lopez]

2 cents from remote exec toolchains team:

Having a PATH and LD_LIBRARY_PATH set in the remote exec container should well for remote execution.

I agree with most of the discussion in this thread that the strict_action_env might be more of a problem for local execution than a benefit.

I do however think that the alternative suggested (not including the PATH in the action key, even for local execution + remote caching) could be very problematic for remote caching as it sounds like it could lead to cache poisoning. I think the core issue at hand is that some rules (most, perhaps, atm?) are depending on the PATH to function properly, and Bazel has no way of expressing that at a finer granularity than the whole build.

I'd think a more sustainable solution would include mechanisms to allow rules to express that they have a dependency on the PATH (or any other env variables), and so rule owners that mark their rules as needing PATH could do this. If that was possible, the targets for those rules only would include the PATH (or any other env var) in their key. This might result in lower cache hits for those kinds of actions, but it might also encourage rule owners to stop depending on something as coarse as the PATH and move to using toolchain rules to specify env/tool details. Note this is what we have been working towards for rules_docker and rules_k8s with recent work to provide toolchain rules (https://github.com/bazelbuild/rules_docker/tree/master/toolchains/docker, https://github.com/bazelbuild/rules_k8s/tree/master/toolchains/kubectl) 

[Steven Bergsieker]

On Mon, Jan 21, 2019 at 2:26 PM 'Nicolas Lopez' via bazel-discuss <bazel-...@googlegroups.com> wrote:

2 cents from remote exec toolchains team:

Having a PATH and LD_LIBRARY_PATH set in the remote exec container should well for remote execution.

I agree with most of the discussion in this thread that the strict_action_env might be more of a problem for local execution than a benefit.

I do however think that the alternative suggested (not including the PATH in the action key, even for local execution + remote caching) could be very problematic for remote caching as it sounds like it could lead to cache poisoning.

I had missed that this--excluding PATH and LD_LIBRARY_PATH from the remote cache key--was an implication of the original proposal. I agree with Nick about this: it's far too easy to poison the remote cache if the PATH is not included in the cache key. Including it forces users to be mindful of their path (assuming they want to get cache hits across machines), and likely enforce good hygiene by running bazel inside some sort of wrapper that controls the environment (including the PATH). I believe that this is a good thing, and will ultimately lead to happier users over time.

As far as remote execution, it's not necessary to include the PATH or other variables in the Action, and indeed is a good idea to exclude them. Including the path for local execution+remote cache and excluding it for remote execution will make it harder to run hybrid builds where users want to pull cache entries generated by remote execution into their local builds, but such hybrid builds are inherently dangerous and should be be the default behavior.

Note that removing the path from remote execution Actions (rather than setting to a "standard" one like "/bin:/usr/bin") is a breaking change and should be handled as such.

 

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/f4b6b98b-0207-42d9-a395-3d27ffbeae8b%40googlegroups.com.

[Jakob Buchgraber]

On Mon, Jan 21, 2019 at 8:26 PM 'Nicolas Lopez' via bazel-discuss <bazel-...@googlegroups.com> wrote:

I do however think that the alternative suggested (not including the PATH in the action key, even for local execution + remote caching) could be very problematic for remote caching as it sounds like it could lead to cache poisoning.

I'd argue that it only makes cache poisoning more likely on identical environments (i.e. docker containers from the same image) that happen to have a different PATH set. On all other systems it seems equally unsafe to use remote caching for rules that don't fully declare their inputs/dependencies (are not hermetic). Wouldn't you agree?

I think the core issue at hand is that some rules (most, perhaps, atm?) are depending on the PATH to function properly, and Bazel has no way of expressing that at a finer granularity than the whole build.

I'd think a more sustainable solution would include mechanisms to allow rules to express that they have a dependency on the PATH (or any other env variables), and so rule owners that mark their rules as needing PATH could do this. If that was possible, the targets for those rules only would include the PATH (or any other env var) in their key.

I do not understand how tracking the value of the PATH makes poisoning less likely, as tools typically don't have version information in their name / path. I'd argue one must move away from PATH towards hermetic rules.

[jga...@butterflynetinc.com]

Will you be keeping `strict_action_env` for those of us who are happily using it (with local execution and remote caching)?

I agree about moving away from the PATH and towards hermetic rules, though I probably fall on the side of PATH should be (as close to) empty by default, and users should have to explicit about adding tool paths.

[Jakob Buchgraber]

On Tue, Jan 22, 2019 at 3:07 AM <jga...@butterflynetinc.com> wrote:

Will you be keeping `strict_action_env` for those of us who are happily using it (with local execution and remote caching)?

This e-mail thread proposes to remove --strict_action_env altogether. Local execution in remote caching would instead use

the PATH of the shell and not include the PATH in the remote cache key computation. I'd be interested to learn why you would

want to keep using strict_action_env.

[Andrew Suffield]

To answer the same question from a different context: we've been using strict_action_env to purge the environment of some native rules (cc_*) that we've made hermetic.

I don't actually need strict_action_env for this, I'm going to put wrapper scripts around all the tools and purge the environment in those.

I would like an alternative way to do this directly, but I'm currently assuming that I have to wait for starlark rules_cc before that happens.

--

You received this message because you are subscribed to the Google Groups "bazel-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CAGQ4vn0Dhwc-oraUApQ%3Dzp3Bme06ORk677VgQdeeKCuw3fuYWA%40mail.gmail.com.

[jga...@butterflynetinc.com]

Sorry, big thread. I might have missed where you proposed to remove it. You feel it is necessary to take it away? I understand changing the defaults for people not using the flag, but for those who are, you're forcing them to immediately deal with a new PATH.

[Philipp Wollermann]

On Tue 22. Jan 2019 at 19:12, <jga...@butterflynetinc.com> wrote:

Sorry, big thread. I might have missed where you proposed to remove it. You feel it is necessary to take it away? I understand changing the defaults for people not using the flag, but for those who are, you're forcing them to immediately deal with a new PATH.

Is using “—action_env=PATH=“ an option?

That should also give every action an empty PATH by default.

On Tuesday, January 22, 2019 at 11:39:10 AM UTC-5, Jakob Buchgraber wrote:
> On Tue, Jan 22, 2019 at 3:07 AM <jga...@butterflynetinc.com> wrote:
>
> Will you be keeping `strict_action_env` for those of us who are happily using it (with local execution and remote caching)?
>
>
>
> This e-mail thread proposes to remove --strict_action_env altogether. Local execution in remote caching would instead use
> the PATH of the shell and not include the PATH in the remote cache key computation. I'd be interested to learn why you would
> want to keep using strict_action_env.

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/f3ab8698-a51b-43b8-8bf1-a1850736f4db%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Philipp Wollermann | Software Engineer | phi...@google.com

[jga...@butterflynetinc.com]

That might work for us, I'm not sure that would please windows users, as there's some Windows specific 'default path' handling.

https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java#L409

[Jakob Buchgraber]

On Wed, Jan 23, 2019 at 3:59 AM <jga...@butterflynetinc.com> wrote:

That might work for us, I'm not sure that would please windows users, as there's some Windows specific 'default path' handling.

https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/bazel/rules/BazelRuleClassProvider.java#L409

Good points Jason. The last time I talked to +Yun Peng about this he mentioned that the default value we are providing there is not very useful

and that for Windows it's mostly impossible to provide a good default PATH because there are no default install locations (correct me if I misremember Yun).

The worry I have with continuing to provide --strict_action_env is the false sense of correctness / strictness / hermeticity that it puts across.

If --strict_action_env would live up to its name it should set the PATH to the empty string. Either way, think we might be able to solve this problem

partly with a dedicated documentation section on our website to clearly explain the issues that PATH brings when used with remote caching. Wdyt?

[jga...@butterflynetinc.com]

I trust your judgement...I think that's a good plan. It might force some work but that's a good thing.

[Ulf Adams]

Would you mind putting the proposal in a doc? I think it's clear we need a better default setting, and what you're proposing sounds like it's feasible. However, I'm not sure that we're sufficiently covering all the current use cases yet, as brought up on this thread. I think it would be good to have a 'completely hermetic' option and maybe we need an additional mechanism to set env variables in a platform-specific way, but in a centralized location?

--
You received this message because you are subscribed to the Google Groups "bazel-discuss" group.
To unsubscribe from this group and stop receiving emails from it, send an email to bazel-discus...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/5a89b0eb-11a0-46d6-a8c8-0beebf2073db%40googlegroups.com.

[Jakob Buchgraber]

Yes of course. I ll do so after my leave (2-3 weeks).

Jakob Buchgraber

Software Engineer

Google Germany GmbH

Erika-Mann-Straße 33

80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Diese E-Mail ist vertraulich. Falls sie diese fälschlicherweise erhalten haben sollten, leiten Sie diese bitte nicht an jemand anderes weiter, löschen Sie alle Kopien und Anhänge davon und lassen Sie mich bitte wissen, dass die E-Mail an die falsche Person gesendet wurde.

    

This e-mail is confidential. If you received this communication by mistake, please don't forward it to anyone else, please erase all copies and attachments, and please let me know that it has gone to the wrong person.

To view this discussion on the web visit https://groups.google.com/d/msgid/bazel-discuss/CALS-RZLqKiNGCL1%3DZmQoxuCeBPvmbd4Pm1FccD%2B_sFHNv8%3D2gQ%40mail.gmail.com.

[m...@tweag.io]

Hi,

On Fri, Feb 1, 2019 at 11:01 PM 'Ulf Adams' via bazel-discuss

> Would you mind putting the proposal in a doc?

On Tuesday, 5 February 2019 11:41:10 UTC+1, Jakob Buchgraber wrote:
> Yes of course. I ll do so after my leave (2-3 weeks).

Did this happen?

Thanks,

Mathieu

[Jakob Buchgraber]

On Sun, Apr 7, 2019 at 9:49 PM <m...@tweag.io> wrote:

> Yes of course. I ll do so after my leave (2-3 weeks).

Did this happen?

Ohh estimates :-). I did not happen yet. I am currently (this week) looking into this.

[Boespflug, Mathieu]

Cool. :)

I'll be interested to read about the way forward. I'm currently resorting to wrappers to control the content of the environment variables that Bazel sees, because this is otherwise having adverse effects on not just result caching with RBE but also build reproducibility when the content of PATH is particularly esoteric.

Best,

--
Mathieu Boespflug
Founder at http://tweag.io.