Let Python stubs respect RUNFILES_* while finding the module space#14740
Closed
EdSchouten wants to merge 1 commit intobazelbuild:masterfrom
Closed
Let Python stubs respect RUNFILES_* while finding the module space#14740EdSchouten wants to merge 1 commit intobazelbuild:masterfrom
EdSchouten wants to merge 1 commit intobazelbuild:masterfrom
Conversation
0aa9bb0 to
538e2e0
Compare
EdSchouten
commented
Feb 16, 2022
src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt
Outdated
Show resolved
Hide resolved
Contributor
Author
|
Friendly ping! |
Contributor
Author
|
Hi @brandjon; happen to have a couple of spare cycles to look into this? Thanks! |
538e2e0 to
868279b
Compare
Contributor
Author
|
Hey @comius, would you be interested in reviewing this change for me? Thanks! |
Contributor
Author
|
Hmmm... The CI failures for "buildkite/bazel-bazel-github-presubmit/darwin-openjdk-11-xcode-shard-*" seem to be caused by infrastructure failure. The underlying system ran out of disk space? |
868279b to
af4d5e5
Compare
af4d5e5 to
e59eb0d
Compare
rickeylev
reviewed
Sep 20, 2022
src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt
Outdated
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt
Outdated
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt
Outdated
Show resolved
Hide resolved
src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt
Outdated
Show resolved
Hide resolved
e59eb0d to
9b00616
Compare
Contributor
Author
|
Hi @rickeylev, I think I've addressed all of your comments. PTAL! |
9b00616 to
73c2f4c
Compare
When invoking a py_binary() through an sh_binary() using the Bash runfiles library, the location of the py_binary() will be resolved from the runfiles manifest file. This means that the argv[0] of the Python stub script will not point to a location under the runfiles directory of the shell script, but to a location in Bazel's execroot. Normally this does not lead to any issues, as argv[0] + ".runfiles" happens to point to be a valid runfiles directory as well. It does become problematic when --nobuild_runfile_links is provided, as in that case only the outer shell script is guaranteed to have a runfiles directory; not the inner Python script. This change extends the Python stub template to also consider RUNFILES_DIR when no runfiles directory can be found. Even though it's not technically correct, we also attempt to derive the runfiles directory from RUNFILES_MANIFEST_FILE. I suspect that this is a necessity as long as py_binary()s cannot operate purely using a manifest file. They currently depend on a concrete instantiation of the runfiles directory. Fixes: bazelbuild#11997
73c2f4c to
86521ab
Compare
rickeylev
approved these changes
Sep 26, 2022
Contributor
|
Ok, I think I set the labels up correctly here for the "import it into google" people to process it. If you don't hear back in a day that the import process has begun, tag me and I'll check it didn't fall into some void |
Contributor
Author
|
Hey @rickeylev, just tagging you because two days have passed. :D |
Member
|
Hello @EdSchouten, Thank you for the patience. I will update once it gets merged. |
Contributor
|
Just to give an update: I've been fighting with our internal windows test environment to make the test added to python_stub_test.sh pass for the last day or so. For whatever reason, it seems to be missing a lot of the supporting code/config that the github environment has. If I don't have it figured out by Monday, I'll just disable this test in our internal builds for now. |
Contributor
|
Woo, got everything happy! Code is out for review internally; should land early next week. |
aiuto
pushed a commit
to aiuto/bazel
that referenced
this pull request
Oct 12, 2022
When invoking a py_binary() through an sh_binary() using the Bash runfiles library, the location of the py_binary() will be resolved from the runfiles manifest file. This means that the argv[0] of the Python stub script will not point to a location under the runfiles directory of the shell script, but to a location in Bazel's execroot. Normally this does not lead to any issues, as argv[0] + ".runfiles" happens to point to be a valid runfiles directory as well. It does become problematic when --nobuild_runfile_links is provided, as in that case only the outer shell script is guaranteed to have a runfiles directory; not the inner Python script. This change extends the Python stub template to also consider RUNFILES_DIR when no runfiles directory can be found. Even though it's not technically correct, we also attempt to derive the runfiles directory from RUNFILES_MANIFEST_FILE. I suspect that this is a necessity as long as py_binary()s cannot operate purely using a manifest file. They currently depend on a concrete instantiation of the runfiles directory. Fixes: bazelbuild#11997 Closes bazelbuild#14740. PiperOrigin-RevId: 478857199 Change-Id: I8cc6ea014bfd4b9ea2f1672e8e814ba38a5bf471
github-merge-queue bot
pushed a commit
to bazel-contrib/rules_python
that referenced
this pull request
Sep 26, 2023
Pass the environment generated by the `runfiles` helper so that the Python launcher can find the runfiles correctly as implemented in bazelbuild/bazel#14740. Fixes #1426
github-merge-queue bot
pushed a commit
to bazel-contrib/rules_python
that referenced
this pull request
Sep 26, 2023
Pass the environment generated by the `runfiles` helper so that the Python launcher can find the runfiles correctly as implemented in bazelbuild/bazel#14740. Fixes #1426
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When invoking a py_binary() through an sh_binary() using the Bash
runfiles library, the location of the py_binary() will be resolved from
the runfiles manifest file. This means that the argv[0] of the Python
stub script will not point to a location under the runfiles directory of
the shell script, but to a location in Bazel's execroot.
Normally this does not lead to any issues, as argv[0] + ".runfiles"
happens to point to be a valid runfiles directory as well. It does
become problematic when --nobuild_runfile_links is provided, as in that
case only the outer shell script is guaranteed to have a runfiles
directory; not the inner Python script.
This change extends the Python stub template to also consider
RUNFILES_DIR when no runfiles directory can be found. Even though it's
not technically correct, we also attempt to derive the runfiles
directory from RUNFILES_MANIFEST_FILE. I suspect that this is a
necessity as long as py_binary()s cannot operate purely using a manifest
file. They currently depend on a concrete instantiation of the runfiles
directory.
Fixes: #11997