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

[build] Remove two constants from python's version.bzl #22357

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
20 changes: 9 additions & 11 deletions tools/install/install.bzl
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("@python//:version.bzl", "PYTHON_SITE_PACKAGES_RELPATH", "PYTHON_VERSION")
load("@python//:version.bzl", "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 @@ -120,13 +120,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)

if type(strip_prefixes) == "dict":
strip_prefix = strip_prefixes.get(
Expand Down Expand Up @@ -590,7 +587,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 Down Expand Up @@ -636,8 +635,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
7 changes: 1 addition & 6 deletions tools/install/libdrake/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,7 @@ load(
"@drake//tools/workspace:cmake_configure_file.bzl",
"cmake_configure_file",
)
load(
"@python//:version.bzl",
"PYTHON_SITE_PACKAGES_RELPATH",
"PYTHON_VERSION",
)
load("@python//:version.bzl", "PYTHON_VERSION")
load(
"//third_party:com_github_bazelbuild_rules_cc/whole_archive.bzl",
"cc_whole_archive_library",
Expand Down Expand Up @@ -63,7 +59,6 @@ cmake_configure_file(
atonly = True,
cmakelists = ["stamp_version.cmake"],
defines = [
"PYTHON_SITE_PACKAGES_RELPATH=" + PYTHON_SITE_PACKAGES_RELPATH,
"PYTHON_VERSION=" + PYTHON_VERSION,
],
)
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
5 changes: 2 additions & 3 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
14 changes: 6 additions & 8 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 @@ -180,7 +179,7 @@ cc_binary(
)

cc_binary(
name = "_lcm" + PYTHON_EXTENSION_SUFFIX,
name = "_lcm.so",
srcs = [
"lcm-python/module.c",
"lcm-python/pyeventlog.c",
Expand Down Expand Up @@ -230,20 +229,19 @@ generate_file(
content = """
import ctypes
import os.path
ctypes.cdll.LoadLibrary(os.path.realpath(
__path__[0] + '/_lcm{extension_suffix}'))
ctypes.cdll.LoadLibrary(os.path.realpath(__path__[0] + '/_lcm.so'))
_filename = __path__[0] + \"/lcm-python/lcm/__init__.py\"
with open(_filename) as f:
_code = compile(f.read(), _filename, 'exec')
exec(_code)
""".format(extension_suffix = PYTHON_EXTENSION_SUFFIX),
""",
visibility = ["//visibility:private"],
)

py_library(
name = "lcm-python-upstream",
srcs = ["lcm-python/lcm/__init__.py"], # Actual code from upstream.
data = [":_lcm" + PYTHON_EXTENSION_SUFFIX],
data = [":_lcm.so"],
visibility = ["//visibility:private"],
)

Expand Down Expand Up @@ -305,10 +303,10 @@ install_files(
install(
name = "install_python",
targets = [
":_lcm" + PYTHON_EXTENSION_SUFFIX,
":_lcm.so",
":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
50 changes: 15 additions & 35 deletions tools/workspace/python/repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,6 @@ def _get_python_interpreter(repo_ctx):
])]).stdout.strip()
return (python, version)

def _get_extension_suffix(repo_ctx, python, python_config):
"""Returns the extension suffix, e.g. ".cpython-310-x86_64-linux-gnu.so" as
queried from python_config. Uses `python` only for error reporting.
"""
if which(repo_ctx, python_config) == None:
fail(("Cannot find corresponding config executable: {}\n" +
" From interpreter: {}").format(python_config, python))
return execute_or_fail(
repo_ctx,
[python_config, "--extension-suffix"],
).stdout.strip()

# TODO(jwnimmer-tri): Much of the logic for parsing includes and linkopts is
# the same or similar to that used in pkg_config.bzl and should be refactored
# and shared instead of being duplicated in both places.
Expand Down Expand Up @@ -165,11 +153,9 @@ def _impl(repo_ctx):
# Set `python` to the the interpreter path specified by our rule attrs,
# and `version` to its "major.minor" string.
python, version = _get_python_interpreter(repo_ctx)
site_packages_relpath = "lib/python{}/site-packages".format(version)

# Get extension_suffix, includes, and linkopts from python_config.
# Get includes and linkopts from python_config.
python_config = "{}-config".format(python)
extension_suffix = _get_extension_suffix(repo_ctx, python, python_config)
includes = _get_includes(repo_ctx, python_config)
linkopts = _get_linkopts(repo_ctx, python_config)

Expand All @@ -192,17 +178,13 @@ def _impl(repo_ctx):
# `BUILD.bazel` or `package.BUILD.bazel` files.

PYTHON_BIN_PATH = "{bin_path}"
PYTHON_EXTENSION_SUFFIX = "{extension_suffix}"
PYTHON_VERSION = "{version}"
PYTHON_SITE_PACKAGES_RELPATH = "{site_packages_relpath}"
PYTHON_INCLUDES = {includes}
PYTHON_LINKOPTS_EMBEDDED = {linkopts_embedded}
PYTHON_LINKOPTS_MODULE = {linkopts_module}
""".format(
bin_path = bin_path,
extension_suffix = extension_suffix,
version = version,
site_packages_relpath = site_packages_relpath,
includes = includes,
linkopts_module = linkopts_module,
linkopts_embedded = linkopts_embedded,
Expand All @@ -213,23 +195,21 @@ PYTHON_LINKOPTS_MODULE = {linkopts_module}
executable = False,
)

interpreter_path_attrs = {
"linux_interpreter_path": attr.string(
default = "/usr/bin/python3",
),
"macos_interpreter_path": attr.string(
# The version listed here should match what's listed in both the root
# CMakeLists.txt and doc/_pages/installation.md.
default = "{homebrew_prefix}/bin/python3.12",
),
"requirements_flavor": attr.string(
default = "test",
values = ["build", "test"],
),
}

python_repository = repository_rule(
_impl,
attrs = interpreter_path_attrs,
attrs = {
"linux_interpreter_path": attr.string(
default = "/usr/bin/python3",
),
"macos_interpreter_path": attr.string(
# The version listed here should match what's listed in both the
# root CMakeLists.txt and doc/_pages/installation.md.
default = "{homebrew_prefix}/bin/python3.12",
),
"requirements_flavor": attr.string(
default = "test",
values = ["build", "test"],
),
},
configure = True,
)