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

Wed Nov 10 14:23:50 PST 2021

phawkins added inline comments.

================
Comment at: utils/bazel/llvm-project-overlay/mlir/BUILD.bazel:774
+    ],
+    alwayslink = True,
 )
----------------
GMNGeoffrey wrote:
> phawkins wrote:
> > mehdi_amini wrote:
> > > GMNGeoffrey wrote:
> > > > 😢 alwayslink is a scourge. Is it really the only way?
> > > Same concern here: why is this needed?
> > I removed it from this target, where I don't think it's needed. In general it *is* needed on all the libraries that implement the exported C API, because otherwise Bazel will strip the C API out.
> Why wasn't it needed before? We're using this internally without issue IIUC. We've worked hard in many places to scour this from the codebase, so I don't love seeing it return
> 
> @stellaraccident can you weigh in on these build rules?
It wasn't needed before, because internally we build monolithic binaries.

Here I'm doing something different: I'm building one shared library that vends the C API (capi.so) and another shared library (a Python extension) that consumes it. Without `alwayslink`, the C API `.so` file will not contain the C API: it ends up completely empty because there are no *users* of the API present and everything ends up stripped out. I did not put this here idly.

I note this is exactly how IREE deploys things, although IREE does it with CMake not Bazel.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113565