Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build failure with --incompatible_use_python_toolchains #8414

Closed
keith opened this issue May 21, 2019 · 6 comments
Closed

Build failure with --incompatible_use_python_toolchains #8414

keith opened this issue May 21, 2019 · 6 comments
Assignees
Labels
P1 I'll work on this now. (Assignee required) team-Rules-Python Native rules for Python type: bug

Comments

@keith
Copy link
Member

keith commented May 21, 2019

Description of the problem / feature request:

When using --incompatible_use_python_toolchains on macOS, the build fails because it can't find any pythons:

% bazel build //examples/... --incompatible_use_python_toolchains
Starting local Bazel server and connecting to it...
INFO: Analyzed 174 targets (64 packages loaded, 2442 targets configured).
INFO: Found 174 targets...
ERROR: /Users/ksmiley/dev/rules_apple/examples/multi_platform/Buttons/BUILD:72:1: StoryboardCompile examples/multi_platform/Buttons/Buttons-intermediates/Base.lproj_storyboardc/Main.storyboardc failed (Exit 1)
which: no python3 in ((null))
which: no python in ((null))
Error: The default python toolchain (@bazel_tools//tools/python:autodetecting_toolchain) was unable to locate a suitable Python interpreter on the target platform at execution time. Please register an appropriate Python toolchain. See the documentation for py_runtime_pair here:
https://github.com/bazelbuild/bazel/blob/master/tools/python/toolchain.bzl.
Failure reason: Cannot locate 'python3' or 'python' on the target platform's PATH, which is:
/usr/gnu/bin:/usr/local/bin:/bin:/usr/bin:.
INFO: Elapsed time: 8.957s, Critical Path: 0.28s
INFO: 1 process: 1 darwin-sandbox.
FAILED: Build did NOT complete successfully

I believe that the issue here is that the calls to which in this script aren't receiving the PATH environment variable. I found 2 workarounds:

  1. Add export PATH above the first which call
  2. Switch to command -v instead of which, which is preferred since it is part of the POSIX standard where which is not https://github.com/koalaman/shellcheck/wiki/SC2230

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

In rules_apple run:

bazel build //examples/... --incompatible_use_python_toolchains

What operating system are you running Bazel on?

macOS

What's the output of bazel info release?

release 0.25.0

Have you found anything relevant by searching the web?

Nothing helpful on #7899

keith added a commit to keith/bazel that referenced this issue May 21, 2019
This fixes an issue on macOS where `which` failed to return any results
because the `PATH` wasn't propagated. `command` is more standard for
this use case as well https://github.com/koalaman/shellcheck/wiki/SC2230

Fixes bazelbuild#8414
@brandjon
Copy link
Member

Very nice, thanks for the diagnosis!

I actually have been debugging this very issue in bazelbuild/continuous-integration#578. I have an in-flight PR to fix it that uses the export PATH approach because I didn't know about command -v. But not only is your way more direct, it also avoids contaminating the environment for the payload Python program: Bazel intentionally (I can only presume) unsets PATH when invoking a tool as part of ctx.actions.run, so we shouldn't propagate the shell's default PATH in its place.

I'll go ahead with my commit since it includes a regression test, then merge yours on top with minor modifications.

@brandjon brandjon added P1 I'll work on this now. (Assignee required) and removed untriaged labels May 21, 2019
@brandjon brandjon self-assigned this May 21, 2019
keith added a commit to keith/bazel that referenced this issue May 21, 2019
This fixes an issue on macOS where `which` failed to return any results
because the `PATH` wasn't propagated. `command` is more standard for
this use case as well https://github.com/koalaman/shellcheck/wiki/SC2230

Fixes bazelbuild#8414
keith added a commit to keith/bazel that referenced this issue May 21, 2019
This fixes an issue on macOS where `which` failed to return any results
because the `PATH` wasn't propagated. `command` is more standard for
this use case as well https://github.com/koalaman/shellcheck/wiki/SC2230

Fixes bazelbuild#8414
@keith
Copy link
Member Author

keith commented May 21, 2019

Fixed by 7f49531 as well

@brandjon
Copy link
Member

FYI I am doubting my understanding of PATH being unset a little bit. If PATH really is unset by Bazel when executing a py_binary as a tool, why does the stub script shebang #!/usr/bin/env python work? Yet if Bazel weren't unsetting PATH, what could be causing the user Python code to see that it's missing from the environment immediately after the exec from the pywrapper script?

@keith
Copy link
Member Author

keith commented May 21, 2019

@brandjon I think that works because it ends up falling back to the default PATH set by the shell, and that happens to work because it likely includes the system python install in /usr/bin

@keith
Copy link
Member Author

keith commented May 21, 2019

I think I've verified this with a simple script:

% unset PATH
% ./foo.sh
which: no python in ((null))
/usr/gnu/bin:/usr/local/bin:/bin:/usr/bin:.

Where the script is:

#!/usr/bin/env bash

which python
echo "$PATH"

Or with a python script:

#!/usr/bin/env python

import sys
print sys.executable

import os
print os.__file__

The first print here has nothing, but the second makes it clear it's using the macOS system python, not the ones earlier in my real PATH

@brandjon
Copy link
Member

brandjon commented May 21, 2019

I had been assuming it was the shell itself that detected when PATH was unset, and gave it a default value. But I suppose that can't be the case if the shebang has already been processed by that point. My confusion was over the fact that the stub script is a Python script, not a bash script, but again that doesn't matter before the shebang is processed.

So I wonder what it is that's initializing that environment. The OS itself (kernel + standard c library), as the user's default PATH, I suppose?

aehlig pushed a commit that referenced this issue May 23, 2019
This is a modification of PR #8415, which changed `which` to `command -v` so it works when PATH isn't exported. `command` is more standard for this kind of use case (https://github.com/koalaman/shellcheck/wiki/SC2230). Unfortunately, `command -v` doesn't check the executable bit, so it's not as useful for us.

The previous fix for this issue (7f49531) was based on exporting PATH, but this changes the environment seen by the exec'd payload Python code.

The solution in this commit is to not export PATH but rather explicitly pass it to each invocation of `which`.

Fixes #8414. See also bazelbuild/continuous-integration#578. Closes #8415.

PiperOrigin-RevId: 249334274
aehlig pushed a commit that referenced this issue May 23, 2019
This is a modification of PR #8415, which changed `which` to `command -v` so it works when PATH isn't exported. `command` is more standard for this kind of use case (https://github.com/koalaman/shellcheck/wiki/SC2230). Unfortunately, `command -v` doesn't check the executable bit, so it's not as useful for us.

The previous fix for this issue (7f49531) was based on exporting PATH, but this changes the environment seen by the exec'd payload Python code.

The solution in this commit is to not export PATH but rather explicitly pass it to each invocation of `which`.

Fixes #8414. See also bazelbuild/continuous-integration#578. Closes #8415.

PiperOrigin-RevId: 249334274
aehlig pushed a commit that referenced this issue May 24, 2019
This is a modification of PR #8415, which changed `which` to `command -v` so it works when PATH isn't exported. `command` is more standard for this kind of use case (https://github.com/koalaman/shellcheck/wiki/SC2230). Unfortunately, `command -v` doesn't check the executable bit, so it's not as useful for us.

The previous fix for this issue (7f49531) was based on exporting PATH, but this changes the environment seen by the exec'd payload Python code.

The solution in this commit is to not export PATH but rather explicitly pass it to each invocation of `which`.

Fixes #8414. See also bazelbuild/continuous-integration#578. Closes #8415.

PiperOrigin-RevId: 249334274
irengrig pushed a commit to irengrig/bazel that referenced this issue Jun 18, 2019
This is a modification of PR bazelbuild#8415, which changed `which` to `command -v` so it works when PATH isn't exported. `command` is more standard for this kind of use case (https://github.com/koalaman/shellcheck/wiki/SC2230). Unfortunately, `command -v` doesn't check the executable bit, so it's not as useful for us.

The previous fix for this issue (bazelbuild@7f49531) was based on exporting PATH, but this changes the environment seen by the exec'd payload Python code.

The solution in this commit is to not export PATH but rather explicitly pass it to each invocation of `which`.

Fixes bazelbuild#8414. See also bazelbuild/continuous-integration#578. Closes bazelbuild#8415.

PiperOrigin-RevId: 249334274
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 I'll work on this now. (Assignee required) team-Rules-Python Native rules for Python type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants