-
Notifications
You must be signed in to change notification settings - Fork 559
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
rules_python failing with Bazel@HEAD in Bazel Downstream pipeline #856
rules_python failing with Bazel@HEAD in Bazel Downstream pipeline #856
Comments
/cc @alexeagle @f0rmiga |
gently ping~ |
Sorry, my son was born the next day you posted this, and I didn't have enough time to dig into it yet. From a first pass, the failure is coming from the fact that I expanded the CI jobs and we were restricting the Bazel versions we tested against in the integration tests. I think we should fix this forward, as CI is more correct now. |
If this is blocking anything, we can at least disable them while the proper fix is not done. |
congrats @f0rmiga! |
Oh, congratualtions! @f0rmiga Maybe someone else can look into this in the meantime? /cc @rickeylev It's not block anything, but it's causing the Bazel downstream pipeline to be red and could shadow any other breakage we could detect for rules_python at Bazel@HEAD. |
Congratulations and a friendly ping :) |
A friendly pong. |
I think this is fixed, though I don't know what did so. Running the following succeeds:
I'll send a PR to re-enable our tests in the Bazel config |
Ok, turns out its not, insofar as the CI is concerned:
The only clue in there is something about not being able to copy bazel from one temp location to another? I'll have to dig into it a bit. |
Ok, after digging a bit, I found the cause: The bazel CI sets We might be able to rewrite these tests to not use a subprocess, though. I'm thinking we could use the multi-version python rules to do the same -- all these "acceptance tests" seem to be doing is running Python and checking the version output. |
note to self: another idea to investigate: a genrule that sets toolchains=. Then run $(PYTHON) --version and check the output (or some equiv of this) |
…ine. The latest Bazel build continous integration testing pipeline sets several flags and environment variables that end up interfering with each other: * `--sandbox_tmpfs_path=/tmp` * `--test_env=USE_BAZEL_VERSION` * `USE_BAZEL_VERSION=/tmp/<something>` * And Bazelisk is used to run Bazel What happens is `USE_BAZEL_VERSION` points to Bazel in /tmp, but then the `--sandbox_tmpfs_path` flag prevents it from being readable. Later, when a test wants to run Bazel, Bazelisk is invoked. It is able to see that it should use a custom Bazel binary because of `--test_env`, but then can't read the file because of `--sandbox_tmpfs_path`, so then fails. To fix, make the test runner that will run `bazel` unset `USE_BAZEL_VERSION` so Bazelisk doesn't try to use it. This also exposed an issue with Bazelisk demanding a cache directory be specified, so set that environment variable to the test's temp dir to keep Bazelisk happy. Fixes #856
https://buildkite.com/bazel/bazel-at-head-plus-downstream/builds/2675#0183d0a7-84bd-4541-b543-01d162c313ed
Possible culprits are: #855 and #851
The text was updated successfully, but these errors were encountered: