From eaafe694d27f31fe05dd9d055da1e57c8d37a004 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Thu, 7 Mar 2024 13:28:55 +0100 Subject: [PATCH] Add Python bindings build using bzlmod (#1764) * Add a bzlmod Python bindings build Uses the newly started `@nanobind_bazel` project to build nanobind extensions. This means that we can drop all in-tree custom build defs and build files for nanobind and the C++ Python headers. Additionally, the temporary WORKSPACE overwrite hack naturally goes away due to the WORKSPACE system being obsolete. * Bump ruff -> v0.3.1, change ruff settings The latest minor releases incurred some formatting and configuration changes, this commit rolls them out. --------- Co-authored-by: dominic <510002+dmah42@users.noreply.github.com> --- .pre-commit-config.yaml | 2 +- MODULE.bazel | 22 +++- WORKSPACE | 6 - bindings/python/BUILD | 3 - bindings/python/build_defs.bzl | 29 ---- bindings/python/google_benchmark/BUILD | 18 +-- bindings/python/google_benchmark/__init__.py | 1 + bindings/python/nanobind.BUILD | 59 --------- bindings/python/python_headers.BUILD | 10 -- pyproject.toml | 3 +- setup.py | 132 +++++++++---------- tools/gbench/util.py | 6 +- 12 files changed, 92 insertions(+), 199 deletions(-) delete mode 100644 bindings/python/BUILD delete mode 100644 bindings/python/build_defs.bzl delete mode 100644 bindings/python/nanobind.BUILD delete mode 100644 bindings/python/python_headers.BUILD diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 0247d1b06..93455ab60 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,7 +11,7 @@ repos: types_or: [ python, pyi ] args: [ "--ignore-missing-imports", "--scripts-are-modules" ] - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.1.13 + rev: v0.3.1 hooks: - id: ruff args: [ --fix, --exit-non-zero-on-fix ] diff --git a/MODULE.bazel b/MODULE.bazel index 7e0e01612..45238d6f9 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -4,11 +4,11 @@ module( ) bazel_dep(name = "bazel_skylib", version = "1.5.0") -bazel_dep(name = "platforms", version = "0.0.7") +bazel_dep(name = "platforms", version = "0.0.8") bazel_dep(name = "rules_foreign_cc", version = "0.10.1") bazel_dep(name = "rules_cc", version = "0.0.9") -bazel_dep(name = "rules_python", version = "0.27.1", dev_dependency = True) +bazel_dep(name = "rules_python", version = "0.31.0", dev_dependency = True) bazel_dep(name = "googletest", version = "1.12.1", dev_dependency = True, repo_name = "com_google_googletest") bazel_dep(name = "libpfm", version = "4.11.0") @@ -19,7 +19,18 @@ bazel_dep(name = "libpfm", version = "4.11.0") # of relying on the changing default version from rules_python. python = use_extension("@rules_python//python/extensions:python.bzl", "python", dev_dependency = True) +python.toolchain(python_version = "3.8") python.toolchain(python_version = "3.9") +python.toolchain(python_version = "3.10") +python.toolchain(python_version = "3.11") +python.toolchain( + is_default = True, + python_version = "3.12", +) +use_repo( + python, + python = "python_versions", +) pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip", dev_dependency = True) pip.parse( @@ -30,3 +41,10 @@ pip.parse( use_repo(pip, "tools_pip_deps") # -- bazel_dep definitions -- # + +bazel_dep(name = "nanobind_bazel", version = "", dev_dependency = True) +git_override( + module_name = "nanobind_bazel", + commit = "97e3db2744d3f5da244a0846a0644ffb074b4880", + remote = "https://github.com/nicholasjng/nanobind-bazel", +) diff --git a/WORKSPACE b/WORKSPACE index 256207022..503202465 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -22,9 +22,3 @@ pip_parse( load("@tools_pip_deps//:requirements.bzl", "install_deps") install_deps() - -new_local_repository( - name = "python_headers", - build_file = "@//bindings/python:python_headers.BUILD", - path = "", # May be overwritten by setup.py. -) diff --git a/bindings/python/BUILD b/bindings/python/BUILD deleted file mode 100644 index d61dcb12a..000000000 --- a/bindings/python/BUILD +++ /dev/null @@ -1,3 +0,0 @@ -exports_files(glob(["*.BUILD"])) - -exports_files(["build_defs.bzl"]) diff --git a/bindings/python/build_defs.bzl b/bindings/python/build_defs.bzl deleted file mode 100644 index b0c1b0f58..000000000 --- a/bindings/python/build_defs.bzl +++ /dev/null @@ -1,29 +0,0 @@ -""" -This file contains some build definitions for C++ extensions used in the Google Benchmark Python bindings. -""" - -_SHARED_LIB_SUFFIX = { - "//conditions:default": ".so", - "//:windows": ".dll", -} - -def py_extension(name, srcs, hdrs = [], copts = [], features = [], deps = []): - for shared_lib_suffix in _SHARED_LIB_SUFFIX.values(): - shared_lib_name = name + shared_lib_suffix - native.cc_binary( - name = shared_lib_name, - linkshared = True, - linkstatic = True, - srcs = srcs + hdrs, - copts = copts, - features = features, - deps = deps, - ) - - return native.py_library( - name = name, - data = select({ - platform: [name + shared_lib_suffix] - for platform, shared_lib_suffix in _SHARED_LIB_SUFFIX.items() - }), - ) diff --git a/bindings/python/google_benchmark/BUILD b/bindings/python/google_benchmark/BUILD index f516a693e..0c8e3c103 100644 --- a/bindings/python/google_benchmark/BUILD +++ b/bindings/python/google_benchmark/BUILD @@ -1,4 +1,4 @@ -load("//bindings/python:build_defs.bzl", "py_extension") +load("@nanobind_bazel//:build_defs.bzl", "nanobind_extension") py_library( name = "google_benchmark", @@ -9,22 +9,10 @@ py_library( ], ) -py_extension( +nanobind_extension( name = "_benchmark", srcs = ["benchmark.cc"], - copts = [ - "-fexceptions", - "-fno-strict-aliasing", - ], - features = [ - "-use_header_modules", - "-parse_headers", - ], - deps = [ - "//:benchmark", - "@nanobind", - "@python_headers", - ], + deps = ["//:benchmark"], ) py_test( diff --git a/bindings/python/google_benchmark/__init__.py b/bindings/python/google_benchmark/__init__.py index e14769f45..c1393b4e5 100644 --- a/bindings/python/google_benchmark/__init__.py +++ b/bindings/python/google_benchmark/__init__.py @@ -26,6 +26,7 @@ def my_benchmark(state): if __name__ == '__main__': benchmark.main() """ + import atexit from absl import app diff --git a/bindings/python/nanobind.BUILD b/bindings/python/nanobind.BUILD deleted file mode 100644 index 9874b80d1..000000000 --- a/bindings/python/nanobind.BUILD +++ /dev/null @@ -1,59 +0,0 @@ -load("@bazel_skylib//lib:selects.bzl", "selects") - -licenses(["notice"]) - -package(default_visibility = ["//visibility:public"]) - -config_setting( - name = "msvc_compiler", - flag_values = {"@bazel_tools//tools/cpp:compiler": "msvc-cl"}, -) - -selects.config_setting_group( - name = "winplusmsvc", - match_all = [ - "@platforms//os:windows", - ":msvc_compiler", - ], -) - -cc_library( - name = "nanobind", - srcs = glob([ - "src/*.cpp", - ]), - additional_linker_inputs = select({ - "@platforms//os:macos": [":cmake/darwin-ld-cpython.sym"], - "//conditions:default": [], - }), - copts = select({ - ":msvc_compiler": [ - "/EHsc", # exceptions - "/Os", # size optimizations - "/GL", # LTO / whole program optimization - ], - # these should work on both clang and gcc. - "//conditions:default": [ - "-fexceptions", - "-flto", - "-Os", - ], - }), - includes = [ - "ext/robin_map/include", - "include", - ], - linkopts = select({ - ":winplusmsvc": ["/LTGC"], # Windows + MSVC. - "@platforms//os:macos": ["-Wl,@$(location :cmake/darwin-ld-cpython.sym)"], # Apple. - "//conditions:default": [], - }), - textual_hdrs = glob( - [ - "include/**/*.h", - "src/*.h", - "ext/robin_map/include/tsl/*.h", - ], - ), - deps = ["@python_headers"], -) diff --git a/bindings/python/python_headers.BUILD b/bindings/python/python_headers.BUILD deleted file mode 100644 index 8f139f862..000000000 --- a/bindings/python/python_headers.BUILD +++ /dev/null @@ -1,10 +0,0 @@ -licenses(["notice"]) - -package(default_visibility = ["//visibility:public"]) - -cc_library( - name = "python_headers", - hdrs = glob(["**/*.h"]), - includes = ["."], - visibility = ["//visibility:public"], -) diff --git a/pyproject.toml b/pyproject.toml index aa24ae8c3..62507a870 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -75,11 +75,12 @@ src = ["bindings/python"] line-length = 80 target-version = "py311" +[tool.ruff.lint] # Enable pycodestyle (`E`, `W`), Pyflakes (`F`), and isort (`I`) codes by default. select = ["E", "F", "I", "W"] ignore = [ "E501", # line too long ] -[tool.ruff.isort] +[tool.ruff.lint.isort] combine-as-imports = true diff --git a/setup.py b/setup.py index cb20042da..910383c76 100644 --- a/setup.py +++ b/setup.py @@ -1,46 +1,27 @@ -import contextlib import os import platform import shutil -import sysconfig from pathlib import Path -from typing import Generator +from typing import Any import setuptools from setuptools.command import build_ext -PYTHON_INCLUDE_PATH_PLACEHOLDER = "" - IS_WINDOWS = platform.system() == "Windows" IS_MAC = platform.system() == "Darwin" - -@contextlib.contextmanager -def temp_fill_include_path(fp: str) -> Generator[None, None, None]: - """Temporarily set the Python include path in a file.""" - with open(fp, "r+") as f: - try: - content = f.read() - replaced = content.replace( - PYTHON_INCLUDE_PATH_PLACEHOLDER, - Path(sysconfig.get_paths()["include"]).as_posix(), - ) - f.seek(0) - f.write(replaced) - f.truncate() - yield - finally: - # revert to the original content after exit - f.seek(0) - f.write(content) - f.truncate() +# hardcoded SABI-related options. Requires that each Python interpreter +# (hermetic or not) participating is of the same major-minor version. +version_tuple = tuple(int(i) for i in platform.python_version_tuple()) +py_limited_api = version_tuple >= (3, 12) +options = {"bdist_wheel": {"py_limited_api": "cp312"}} if py_limited_api else {} class BazelExtension(setuptools.Extension): """A C/C++ extension that is defined as a Bazel BUILD target.""" - def __init__(self, name: str, bazel_target: str): - super().__init__(name=name, sources=[]) + def __init__(self, name: str, bazel_target: str, **kwargs: Any): + super().__init__(name=name, sources=[], **kwargs) self.bazel_target = bazel_target stripped_target = bazel_target.split("//")[-1] @@ -67,49 +48,58 @@ def copy_extensions_to_source(self): def bazel_build(self, ext: BazelExtension) -> None: """Runs the bazel build to create the package.""" - with temp_fill_include_path("WORKSPACE"): - temp_path = Path(self.build_temp) - - bazel_argv = [ - "bazel", - "build", - ext.bazel_target, - "--enable_bzlmod=false", - f"--symlink_prefix={temp_path / 'bazel-'}", - f"--compilation_mode={'dbg' if self.debug else 'opt'}", - # C++17 is required by nanobind - f"--cxxopt={'/std:c++17' if IS_WINDOWS else '-std=c++17'}", - ] - - if IS_WINDOWS: - # Link with python*.lib. - for library_dir in self.library_dirs: - bazel_argv.append("--linkopt=/LIBPATH:" + library_dir) - elif IS_MAC: - if platform.machine() == "x86_64": - # C++17 needs macOS 10.14 at minimum - bazel_argv.append("--macos_minimum_os=10.14") - - # cross-compilation for Mac ARM64 on GitHub Mac x86 runners. - # ARCHFLAGS is set by cibuildwheel before macOS wheel builds. - archflags = os.getenv("ARCHFLAGS", "") - if "arm64" in archflags: - bazel_argv.append("--cpu=darwin_arm64") - bazel_argv.append("--macos_cpus=arm64") - - elif platform.machine() == "arm64": - bazel_argv.append("--macos_minimum_os=11.0") - - self.spawn(bazel_argv) - - shared_lib_suffix = ".dll" if IS_WINDOWS else ".so" - ext_name = ext.target_name + shared_lib_suffix - ext_bazel_bin_path = ( - temp_path / "bazel-bin" / ext.relpath / ext_name - ) - - ext_dest_path = Path(self.get_ext_fullpath(ext.name)) - shutil.copyfile(ext_bazel_bin_path, ext_dest_path) + temp_path = Path(self.build_temp) + # omit the patch version to avoid build errors if the toolchain is not + # yet registered in the current @rules_python version. + # patch version differences should be fine. + python_version = ".".join(platform.python_version_tuple()[:2]) + + bazel_argv = [ + "bazel", + "build", + ext.bazel_target, + f"--symlink_prefix={temp_path / 'bazel-'}", + f"--compilation_mode={'dbg' if self.debug else 'opt'}", + # C++17 is required by nanobind + f"--cxxopt={'/std:c++17' if IS_WINDOWS else '-std=c++17'}", + f"--@rules_python//python/config_settings:python_version={python_version}", + ] + + if ext.py_limited_api: + bazel_argv += ["--@nanobind_bazel//:py-limited-api=cp312"] + + if IS_WINDOWS: + # Link with python*.lib. + for library_dir in self.library_dirs: + bazel_argv.append("--linkopt=/LIBPATH:" + library_dir) + elif IS_MAC: + if platform.machine() == "x86_64": + # C++17 needs macOS 10.14 at minimum + bazel_argv.append("--macos_minimum_os=10.14") + + # cross-compilation for Mac ARM64 on GitHub Mac x86 runners. + # ARCHFLAGS is set by cibuildwheel before macOS wheel builds. + archflags = os.getenv("ARCHFLAGS", "") + if "arm64" in archflags: + bazel_argv.append("--cpu=darwin_arm64") + bazel_argv.append("--macos_cpus=arm64") + + elif platform.machine() == "arm64": + bazel_argv.append("--macos_minimum_os=11.0") + + self.spawn(bazel_argv) + + if IS_WINDOWS: + suffix = ".pyd" + else: + suffix = ".abi3.so" if ext.py_limited_api else ".so" + + ext_name = ext.target_name + suffix + ext_bazel_bin_path = temp_path / "bazel-bin" / ext.relpath / ext_name + ext_dest_path = Path(self.get_ext_fullpath(ext.name)).with_name( + ext_name + ) + shutil.copyfile(ext_bazel_bin_path, ext_dest_path) setuptools.setup( @@ -118,6 +108,8 @@ def bazel_build(self, ext: BazelExtension) -> None: BazelExtension( name="google_benchmark._benchmark", bazel_target="//bindings/python/google_benchmark:_benchmark", + py_limited_api=py_limited_api, ) ], + options=options, ) diff --git a/tools/gbench/util.py b/tools/gbench/util.py index 4d061a3a1..1119a1a2c 100644 --- a/tools/gbench/util.py +++ b/tools/gbench/util.py @@ -1,5 +1,5 @@ -"""util.py - General utilities for running, loading, and processing benchmarks -""" +"""util.py - General utilities for running, loading, and processing benchmarks""" + import json import os import re @@ -37,7 +37,7 @@ def is_executable_file(filename): elif sys.platform.startswith("win"): return magic_bytes == b"MZ" else: - return magic_bytes == b"\x7FELF" + return magic_bytes == b"\x7fELF" def is_json_file(filename):