search

hedronvision / bazel-compile-commands-extractor

Goal: Enable awesome tooling for Bazel users of the C language family.
Other
697 stars 115 forks source link

Add support for user-specified hermetic Python py_binary #139

Closed tervay-bdai closed 10 months ago

tervay-bdai commented 1 year ago

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.

cpsauer commented 11 months ago

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 commented 10 months ago

^ Ended up switching to hermetic python in https://github.com/hedronvision/bazel-compile-commands-extractor/commit/0e5b1aa26d87a431d2a52676d0b9ce469448ee54, so I think I should close. If you still have issues, please holler!

cpsauer commented 10 months ago

Hey Justin, we had to revert hermetic rules_python in https://github.com/hedronvision/bazel-compile-commands-extractor/commit/0b821b7e4286aec887757461366f6eaaa0972cb9, because there were lots of issues :( Tracking restoration in https://github.com/hedronvision/bazel-compile-commands-extractor/issues/168.

LMK if you still need this. I'm wondering also if this is a matter of transition configuration? Please see https://github.com/hedronvision/bazel-compile-commands-extractor/issues/164#issuecomment-1913052639, which is the same, I think, even when not using system python.