-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Allow custom stub template for py_binary's #137
Comments
There is no such attribute unfortunately. Shouldn't be too hard to add. Adding an attribute to: And use it to load an external template if not specified there: |
My two cents: in this case I think updating the existing py_binary rules is better than implementing a custom one. The feature sounds reasonable and could be useful to others too. |
My team could use this as well - we have been wanting to tweak the python stub to exclude local system site-packages by adding |
I took @damienmg's suggestions and added a |
Thanks a lot for the PR Fahrzin! It doesn't surprise me that people would want to customize the stub script, but I do wonder what all the use cases are and whether they can be achieved with a more narrow parameterization. For instance, we could add a place to insert custom PYTHONPATHs and custom interpreter flags. I have two main concerns with this proposal. First, it seems to me that stub customization should be per-interpreter, rather than per-binary. For example, if the same binary is built on different developer machines that have different system interpreters (which is possible in the future via a This could be implemented by adding the attribute to My second concern is that if we expose the template, it becomes an API that we have to worry about breaking. In particular, we need to worry about the availability and meaning of the substitution placeholders. One way around this might be to guard the feature with an experimental flag. In any case, I think this raises enough questions that I should put the above points into a design doc. |
Please consider also that py_* rules on Windows do not use a stub script but a pre-built binary preamble. I suggest pointing out this difference in documentation, i.e. customizing the stub script would have no effect on Windows. |
@kwatts, @benley, @fahhem: What are the exact kinds of customization people want for the stub template? It's hard for me to imagine that users should be able to manipulate the runfiles logic for instance. I understand |
@laszlocsomor: Does that mean the windows-specific logic in python_stub_template.txt is dead code and can be removed? |
Sorry, forget almost everything I said, I got confused. I got confused over the fact that for Java we also have a stub script, which is a Bash script, and its logic is fully implemented in the launcher. (We use the same launcher preamble for Python and Java binaries, but with different launch parameters, so one starts python, the other starts the JVM.) |
Thanks for the clarification. So if we go ahead with this, I won't distinguish the case of windows vs unix. |
Please see design doc posted here, and in particular the open questions. |
Yes, I can confirm what @laszlocsomor said, we do use the stub template on Windows. The native python launcher is just used to run the stub template. |
My current use case is that I'd like to insert code that runs in the stub for a certain subset of rules, such as enabling (and extracting) coverage instrumentation for Other use cases in the past that would have been easier with this:
|
I'm aware of two common use cases that require changes to the
|
Any update here @brandjon? I can update my PR again if we're okay allowing custom stubs on the If you want to pull the stub from the toolchain, then I'm happy to switch to that tactic if I have a chance of actually getting it merged in. |
Fixes bazelbuild#137, but unlike my approach in bazelbuild#6632, this adds an attribute to `py_runtime` rather than to `py_binary` Open to suggestions on the attribute name and documentation
Fixes bazelbuild#137, but unlike my approach in bazelbuild#6632, this adds an attribute to `py_runtime` rather than to `py_binary` Open to suggestions on the attribute name and documentation
Fixes bazelbuild#137, but unlike my approach in bazelbuild#6632, this adds an attribute to `py_runtime` rather than to `py_binary`
Fixes bazelbuild#137, but unlike my approach in bazelbuild#6632, this adds an attribute to `py_runtime` rather than to `py_binary` Open to suggestions on the attribute name and documentation
FYI, I've put out a new PR with changes to the runtime (that I think is close to getting in, but @rickeylev decides that): #16806 Once that's in, a new attribute |
Is there something I can do to help push #16806 forwards? Looks like there's an outstanding review but it's pretty close |
Not other than making the changes necessary :D I'm coming back to this now so I hope it's ready this week |
Fixes bazelbuild#137, but unlike my approach in bazelbuild#6632, this adds an attribute to `py_runtime` rather than to `py_binary` Open to suggestions on the attribute name and documentation
I'm not working on Python rules anymore. Ccing @rickeylev for any decisions regarding prioritization of this. |
That PR is approved and just needs to be imported. I got back from vacation today; it's one of changes I'll be looking at first. |
Fixes bazelbuild#137, but unlike my approach in bazelbuild#6632, this adds an attribute to `py_runtime` rather than to `py_binary` Open to suggestions on the attribute name and documentation
Fixes bazelbuild#137, but unlike my approach in bazelbuild#6632, this adds an attribute to `py_runtime` rather than to `py_binary` Open to suggestions on the attribute name and documentation
Fixes bazelbuild#137, but unlike my approach in bazelbuild#6632, this adds an attribute to `py_runtime` rather than to `py_binary` Open to suggestions on the attribute name and documentation Patched cc_shared_library to fix cc shared library transitive dependencies taking in upb protos Edit bootstrap template. Windows related updates. Use our stub template directly. Remove host version warning emission. Update builtins_bzl with better support for cc_shared_library's Transitive debug and run files. Repair transitive debug files. Some more fixes. Update python_bootstrap_template.txt Fix error prone ComparisonOutOfRange in PersistentStringIndexer PiperOrigin-RevId: 544294153 Change-Id: Ia89d02a025dfafa213d68ea2110a79e5adccf79c fix changed var name
Feature request: My team would like to use a custom stub template for our py_binary executables (setting up a customization points, etc). Is there any way to do this with Bazel, or should we write a "custom_py_binary" rule?
The text was updated successfully, but these errors were encountered: