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

Add support for user-specified hermetic Python py_binary #139

Closed
wants to merge 1 commit into from

Conversation

tervay-bdai
Copy link

We are using pybind11_bazel and a hermetic Python interpreter in our system. When running refresh_compile_commands, we often get this error:

In file included from <snip>.cc:3:
In file included from external/pybind11/include/pybind11/pybind11.h:13:
In file included from external/pybind11/include/pybind11/detail/class.h:12:
In file included from external/pybind11/include/pybind11/detail/../attr.h:13:
external/pybind11/include/pybind11/detail/common.h:269:6: error: "PYTHON < 3.6 IS UNSUPPORTED. pybind11 v2.9 was the last to support Python 2 and 3.5."
#    error "PYTHON < 3.6 IS UNSUPPORTED. pybind11 v2.9 was the last to support Python 2 and 3.5."
     ^
1 error generated.

This is because refresh_compile_commands.bzl is using native.py_binary, which defaults to the user's system interpreter, which may or may not be the same version as the hermetic interpreter we're using alongside pybind.

This PR adds an optional argument to refresh_compile_commands(), named py_binary_rule, which defaults to native.py_binary. The user can override it and run the main script with whichever Python version they want, which fixes the above pybind errors.

@tervay-bdai tervay-bdai changed the title Add support for hermetic Python py_binary Add support for user-specified hermetic Python py_binary Aug 29, 2023
@cpsauer
Copy link
Contributor

cpsauer commented Dec 20, 2023

I'm really sorry I missed this, Justin. I really value your being excited to contribute, and I'd like to get you back on the mainline.

Happy to merge as is if you think, but I wanted to see if you thought it'd be better if we just took a dependency on rules_python and pull in hermetic Python ourselves, rather than export the complexity to the user?

There's also something I'm not understanding: Why is it that this tool's use of system Python causes the pybind11 error? I'd have thought offhand that we'd call through to however you were using it. (I want to make sure also that I shouldn't be recommending you use case (3) in the readme to be getting your top level Python configuration rather than :refresh_all.)

@cpsauer
Copy link
Contributor

cpsauer commented Jan 6, 2024

^ Ended up switching to hermetic python in 0e5b1aa, so I think I should close. If you still have issues, please holler!

@cpsauer
Copy link
Contributor

cpsauer commented Feb 1, 2024

Hey Justin, we had to revert hermetic rules_python in 0b821b7, because there were lots of issues :( Tracking restoration in #168.

LMK if you still need this. I'm wondering also if this is a matter of transition configuration? Please see #164 (comment), which is the same, I think, even when not using system python.

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

Successfully merging this pull request may close these issues.

2 participants