[PATCH] D113565: Split headers from implementations in MLIR C API Bazel build.

[Geoffrey Martin-Noble via Phabricator via llvm-commits]

GMNGeoffrey added inline comments.


================
Comment at: utils/bazel/llvm-project-overlay/mlir/BUILD.bazel:315
+
+CAPI_IR_HEADERS = [
+    "include/mlir-c/AffineExpr.h",
----------------
Could this be a glob with an exclude for each of the things in a separate library? I think that would be easier to maintain


================
Comment at: utils/bazel/llvm-project-overlay/mlir/BUILD.bazel:605
 cc_library(
     name = "CAPIExecutionEngine",
     srcs = ["lib/CAPI/ExecutionEngine/ExecutionEngine.cpp"],
----------------
Why isn't this one also split?


================
Comment at: utils/bazel/llvm-project-overlay/mlir/BUILD.bazel:677-680
+    textual_hdrs = glob([
+        "lib/Bindings/Python/*.h",
+        "include/mlir-c/Bindings/Python/*.h",
+        "include/mlir/Bindings/Python/*.h",
----------------
Factor this into a variable also?


================
Comment at: utils/bazel/llvm-project-overlay/mlir/BUILD.bazel:769
+cc_library(
+    name = "MLIRBindingsPythonCore",
+    deps = [
----------------
This is causing build failures. There isn't any pybind11 repository configured in the workspace here, so all transitive dependencies on pybind11 need to be tagged "nobuildkite" and "manual". Even if we had pybind11 configured in this WORKSPACE, we've marked everything that has a transitive external dependency as "manual" so a wildcard build can always build everything with no external dependencies. Carrying forward the tags is a bit annoying, but I don't think there's a way in Bazel to have transitive tags or something like that.


================
Comment at: utils/bazel/llvm-project-overlay/mlir/BUILD.bazel:774
+    ],
+    alwayslink = True,
 )
----------------
😢 alwayslink is a scourge. Is it really the only way?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113565/new/

https://reviews.llvm.org/D113565