[bazel] Add MODULE.bazel #164891
[bazel] Add MODULE.bazel #164891
Conversation
|
Hey! I'm the maintainer of |
Thanks for being a maintainer! The build errors are link failures for libMLIRBindingsPythonCoreNoCAPI.so and libMLIRBindingsPythonCore.so: https://gist.github.com/rupprecht/ec8199ec929fe212f5b5ef967435efdc Maybe those would be fixed if we also used the rules like If you want to patch in and repro yourself:
I would definitely be interested in not having a local build file for nanobind (and robin_map). To keep changes small, I would like to do it separately from switching to bzlmod -- maybe that means only switching to your repo after switching to bzlmod, unless it'd be just as easy to use it w/ the WORKSPACE config, in which case we can do things in either order. |
|
Thanks, that helps! I will look into it. |
|
I think we actually need to change this PR to mostly avoid one off http_archive calls. The problem is before if the archives were named the same thing they would be shared, but that is no longer true with bzlmod, so for a project like ours that pulls in llvm with bazel, we would end up with 2 copies of nanobind for example, since we're providing one, and this would provide a new one. Ideally we could move every one of these calls into the BCR which would solve these potential problems. Alternatively it is possible that we could move these calls into a separate module_extension and potentially share them that way. |
|
@rupprecht I got a clean build with the following changes on top of this PR: diff --git a/utils/bazel/MODULE.bazel b/utils/bazel/MODULE.bazel
index e5ae53139abe..b425884701b1 100644
--- a/utils/bazel/MODULE.bazel
+++ b/utils/bazel/MODULE.bazel
@@ -6,6 +6,7 @@
bazel_dep(name = "apple_support", version = "1.24.1", repo_name = "build_bazel_apple_support")
bazel_dep(name = "bazel_skylib", version = "1.8.2")
+bazel_dep(name = "nanobind_bazel", version = "2.9.2")
bazel_dep(name = "platforms", version = "1.0.0")
bazel_dep(name = "rules_android", version = "0.6.6")
bazel_dep(name = "rules_cc", version = "0.2.11")
@@ -126,10 +127,10 @@ http_archive(
url = "https://github.com/Tessil/robin-map/archive/refs/tags/v1.3.0.tar.gz",
)
-http_archive(
- name = "nanobind",
- build_file = "@llvm-raw//utils/bazel/third_party_build:nanobind.BUILD",
- sha256 = "8ce3667dce3e64fc06bfb9b778b6f48731482362fb89a43da156632266cd5a90",
- strip_prefix = "nanobind-2.9.2",
- url = "https://github.com/wjakob/nanobind/archive/refs/tags/v2.9.2.tar.gz",
-)
+# http_archive(
+# name = "nanobind",
+# build_file = "@llvm-raw//utils/bazel/third_party_build:nanobind.BUILD",
+# sha256 = "8ce3667dce3e64fc06bfb9b778b6f48731482362fb89a43da156632266cd5a90",
+# strip_prefix = "nanobind-2.9.2",
+# url = "https://github.com/wjakob/nanobind/archive/refs/tags/v2.9.2.tar.gz",
+# )
diff --git a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
index d528daeb160c..09ff4869d0d4 100644
--- a/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/mlir/BUILD.bazel
@@ -7,6 +7,7 @@
load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
load("@bazel_skylib//rules:expand_template.bzl", "expand_template")
+load("@nanobind_bazel//:build_defs.bzl", "nanobind_extension", "nanobind_library")
load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_library")
load("//llvm:targets.bzl", "llvm_targets")
load(
@@ -1101,7 +1102,7 @@ cc_library(
],
)
-cc_library(
+nanobind_library(
name = "MLIRBindingsPythonNanobindHeaders",
includes = [
"include",
@@ -1109,12 +1110,11 @@ cc_library(
textual_hdrs = [":MLIRBindingsPythonHeaderFiles"],
deps = [
":CAPIIRHeaders",
- "@nanobind",
"@rules_python//python/cc:current_py_cc_headers",
],
)
-cc_library(
+nanobind_library(
name = "MLIRBindingsPythonNanobindHeadersAndDeps",
includes = [
"include",
@@ -1122,17 +1122,10 @@ cc_library(
textual_hdrs = [":MLIRBindingsPythonHeaderFiles"],
deps = [
":CAPIIR",
- "@nanobind",
"@rules_python//python/cc:current_py_cc_headers",
],
)
-# These flags are needed for pybind11 to work.
-PYBIND11_COPTS = [
- "-fexceptions",
- "-frtti",
-]
-
PYBIND11_FEATURES = [
# Cannot use header_modules (parse_headers feature fails).
"-use_header_modules",
@@ -1159,10 +1152,9 @@ filegroup(
]),
)
-cc_library(
+nanobind_library(
name = "MLIRBindingsPythonCore",
srcs = [":MLIRBindingsPythonSourceFiles"],
- copts = PYBIND11_COPTS,
features = PYBIND11_FEATURES,
includes = [
"lib/Bindings/Python",
@@ -1178,15 +1170,13 @@ cc_library(
":Support",
":config",
"//llvm:Support",
- "@nanobind",
"@rules_python//python/cc:current_py_cc_headers",
],
)
-cc_library(
+nanobind_library(
name = "MLIRBindingsPythonCoreNoCAPI",
srcs = [":MLIRBindingsPythonSourceFiles"],
- copts = PYBIND11_COPTS,
features = PYBIND11_FEATURES,
includes = [
"lib/Bindings/Python",
@@ -1201,7 +1191,6 @@ cc_library(
":Support",
":config",
"//llvm:Support",
- "@nanobind",
"@rules_python//python/cc:current_py_cc_headers",
],
)
@@ -1220,130 +1209,105 @@ cc_library(
)
# Dynamic library with the MLIR Python extension.
-cc_binary(
- name = "_mlir.so",
+nanobind_extension(
+ name = "_mlir",
srcs = ["lib/Bindings/Python/MainModule.cpp"],
- # These flags are needed for pybind11 to work.
- copts = PYBIND11_COPTS,
features = PYBIND11_FEATURES,
includes = [
"lib/Bindings/Python",
],
- linkshared = 1,
linkstatic = 0,
deps = [
":MLIRBindingsPythonCore",
":MLIRBindingsPythonHeadersAndDeps",
- "@nanobind",
],
)
-cc_binary(
- name = "_mlirDialectsIRDL.so",
+nanobind_extension(
+ name = "_mlirDialectsIRDL",
srcs = ["lib/Bindings/Python/DialectIRDL.cpp"],
- copts = PYBIND11_COPTS,
features = PYBIND11_FEATURES,
- linkshared = 1,
linkstatic = 0,
deps = [
":CAPIIR",
":MLIRBindingsPythonNanobindHeadersAndDeps",
- "@nanobind",
],
)
-cc_binary(
- name = "_mlirDialectsLinalg.so",
+nanobind_extension(
+ name = "_mlirDialectsLinalg",
srcs = ["lib/Bindings/Python/DialectLinalg.cpp"],
- copts = PYBIND11_COPTS,
features = PYBIND11_FEATURES,
includes = [
"lib/Bindings/Python",
],
- linkshared = 1,
linkstatic = 0,
deps = [
":CAPIIR",
":CAPILinalg",
":MLIRBindingsPythonNanobindHeadersAndDeps",
- "@nanobind",
],
)
-cc_binary(
- name = "_mlirDialectsLLVM.so",
+nanobind_extension(
+ name = "_mlirDialectsLLVM",
srcs = ["lib/Bindings/Python/DialectLLVM.cpp"],
- copts = PYBIND11_COPTS,
features = PYBIND11_FEATURES,
- linkshared = 1,
linkstatic = 0,
deps = [
":CAPIIR",
":CAPILLVM",
":CAPITarget",
":MLIRBindingsPythonNanobindHeadersAndDeps",
- "@nanobind",
],
)
-cc_binary(
- name = "_mlirDialectsQuant.so",
+nanobind_extension(
+ name = "_mlirDialectsQuant",
srcs = ["lib/Bindings/Python/DialectQuant.cpp"],
- copts = PYBIND11_COPTS,
features = PYBIND11_FEATURES,
- linkshared = 1,
linkstatic = 0,
deps = [
":CAPIIR",
":CAPIQuant",
":MLIRBindingsPythonNanobindHeadersAndDeps",
- "@nanobind",
],
)
-cc_binary(
- name = "_mlirDialectsSparseTensor.so",
+nanobind_extension(
+ name = "_mlirDialectsSparseTensor",
srcs = ["lib/Bindings/Python/DialectSparseTensor.cpp"],
- copts = PYBIND11_COPTS,
features = PYBIND11_FEATURES,
- linkshared = 1,
linkstatic = 0,
deps = [
":CAPIIR",
":CAPISparseTensor",
":MLIRBindingsPythonNanobindHeadersAndDeps",
- "@nanobind",
],
)
# Dynamic library with the MLIR Conversions Python extension.
-cc_binary(
- name = "_mlirExecutionEngine.so",
+nanobind_extension(
+ name = "_mlirExecutionEngine",
srcs = ["lib/Bindings/Python/ExecutionEngineModule.cpp"],
- copts = PYBIND11_COPTS,
features = PYBIND11_FEATURES,
- linkshared = 1,
linkstatic = 0,
deps = [
":CAPIExecutionEngine",
":MLIRBindingsPythonNanobindHeadersAndDeps",
- "@nanobind",
"@rules_python//python/cc:current_py_cc_headers",
],
)
# Dynamic library with the MLIR Linalg dialect+passes Python extension.
-cc_binary(
- name = "_mlirLinalgPasses.so",
+nanobind_extension(
+ name = "_mlirLinalgPasses",
srcs = ["lib/Bindings/Python/LinalgPasses.cpp"],
- copts = PYBIND11_COPTS,
features = PYBIND11_FEATURES,
- linkshared = 1,
linkstatic = 0,
deps = [
":CAPILinalg",
":MLIRBindingsPythonNanobindHeadersAndDeps",
- "@nanobind",
"@rules_python//python/cc:current_py_cc_headers",
],
)For context on this diff, I inject PYBIND11_COPTS as well as Furthermore, both the library and extension targets have You can view my nanobind BUILD file under https://github.com/nicholasjng/nanobind-bazel/blob/master/nanobind.BUILD. It is a little different from the one currently checked in (more specifically, the defines), but we could sort that out separately. Final note: I had to comment out lines 101 (use lld) and 207 (ram disk) in the |
|
another option is to use the |
|
IIRC the tricky part was to not break the "other" workflows that are not just the local builds. I think it was something where if you pull in the bazel deps and there is a MODULE.bazel file in it already, that it broke depending on llvm as external repo. I believe the key there was the use of (Also in that PR i had a comment mentioning |
utils/bazel/MODULE.bazel
Outdated
| name = "robin_map", | ||
| build_file = "@llvm-raw//utils/bazel/third_party_build:robin_map.BUILD", | ||
| sha256 = "a8424ad3b0affd4c57ed26f0f3d8a29604f0e1f2ef2089f497f614b1c94c7236", | ||
| strip_prefix = "robin-map-1.3.0", |
There was a problem hiding this comment.
Driveby comment, the latest nanobind uses 1.4.0
There was a problem hiding this comment.
Ack, I put in a TODO that we should bump it to 1.4.0, although probably instead of doing that, we will migrate to a central nanobind bazel_dep that pulls in the right robin-map version for us.
Having them all be part of BCR sounds like a good end goal. I like the idea of using a module extension for now, just to make this a straightforward migration to bzlmod, and then we can work on migrating deps individually after that.
Thanks @nicholasjng! I will give that a try once this is in.
I included the fix you pointed to, but I haven't tested it out yet to confirm it works when using LLVM as an external repo in another project. |
|
sounds good, fwiw there is still a behavior difference here that downstream projects cannot influence the version used by llvm with the current solution. so you can use the same one but you can't use a newer point release etc until we do that |
|
If there are no objections, I'll land this a bit later. Happy to fix forward or revert if it causes downstream issues.
Not sure what situation you're describing. Since these files live in tree, it's inherently tied to an LLVM version anyway. Do you mean someone would use this MODULE.bazel config but supply their own LLVM sources, either pinned to a different version or just forked from the same version, and this approach would not allow that? |
6 participants
This is a simple translation of the current WORKSPACE file.
bazel_dep(). The versions have been bumped to newer versions.maybe()doesn't seem to be a thing, so I just removed that.nanobind_bazelcould replace thenanobindconfig we have, but switching to that caused some build errors.This should have no effect since
.bazelrcdefinescommon --enable_bzlmod=false --enable_workspaceTested locally:
bazel test --enable_bzlmod --noenable_workspace --config=generic_clang @llvm-project//... //...