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

CI fails with Python toolchains (Bazel 0.27) #809

Closed
brandjon opened this issue May 31, 2019 · 13 comments
Closed

CI fails with Python toolchains (Bazel 0.27) #809

brandjon opened this issue May 31, 2019 · 13 comments

Comments

@brandjon
Copy link
Contributor

Bazel 0.27 enables --incompatible_use_python_toolchains. This breaks rules_nodejs's BuildKite pipeline.

The error message indicates that this is because you have Python 3 targets, and the autodetecting toolchain can't find the python3 command. Our CI workers have python3 installed under /usr/local/bin, but the PATH is apparently .:/bin:/usr/bin.

This is likely because you have --incompatible_strict_action_env in your bazelrc. I've filed bazelbuild/bazel#8536 to track this problem. In the meantime, you may be able to work around this by setting --action_env in your bazelrc.

@brandjon
Copy link
Contributor Author

This problem does not reproduce in the most recent runs due to an unrelated breakage.

@brandjon
Copy link
Contributor Author

I've confirmed that in the case of a py_test (but not necessarily py_binary!), the action environment (--action_env) is respected. The error indicates that the PATH is

.:/bin:/usr/bin

The /bin:/usr/bin part comes from the default value for PATH under a strict action env on a non-windows platform. The preceding . part comes from test-setup.sh.

It seems that a solution would be to add --action_env=PATH to the bazelrc. I've confirmed this on a mac worker with an image similar to our CI runners. The alternative requires adding an explicit Python toolchain that knows how to find Python 3 on mac. It's unclear whether that kind of toolchain definition belongs in rules_nodejs or our CI workers.

@brandjon
Copy link
Contributor Author

Ping @alexeagle for visibility, as this affects the 0.27 Bazel release candidate.

@alexeagle
Copy link
Collaborator

The fix here is to remove any python use from this repo. We have a JS toolchain and should use only that.

@brandjon
Copy link
Contributor Author

In the meantime, --action_env=PATH will turn CI green in Bazel's downstream presubmit.

@laurentlb
Copy link
Contributor

Can you please set --action_env=PATH to turn the CI green?
rules_nodejs has been failing for a week already

@alexeagle
Copy link
Collaborator

looking at it now. The link to the build breakage you gave above takes me to a BuildKite page where everything is green. I tried to reproduce with bazel test --incompatible_use_python_toolchains //internal/common:check_version_test (on Linux) but it still passes.

Our one python test is for a starlark helper. I'll just comment out that test for now to unblock the release.

(I don't want to put the $PATH in the action_env because it makes our action keys unstable when run under another tool that puts unique TMPDIR in the path for every run, totally breaking build incrementality.)

I would love some help to get the starlark unit test working. I'm getting
ERROR: While resolving toolchains for target //internal/common:check_version_tests_test_0: no matching toolchains found for types @bazel_skylib//toolchains/unittest:toolchain_type

@brandjon
Copy link
Contributor Author

brandjon commented Jun 11, 2019

The breakage only occurs on Mac because the default PATH for a strict action env is /bin:/usr/bin, which includes the location of python3 on Linux but not Mac.

The CI run I linked to is a pipeline that reports breakages with upcoming flags in the log output, not by turning red. You can see a red pipeline here, example run here.

If you don't want to use --action_env=PATH, you can also give a specific value for PATH that includes the python3 command on Mac (if installed), i.e. --action_env=PATH=/bin:/usr/bin:/usr/local/bin. Disabling the test isn't ideal but should be acceptable from a downstream impact POV, since downstream users should have their own toolchains / build flags anyway. A proper fix that doesn't involve finagling with PATH would be to use an explicit Python toolchain, but that's typically done at the level of the end user. In the future we could add some support for autodetecting an interpreter without relying on PATH at execution time (bazelbuild/bazel#8603).

@brandjon
Copy link
Contributor Author

+@c-parsons about the starlark unit test toolchain issue.

@brandjon
Copy link
Contributor Author

Also as mentioned offline: Previously the failing test would have used the system python command to run, which is Python 2 on most systems including Mac. So your test is Python 2 compatible. You can therefore change the py_test target to declare itself as Python 2 by setting the attribute python_version = "PY2" (the default is PY3). That may sidestep this whole discussion about action env and toolchains.

@c-parsons
Copy link

Please see the README for https://github.com/bazelbuild/bazel-skylib , particularly the following note:

If you want to use lib/unittest.bzl from Skylib versions released in or after
December 2018, then you also should add to the WORKSPACE file:

load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace")

bazel_skylib_workspace()

Hopefully that fixes things :)

@alexeagle
Copy link
Collaborator

I think the 0.27 release should be unblocked, we added build --incompatible_use_python_toolchains to .bazelrc

@brandjon
Copy link
Contributor Author

Confirmed that the (renamed) test target passes locally on Mac. If for whatever reason it's still a problem on CI I'll respond here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants