bazel run of py_binary target uses client PATH

[keith]

Description of the problem:

Currently when setting up a py_runtime, any script you have called for the interpreter isn't called with any PATH, and ends up defaulting to the shell's default PATH, which can lead to using the wrong binaries depending on your system's configuration.

This was discovered while debugging #8414

In our case we exec bazel with env -i PATH=/usr/bin:/bin and pass --action_env=PATH=/usr/bin:/bin because we don't want user configuration to leak into the build. Because of this issue, you can see the error message in the linked issue is:

Failure reason: Cannot locate 'python3' or 'python' on the target platform's PATH, which is:
/usr/gnu/bin:/usr/local/bin:/bin:/usr/bin:.

Where this default path (on macOS) appears to be coming from bash's default path (where /bin/sh on macOS is actually just bash), which is defined as:

/* The default value of the PATH variable. */
#ifndef DEFAULT_PATH_VALUE
#define DEFAULT_PATH_VALUE \
  "/usr/gnu/bin:/usr/local/bin:/bin:/usr/bin:."
#endif

Not being called with a PATH is what causes #8414 (although because of this default PATH you can export PATH to fix the specific issues there).

This issue cascades to this script as well https://github.com/bazelbuild/bazel/blob/b7a961517f928e6539b62f73bcb5b1c311396559/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt that asks for /usr/bin/env python, which has to be pulled from the default PATH, which could result in a homebrew installed python (on macOS) instead of the system one.

It also attempts to read the PATH environment variable directly

bazel/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt

Line 46 in b7a9615

search_path = os.getenv('PATH', os.defpath).split(os.pathsep)

which would never work in this case.

I believe the interpreter set for py_runtime should inherit the --action_env=PATH value set, if it is, and otherwise bazel should be calling these with a default PATH, and not falling back on the shell to do this itself.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

Make the auto detection wrapper script echo $PATH and exit 1 https://github.com/bazelbuild/bazel/blob/1193e696b08f0932e1430c87dcfb51fe6c18d7e9/tools/python/pywrapper_template.txt and try to build a python target

What operating system are you running Bazel on?

macOS

What's the output of bazel info release?

development version

If bazel info release returns "development version" or "(@Non-Git)", tell us how you built Bazel.

bazel build src:bazel

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

https://github.com/bazelbuild/bazel
1193e69
1193e69

Have you found anything relevant by searching the web?

Related to #8414

[brandjon]

#7026 might be relevant to this. Not sure.

that asks for /usr/bin/env python, which has to be pulled from the default PATH, which could result in a homebrew installed python (on macOS) instead of the system one.

The stub script should be able to run on any reasonable modern system Python interpreter. If it can't we'd probably consider that a bug. Alternatively, we could add a feature to customize the shebang, but I'd like to avoid that for now if possible.

The problems with how the environment is setup for an in-workspace py_runtime may have to do with how the rule is designed. IIRC, the way the interpreter attribute works is that it expects to get an executable file target, as opposed to an arbitrary executable target that has an associated set of runfiles. So the code path for invoking that executable might be different from the standard way that propagates the action env.

Edit: For example, it might be that we're adding a bunch of artifacts instead of using something like Runfiles.Builder#addTarget.

Edit edit: And sure enough, here's my previous writeup of what I was thinking of, which may or may not be the cause of this issue.

[brandjon]

After doing more experimentation for #8536, it appears that the action environment, whether set by --action_env or by --incompatible_strict_action_env, does not override the client's environment when using bazel run to launch a py_binary target. However, it does override it for a py_test target (whether using bazel run or bazel test). It also overrides it for genrule actions, e.g. when using a py_binary as a tool.

So I think the question is whether action environments should override the client environment for executable artifacts that are not being used as tools for other build steps. I can make an argument that they shouldn't: For instance, you wouldn't be able to write a command line tool that inspects the current environment and launch that tool with bazel run. Then again, you could counter-argue that bazel run should be hermetic, and that users should directly launch the resulting artifact if they want the client env to be read.

In any case, to the extent that this interferes with a py_binary's ability to locate a Python interpreter, the answer is 1) to avoid depending on /usr/bin/env python to launch the stub script (#8446), and 2) to use an explicit toolchain appropriate for the target platform, in place of the default autodetecting toolchain.

[brandjon]

I can confirm that this same environment behavior is present for the java rules: --action_env and --incompatible_strict_action_env affect java_binary when it's used as a tool, and java_test when it's the end-target, but not java_binary when it's the end-target.

So the real issue here is how Python artifacts depend on the PATH, and not the fact that PATH is propagated in different ways depending on what kind of target it is and how it's used.

[keith]

@brandjon any advice on how to go about fixing this? We had our first case of this actually causing issues today where because of this /usr/local/bin PATH someone who manually created a python there that was python3 resulted in their build failing because we needed it to be python 2

[keith]

Turns out this issue affects more than just py #12049

[jmmv]

OK, then let's close this as a duplicate of #12049.