Skip to content

Commit

Permalink
[build] Use py_cc_toolchain for configuring pybind11
Browse files Browse the repository at this point in the history
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 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.

Therefore, it's no longer necessary for our prior approach (the
'@python' repository) to be public, so that repository is now
deprecated for removal.

To simplify this transition, we also revise how pydrake module shared
libraries are named in our binary packages. The importlib.machinery
package defines many possible values for EXTENSION_SUFFIXES (i.e., the
filename extension used for native code), e.g., for example on Ubuntu
22.04 the possible values are ['.cpython-310-x86_64-linux-gnu.so',
'.abi3.so', '.so']. Previously we used the first choice (with the
python version baked in) but this is awkward, so now we use the last
option (plain '.so'). The distinction only matters in case we package
multiple python versions into the same release artifact at once, which
we never do. It will also be moot once we switch to using ABI 3 in the
future (in which case we'll use the middle choice).

Ditto for lcm-python's shared library.

Details:

We now call our python_repository rule twice: once to create "python"
(which is deprecated and no longer unused by Drake) and once to create
"python_internal". The internal flavor provides fewer constants since
many are no longer needed with the py_cc_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 PYTHON_SITE_PACKAGES is no longer used as an opaque constant; its
correct value is trivially derivable from the Python version number so
we'll just use the longhand anywhere we need such a path.
  • Loading branch information
jwnimmer-tri committed Dec 28, 2024
1 parent 6fdfa7e commit b33d113
Show file tree
Hide file tree
Showing 18 changed files with 242 additions and 168 deletions.
3 changes: 3 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,9 @@ endfunction()
set(BAZEL_WORKSPACE_EXTRA)
set(BAZEL_WORKSPACE_EXCLUDES)

# Our cmake/WORKSPACE.bzlmod always provides @python.
list(APPEND BAZEL_WORKSPACE_EXCLUDES "python")

macro(override_repository NAME)
set(repo "${CMAKE_CURRENT_BINARY_DIR}/external/workspace/${NAME}")
string(APPEND BAZEL_WORKSPACE_EXTRA
Expand Down
5 changes: 0 additions & 5 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,6 @@ cc_configure = use_extension(
)
use_repo(cc_configure, "local_config_cc")

register_toolchains(
"//tools/py_toolchain:toolchain",
"//tools/py_toolchain:exec_tools_toolchain",
)

# TODO(#20731) Move all of our dependencies from WORKSPACE.bzlmod into this
# file, so that downstream projects can consume Drake exclusively via bzlmod
# (and so that we can delete our WORKSPACE files prior to Bazel 9 which drops
Expand Down
2 changes: 1 addition & 1 deletion bindings/pydrake/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ install(
":pydrake_pyi",
":pydrake_typed",
],
data_dest = "@PYTHON_SITE_PACKAGES@",
data_dest = "lib/python@PYTHON_VERSION@/site-packages",
visibility = ["//visibility:public"],
deps = get_drake_py_installs(PY_LIBRARIES_WITH_INSTALL) + [
# These three modules are a special case.
Expand Down
5 changes: 1 addition & 4 deletions cmake/WORKSPACE.bzlmod.in
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,8 @@ python_repository(
# Custom repository rules injected by CMake.
@BAZEL_WORKSPACE_EXTRA@

# The list of repositories already provided via BAZEL_WORKSPACE_EXTRA.
_BAZEL_WORKSPACE_EXCLUDES = split_cmake_list("@BAZEL_WORKSPACE_EXCLUDES@")

# For anything not already overridden, use Drake's default externals.
add_default_workspace(
repository_excludes = ["python"] + _BAZEL_WORKSPACE_EXCLUDES,
repository_excludes = split_cmake_list("@BAZEL_WORKSPACE_EXCLUDES@"),
bzlmod = True,
)
3 changes: 3 additions & 0 deletions cmake/bazel.rc.in
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ startup --output_base="@BAZEL_OUTPUT_BASE@"
# Environment variables to be used in repository rules (if any).
common @BAZEL_REPO_ENV@

# Use the Python interpreter from our cmake/WORKSPACE.bzlmod.in.
build --extra_toolchains=@python//:all

# Disable the "convenience symlinks" intended for Bazel users; they only add
# confusion for the CMake use case.
build --symlink_prefix=/
Expand Down
3 changes: 3 additions & 0 deletions tools/bazel.rc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ build -c opt
build --strip=never
build --strict_system_includes

# Use the host Python interpreter by default.
build --extra_toolchains=@python//:all

# Use C++20 by default.
build --cxxopt=-std=c++20
build --host_cxxopt=-std=c++20
Expand Down
33 changes: 22 additions & 11 deletions tools/install/install.bzl
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
load("@python//:version.bzl", "PYTHON_SITE_PACKAGES_RELPATH", "PYTHON_VERSION")
load("@rules_license//rules:providers.bzl", "LicenseInfo")
load("//tools/skylark:cc.bzl", "CcInfo")
load("//tools/skylark:drake_java.bzl", "MainClassInfo")
Expand Down Expand Up @@ -48,6 +47,16 @@ def _depset_to_list(x):
iter_list = x.to_list() if type(x) == "depset" else x
return iter_list

#------------------------------------------------------------------------------

_PY_CC_TOOLCHAIN_TYPE = "@rules_python//python/cc:toolchain_type"

def _python_version(ctx):
"""Returns a string a containing the major.minor version number of the
current Python toolchain."""
py_cc_toolchain = ctx.toolchains[_PY_CC_TOOLCHAIN_TYPE].py_cc_toolchain
return py_cc_toolchain.python_version

#------------------------------------------------------------------------------
def _output_path(ctx, input_file, strip_prefix = [], ignore_errors = False):
"""Compute output path (without destination prefix) for install action.
Expand Down Expand Up @@ -120,13 +129,10 @@ def _install_action(
else:
dest = dests

dest_replacements = (
("@WORKSPACE@", _workspace(ctx)),
("@PYTHON_SITE_PACKAGES@", PYTHON_SITE_PACKAGES_RELPATH),
)
for old, new in dest_replacements:
if old in dest:
dest = dest.replace(old, new)
if "@WORKSPACE@" in dest:
dest = dest.replace("@WORKSPACE@", _workspace(ctx))
if "@PYTHON_VERSION@" in dest:
dest = dest.replace("@PYTHON_VERSION@", _python_version(ctx))

if type(strip_prefixes) == "dict":
strip_prefix = strip_prefixes.get(
Expand Down Expand Up @@ -590,7 +596,9 @@ _install_rule = rule(
"runtime_strip_prefix": attr.string_list(),
"java_dest": attr.string(default = "share/java"),
"java_strip_prefix": attr.string_list(),
"py_dest": attr.string(default = "@PYTHON_SITE_PACKAGES@"),
"py_dest": attr.string(
default = "lib/python@PYTHON_VERSION@/site-packages",
),
"py_strip_prefix": attr.string_list(),
"rename": attr.string_dict(),
"install_tests": attr.label_list(
Expand All @@ -609,6 +617,10 @@ _install_rule = rule(
},
executable = True,
implementation = _install_impl,
toolchains = [
# Used to discern the major.minor site-packages path to install into.
_PY_CC_TOOLCHAIN_TYPE,
],
)

def install(tags = [], **kwargs):
Expand Down Expand Up @@ -636,8 +648,7 @@ Destination paths may include the following placeholders:
* ``@WORKSPACE@``, replaced with ``workspace`` (if specified) or the name of
the workspace which invokes ``install``.
* ``@PYTHON_SITE_PACKAGES``, replaced with the Python version-specific path of
"site-packages".
* ``@PYTHON_VERSION@``, replaced with the Python major.minor version.
Note:
By default, headers and resource files to be installed must be explicitly
Expand Down
19 changes: 10 additions & 9 deletions tools/install/libdrake/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@ load(
"@drake//tools/workspace:cmake_configure_file.bzl",
"cmake_configure_file",
)
load(
"@python//:version.bzl",
"PYTHON_SITE_PACKAGES_RELPATH",
"PYTHON_VERSION",
)
load(
"//third_party:com_github_bazelbuild_rules_cc/whole_archive.bzl",
"cc_whole_archive_library",
Expand All @@ -33,6 +28,13 @@ load(":header_lint.bzl", "cc_check_allowed_headers")

package(default_visibility = ["//visibility:private"])

genrule(
name = "python_version_cmake",
srcs = ["//tools/workspace/python:python_version.txt"],
outs = ["python_version.cmake"],
cmd = """echo "set(PYTHON_VERSION \\"$$(cat $<)\\")" > $@""",
)

genrule(
name = "stamp_version",
outs = ["stamp_version.txt"],
Expand Down Expand Up @@ -61,10 +63,9 @@ cmake_configure_file(
src = "drake-config.cmake.in",
out = "drake-config.cmake",
atonly = True,
cmakelists = ["stamp_version.cmake"],
defines = [
"PYTHON_SITE_PACKAGES_RELPATH=" + PYTHON_SITE_PACKAGES_RELPATH,
"PYTHON_VERSION=" + PYTHON_VERSION,
cmakelists = [
":python_version.cmake",
":stamp_version.cmake",
],
)

Expand Down
2 changes: 1 addition & 1 deletion tools/install/libdrake/drake-config.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ set(${CMAKE_FIND_PACKAGE_NAME}_VERSION_PATCH "@DRAKE_VERSION_PATCH@")
set(${CMAKE_FIND_PACKAGE_NAME}_VERSION_TWEAK "@DRAKE_VERSION_TWEAK@")


set(${CMAKE_FIND_PACKAGE_NAME}_PYTHON_DIR "${${CMAKE_FIND_PACKAGE_NAME}_IMPORT_PREFIX}/@PYTHON_SITE_PACKAGES_RELPATH@")
set(${CMAKE_FIND_PACKAGE_NAME}_PYTHON_DIR "${${CMAKE_FIND_PACKAGE_NAME}_IMPORT_PREFIX}/lib/python@PYTHON_VERSION@/site-packages")
# Allow users to easily check Drake's expected CPython version.
set(${CMAKE_FIND_PACKAGE_NAME}_PYTHON_VERSION "@PYTHON_VERSION@")

Expand Down
37 changes: 0 additions & 37 deletions tools/py_toolchain/BUILD.bazel

This file was deleted.

7 changes: 3 additions & 4 deletions tools/skylark/pybind.bzl
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
load("@cc//:compiler.bzl", "COMPILER_ID")
load("@python//:version.bzl", "PYTHON_EXTENSION_SUFFIX")
load("//tools/install:install.bzl", "install")
load("//tools/skylark:cc.bzl", "CcInfo", "cc_binary")
load("//tools/skylark:drake_cc.bzl", "drake_cc_binary", "drake_cc_googletest")
Expand Down Expand Up @@ -57,7 +56,7 @@ def pybind_py_library(

# TODO(eric.cousineau): See if we can keep non-`*.so` target name, but
# output a *.so, so that the target name is similar to what is provided.
cc_so_target = cc_so_name + PYTHON_EXTENSION_SUFFIX
cc_so_target = cc_so_name + ".so"

# Add C++ shared library.
cc_binary_rule(
Expand Down Expand Up @@ -232,7 +231,7 @@ def get_pybind_package_info(base_package, sub_package = None):
package_info = _get_package_info(base_package, sub_package)
return struct(
py_imports = [package_info.base_path_rel],
py_dest = "@PYTHON_SITE_PACKAGES@/{}".format(
py_dest = "lib/python@PYTHON_VERSION@/site-packages/{}".format(
package_info.sub_path_rel,
),
)
Expand Down Expand Up @@ -288,8 +287,8 @@ def drake_pybind_cc_googletest(
deps = cc_deps + [
"//:drake_shared_library",
"//bindings/pydrake:pydrake_pybind",
"//tools/workspace/python:cc_libpython",
"@pybind11",
"@python//:python_direct_link",
],
copts = cc_copts,
use_default_main = False,
Expand Down
10 changes: 3 additions & 7 deletions tools/workspace/default.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -391,16 +391,12 @@ def add_default_toolchains(
set this to True if you are using bzlmod.
"""
if bzlmod:
# All toolchains are in MODULE.bazel already.
# The cc toolchain is in MODULE.bazel already.
# The py toolchain is in tools/bazel.rc already.
return

if "py" not in excludes:
native.register_toolchains(
"//tools/py_toolchain:toolchain",
)
native.register_toolchains(
"//tools/py_toolchain:exec_tools_toolchain",
)
native.register_toolchains("@python//:all")
if "rust" not in excludes:
register_rust_toolchains()

Expand Down
8 changes: 5 additions & 3 deletions tools/workspace/lcm/package.BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ load(
"generate_export_header",
)
load("@drake//tools/workspace:generate_file.bzl", "generate_file")
load("@python//:version.bzl", "PYTHON_EXTENSION_SUFFIX")

licenses([
"notice", # BSD-3-Clause
Expand Down Expand Up @@ -179,6 +178,8 @@ cc_binary(
],
)

PYTHON_EXTENSION_SUFFIX = ".so"

cc_binary(
name = "_lcm" + PYTHON_EXTENSION_SUFFIX,
srcs = [
Expand All @@ -196,7 +197,8 @@ cc_binary(
visibility = ["//visibility:private"],
deps = [
":lcm",
"@python",
"@drake//tools/workspace/python:cc_headers",
"@drake//tools/workspace/python:cc_libs",
],
)

Expand Down Expand Up @@ -308,7 +310,7 @@ install(
":_lcm" + PYTHON_EXTENSION_SUFFIX,
":lcm-python-upstream",
],
library_dest = "@PYTHON_SITE_PACKAGES@/lcm",
library_dest = "lib/python@PYTHON_VERSION@/site-packages/lcm",
py_strip_prefix = ["lcm-python"],
)

Expand Down
3 changes: 2 additions & 1 deletion tools/workspace/pybind11/package.BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ cc_library(
includes = ["include"],
deps = [
"@eigen",
"@python",
"@drake//tools/workspace/python:cc_headers",
"@drake//tools/workspace/python:cc_libs",
],
)

Expand Down
32 changes: 29 additions & 3 deletions tools/workspace/python/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,6 +1,32 @@
# This file exists to make our directory into a Bazel package, so that our
# neighboring *.bzl file can be loaded elsewhere.

load("//tools/lint:lint.bzl", "add_lint_tests")
load(":defs.bzl", "current_py_cc_libpython", "python_version_txt")

package(default_visibility = ["//visibility:public"])

# Provides a text file containing the major.minor version number of the current
# Python toolchain, without any newlines.
python_version_txt(
name = "python_version.txt",
)

# Provides a single point of control within Drake for how to compile a native
# C/C++ Python module (e.g., for pybind11).
alias(
name = "cc_headers",
actual = "@rules_python//python/cc:current_py_cc_headers",
)

# Provides a single point of control within Drake for how to link a native
# C/C++ Python module (e.g., for pybind11).
alias(
name = "cc_libs",
actual = "@rules_python//python/cc:current_py_cc_libs",
)

# Provides a single point of control within Drake for how to link a C/C++
# exectuable that embeds a Python interpreter (e.g., for unit testing).
current_py_cc_libpython(
name = "cc_libpython",
)

add_lint_tests()
Loading

0 comments on commit b33d113

Please sign in to comment.