[build] Use py_cc_toolchain for configuring pybind11 #22346
Conversation
jwnimmer-tri
added
priority: medium
release notes: newly deprecated
jwnimmer-tri
force-pushed
the
repo-python
branch
2 times, most recently
from
53905ea to
00f2983
Compare
jwnimmer-tri
force-pushed
the
repo-python
branch
from
00f2983 to
7d91e3e
Compare
jwnimmer-tri
force-pushed
the
repo-python
branch
from
7d91e3e to
b33d113
Compare
jwnimmer-tri
force-pushed
the
repo-python
branch
2 times, most recently
from
9482c13 to
25f448b
Compare
jwnimmer-tri
force-pushed
the
repo-python
branch
from
25f448b to
38b0dbc
Compare
There was a problem hiding this comment.
+@rpoyner-tri 
Reviewed 12 of 19 files at r6, 3 of 4 files at r7, 17 of 17 files at r8, all commit messages.
Reviewable status: 1 unresolved discussion, needs at least two assigned reviewers
tools/workspace/python/defs.bzl line 32 at r9 (raw file):
toolchains = [_PY_CC_TOOLCHAIN_TYPE], provides = [CcInfo], doc = """Provides the linker flags for how to link a C/C++ exectuable
typo
Suggestion:
executable
The py_cc_toolchain is a rules_python concept that bridges the Python and C++ toolchains by providing cc_library targets for the headers and libraries for compiling C code against the Python interpreter selected by the Python toolchain. This commit changes our python repository to define that new kind of toolchain and our pydrake bindings to use it. The toolchains are now encapsulated as `@python//:all`, moved from `//tools/py_toolchain`. Important note for future bzlmod users: because our python repository rule can fail when run on a system without our default interpreter, our MODULE.bazel will no longer register our default python toolchain automatically. Downstream Bazel projects using Drake as a module will need to opt-in to the local toolchain. We provide new labels in //tools/workspace/python as a single point of control for depending on Python. The comments in python/repository.bzl provide an overview of how the pieces all fit together. The legacy libraries `@python` and `@python//:python_direct_link` are deprecated.
jwnimmer-tri
force-pushed
the
repo-python
branch
from
38b0dbc to
38d4428
Compare
There was a problem hiding this comment.
Reviewed 7 of 7 files at r1, 13 of 19 files at r3, 1 of 3 files at r4, 7 of 7 files at r5, 19 of 19 files at r6, 4 of 4 files at r7, 17 of 17 files at r8, 7 of 7 files at r10, all commit messages.
Reviewable status: needs at least two assigned reviewers
There was a problem hiding this comment.
Reviewed 7 of 7 files at r10, all commit messages.
Reviewable status: needs at least two assigned reviewers
|
+@ggould-tri for platform review per schedule, please. |
There was a problem hiding this comment.
Reviewed 12 of 19 files at r6, 3 of 4 files at r7, 17 of 17 files at r8, 7 of 7 files at r10, all commit messages.
Reviewable status: 1 unresolved discussion
tools/workspace/python/repository.bzl line 13 at r9 (raw file):
Anywhere in Drake that needs to *consume* the python toolchain information should use the aliases provided by //tools/workspace/python which always cite the current toolchain (which might not be Drake's toolchain in case a user
nit: this phrasing reads oddly to me. Reword?
Suggestion:
if
There was a problem hiding this comment.
Reviewable status:
complete! all discussions resolved, LGTM from assignees rpoyner-tri(platform),ggould-tri(platform)
tools/workspace/python/repository.bzl line 13 at r9 (raw file):
Previously, ggould-tri wrote…
nit: this phrasing reads oddly to me. Reword?
Meh. I like my phrasing slightly better.
jwnimmer-tri
added
the
release notes: fix
The
py_cc_toolchainis arules_pythonconcept that bridges the Python and C++ toolchains by providingcc_librarytargets for the headers and libraries that allow compiling C code against the Python interpreter selected by the Python toolchain.This commit changes our MODULE and WORKSPACE to add that new kind of toolchain and our pydrake bindings to use it.
Towards #20731.
This change is