Skip to content

Conversation

jsharpe
Copy link

@jsharpe jsharpe commented Jul 22, 2025

Make the python sdk targets configurable so that they can be overriden with e.g.@python_3_12//:python_headers to avoid needing to transition the current python toolchain to another version.

Fixes:

@jsharpe jsharpe force-pushed the configurable_python_sdk branch from dc1b85f to 2f2fb8f Compare July 22, 2025 10:31
@Mizux Mizux self-assigned this Jul 22, 2025
@Mizux
Copy link
Collaborator

Mizux commented Jul 22, 2025

Seems to follow the intended use of rules_python
src: https://github.com/bazel-contrib/rules_python/blob/main/tests/cc/current_py_cc_libs/BUILD.bazel

question: why not naming them py_cc_headers and py_cc_libs i.e.

 py_cc_headers = "@rules_python//python/cc:current_py_cc_headers",

@jsharpe
Copy link
Author

jsharpe commented Jul 22, 2025

Not particularly attached to any name in particular but I named it based on the target you'd typically override it with i.e. https://github.com/bazel-contrib/rules_python/blob/5281261a97235099a65aaeb83b74b30d16409798/python/private/hermetic_runtime_repo_setup.bzl#L110 - the python_headers target from a specific python version e.g. @python_3_10//:python_headers.

@Mizux Mizux added the enhancement New feature or request label Jul 22, 2025
@Mizux
Copy link
Collaborator

Mizux commented Oct 1, 2025

After second thought, I'm not sure this change will:

  • Interact nicely with the --@rules_python//python/config_settings:python_version=3.X ?
  • need that the build of the :pybind11 target also need to use the same custom python headers
    ref:
    deps = ["@rules_python//python/cc:current_py_cc_headers"],
  • Work as expected if a super build project want to integrate a repo which is hacking the python_headers/python_library in its pybind_extension() internal usage i.e. leading to mismatch and strange errors...

ps: concerning naming, I would instead use python_headers/python_library removing the _target suffix since bazel args never use this naming convention afaik...

@Mizux Mizux added the invalid This doesn't seem right label Oct 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants