Don't define //mlir:MLIRBindingsPythonCore in terms of the NoCAPI and CAPIDeps targets.

[phawkins]

Summary

We noticed that the library structure causes link ordering problems in Google's internal build. However, we don't think the problem is specific to Google's build, it probably can be reproduced anywhere with the right library structure.

In general splitting the Python bindings from their dependencies (the C API targets) creates the possibility that the two libraries might end up in the wrong order on the linker command line. We can avoid this problem happening by reverting the structure of the MLIRBindingsPythonCore to represent its dependencies in the usual way, rather than composing an incomplete MLIRBindingsPythonCoreNoCAPI target and their CAPI dependencies. It was probably a mistake to rewrite this particular cc_library() rule in terms of the two, since nothing guarantees that the two will be correctly ordered by the linker when both are being linked into the same binary, and it was only an incidental "cleanup" done in passing.

Otherwise the previous PR (D113565) is fine, since that was about the case where both are being built into two separate shared libraries. It just shouldn't have made this (unrelated) change.

[GMNGeoffrey]

Is there something special about Google's internal version of Bazel that would make this different? Or just that's where you noticed it and it's a potential problem in OSS as well? I want to make sure we're making the decisions that make the most sense for Bazel users (the differences between Bazel and Blaze are on Google(rs) to figure out IMO)

[phawkins]

Updated the PR description to clarify that this problem might be seen generally. Please take a look!

[GMNGeoffrey]

In D113773#3128113, @phawkins wrote:

Updated the PR description to clarify that this problem might be seen generally. Please take a look!

Thanks for adding the extra detail :-) I'll land now

utils/bazel/llvm-project-overlay/mlir/BUILD.bazel

cc_library(
     textual_hdrs = glob(MLIR_BINDINGS_PYTHON_HEADERS),
     deps = [
         ":CAPIIR",
         "@pybind11",
         "@python_runtime//:headers",
     ],
 )
 
-cc_library(
-    name = "MLIRBindingsPythonCoreNoCAPI",
-    srcs = [
-        "lib/Bindings/Python/DialectLinalg.cpp",
-        "lib/Bindings/Python/DialectSparseTensor.cpp",
-        "lib/Bindings/Python/IRAffine.cpp",
-        "lib/Bindings/Python/IRAttributes.cpp",
-        "lib/Bindings/Python/IRCore.cpp",
-        "lib/Bindings/Python/IRInterfaces.cpp",
-        "lib/Bindings/Python/IRModule.cpp",
-        "lib/Bindings/Python/IRTypes.cpp",
-        "lib/Bindings/Python/Pass.cpp",
-        "lib/Bindings/Python/PybindUtils.cpp",
-    ],
+MLIR_PYTHON_BINDINGS_SOURCES = [
+]
+
+cc_library(
+    name = "MLIRBindingsPythonCore",
+    srcs = MLIR_PYTHON_BINDINGS_SOURCES,
     # These flags are needed for pybind11 to work.
     copts = [
         "-fexceptions",
         "-frtti",
     ],
     features = [
         # Cannot use header_modules (parse_headers feature fails).
         "-use_header_modules",
     ],
     tags = [
         "manual",  # External dependency
         "nobuildkite",  # TODO(gcmn): Add support for this target
     ],
     deps = [
-        ":CAPIAsyncHeaders",
-        ":CAPIDebugHeaders",
-        ":CAPIGPUHeaders",
-        ":CAPIIRHeaders",
-        ":CAPILinalgHeaders",
-        ":CAPIRegistrationHeaders",
-        ":CAPISparseTensorHeaders",
-        ":MLIRBindingsPythonHeaders",
+        ":CAPIAsync",
+        ":CAPIDebug",
+        ":CAPIGPU",
+        ":CAPIIR",
+        ":CAPIInterfaces",
+        ":CAPILinalg",
+        ":CAPIRegistration",
+        ":CAPISparseTensor",
+        ":MLIRBindingsPythonHeadersAndDeps",
         "//llvm:Support",
         "@pybind11",
         "@python_runtime//:headers",
     ],
 )
 
 cc_library(
-    name = "MLIRBindingsPythonCAPIDeps",
+    name = "MLIRBindingsPythonCoreNoCAPI",
+    srcs = MLIR_PYTHON_BINDINGS_SOURCES,
+    # These flags are needed for pybind11 to work.
+    copts = [
+        "-fexceptions",
+        "-frtti",
+    ],
+    features = [
+        # Cannot use header_modules (parse_headers feature fails).
+        "-use_header_modules",
+    ],
     tags = [
         "manual",  # External dependency
         "nobuildkite",  # TODO(gcmn): Add support for this target
     ],
     deps = [
-        ":CAPIAsync",
-        ":CAPIDebug",
-        ":CAPIGPU",
-        ":CAPIIR",
-        ":CAPIInterfaces",
-        ":CAPILinalg",
-        ":CAPIRegistration",
-        ":CAPISparseTensor",
+        ":CAPIAsyncHeaders",
+        ":CAPIDebugHeaders",
+        ":CAPIGPUHeaders",
+        ":CAPIIRHeaders",
+        ":CAPILinalgHeaders",
+        ":CAPIRegistrationHeaders",
+        ":CAPISparseTensorHeaders",
+        ":MLIRBindingsPythonHeaders",
+        "//llvm:Support",
+        "@pybind11",
+        "@python_runtime//:headers",
     ],
 )
 
+# Target that bundles together the CAPI objects needed for
+# MLIRBindingsPythonCoreNoCAPI.
 cc_library(
     name = "MLIRBindingsPythonCAPIObjects",
     tags = [
         "manual",  # External dependency
         "nobuildkite",  # TODO(gcmn): Add support for this target
     ],
     deps = [
         ":CAPIAsyncObjects",
         ":CAPIDebugObjects",
         ":CAPIGPUObjects",
         ":CAPIIRObjects",
         ":CAPIInterfacesObjects",
         ":CAPILinalgObjects",
         ":CAPIRegistrationObjects",
         ":CAPISparseTensorObjects",
     ],
 )
 
-cc_library(
-    name = "MLIRBindingsPythonCore",
-    tags = [
-        "manual",  # External dependency
-        "nobuildkite",  # TODO(gcmn): Add support for this target
-    ],
-    deps = [
-        ":MLIRBindingsPythonCAPIDeps",
-        ":MLIRBindingsPythonCoreNoCAPI",
-    ],
-)
-
 PYBIND11_COPTS = [
     "-fexceptions",
     "-frtti",
 ]
 
 PYBIND11_FEATURES = [
     # Cannot use header_modules (parse_headers feature fails).
     "-use_header_modules",