Add MRBIND_LINK_CLANG_CPP_DYLIB opt-in for smaller mrbind on macOS#35
Open
Fedr wants to merge 4 commits into
Open
Add MRBIND_LINK_CLANG_CPP_DYLIB opt-in for smaller mrbind on macOS#35Fedr wants to merge 4 commits into
Fedr wants to merge 4 commits into
Conversation
Adds a new CMake option, off by default. When ON, mrbind links against `libclang-cpp.dylib` + `libLLVM.dylib` instead of pulling clang/LLVM in statically via `clangTooling`. The resulting binary is much smaller -- single shared lib resolved at runtime vs. the entire frontend in the exe -- which is useful for CI caches. Not enabled by default because the existing comment documents that on apt-clang/Ubuntu the dynamic path crashes at startup with `LLVM ERROR: inconsistency in registered CommandLine options` (duplicate `cl::opt` registrations between libclang-cpp.so and the granular libs that still slip into the link line). Homebrew's `llvm@N` on macOS is built with `LLVM_LINK_LLVM_DYLIB=ON` and doesn't have that problem, so consumers building on macOS can pass `-DMRBIND_LINK_CLANG_CPP_DYLIB=ON` to opt in. The fallback for platforms where `clangTooling` isn't exposed (Arch, Rocky) is unchanged -- they continue to take the dynamic path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
…'t leak
Linking `mrbind` against `clang-cpp` on macOS (Homebrew llvm@N) broke
`<cstddef>` at compile time:
/opt/homebrew/opt/llvm@N/.../c++/v1/cstddef:46: error: <cstddef> tried
including <stddef.h> but didn't find libc++'s <stddef.h> header.
Cause: the `clang-cpp` CMake target's `INTERFACE_INCLUDE_DIRECTORIES`
contains `/opt/homebrew/opt/llvm@N/include`, and `target_link_libraries`
propagates that as a `-I` to every `mrbind` source file. That dir
shadows libc++'s wrapper for `<stddef.h>` and the include chain breaks.
Wrap the dynamic-link targets in `$<LINK_ONLY:...>` so they appear on
the link line but their interface properties (include dirs, compile
defs, etc.) are not propagated to `mrbind`. The explicit
`target_include_directories(mrbind PRIVATE ${LLVM_INCLUDE_DIR}
${CLANG_INCLUDE_DIR})` above already provides the include paths the
parser actually needs, so dropping the propagation is safe.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…agation The previous attempt wrapped `clang-cpp` / `LLVM` / `LLVMSupport` in `$<LINK_ONLY:...>` hoping that would strip their usage requirements, but CI still failed at libc++ `<cstddef>`: the imported targets' INTERFACE properties (and/or some equivalent compile-time hook) are still leaking into mrbind's compile commands somehow. Sidestep the whole machinery: `find_library` the libs by name and link to the absolute paths. CMake adds raw-path link libraries to the link line directly, with no INTERFACE_* propagation, no usage requirements. mrbind already explicitly adds the include paths it needs via `target_include_directories`, so dropping any reliance on the imported targets here is safe. `LLVM` and `LLVMSupport` are now both optional (the comment above already documented "`LLVM` part is only needed on Rocky"). They're linked only when found, matching the existing platform-specific shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Restructure so the OFF branch (the default) preserves the original `if (TARGET clangTooling) ... else() clang-cpp LLVM LLVMSupport` shape verbatim, including its original Ubuntu-CommandLine-error comment. The new find_library-based dynamic path is now reachable only via the explicit `MRBIND_LINK_CLANG_CPP_DYLIB=ON` opt-in. No behaviour change for existing consumers (no change when the option is OFF, no change to Arch/Rocky who hit the fall-through implicitly). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
mrbind's parser binary is currently ~30 MiB stripped on macOS / Ubuntu / Windows-MSYS2 because it statically links the granularclangTooling/ friends. On Rocky Linux it's ~5 MiB stripped because the local LLVM cmake config doesn't exposeclangToolingand we fall through to theclang-cpp + LLVM(dynamic) branch.For consumers caching
thirdparty/mrbind/buildacross CI runs (e.g. MeshLib/build-mrbind), the per-platform cache footprint is ~12 MiB compressed where the static path is used vs ~2 MiB on Rocky. Across the matrix that's the difference between ~90 MiB and ~20 MiB of cache.What
Add a new CMake option,
MRBIND_LINK_CLANG_CPP_DYLIB, off by default. When ON, link mrbind againstlibclang-cpp+libLLVM(single shared lib resolved at runtime) instead ofclangTooling. The fallback for platforms whereclangToolingisn't exposed (Arch, Rocky) is unchanged — they continue to take the dynamic path implicitly.The OFF branch keeps the old
clangTooling/clang-cpp + LLVM + LLVMSupportlogic untouched. The ON branch is written separately and usesfind_libraryto resolve to absolute library paths rather than linking through the imported targetsclang-cpp/LLVM/LLVMSupport— see "find_library vs imported targets" below.find_library vs imported targets
The naive shape
target_link_libraries(mrbind PRIVATE clang-cpp LLVM LLVMSupport)linked through Clang's imported targets and propagated theirINTERFACE_INCLUDE_DIRECTORIESinto mrbind's compile commands. On Homebrew'sllvm@Nthat includes/opt/homebrew/opt/llvm@N/include, which gets inserted before libc++'s wrapper headers and breaks the libc++<cstddef>include chain (<cstddef>ends up pulling LLVM-bundled<stddef.h>instead of the libc++ wrapper).find_libraryreturns an absolute path, so passing that totarget_link_librariesadds the lib to the link line without any usage-requirement propagation. mrbind already explicitly adds the include paths it needs above viaLLVM_INCLUDE_DIR/CLANG_INCLUDE_DIR.Why off by default
The existing comment in the file documents that on apt-clang/Ubuntu, the dynamic path crashed at startup:
This is duplicate
cl::optregistrations betweenlibclang-cpp.soand the granular libs that still slip into the link line on Debian-family LLVM packaging. Default-off preserves the current safe behaviour for everyone.When to turn it on
Consumers who know their LLVM was built with
LLVM_LINK_LLVM_DYLIB=ON(Homebrew'sllvm@Non macOS is a confirmed case) can pass-DMRBIND_LINK_CLANG_CPP_DYLIB=ONto their cmake invocation. The resulting binary depends onlibclang-cpp.dylib/libLLVM.dylibbeing resolvable at the same path at runtime, which is usually fine for CI but worth being explicit about.Test plan
install_mrbind_macos.shand goes green on all three macOS jobs (x64 Release,arm64 Release,arm64 Debug). Build MRBind, Generate C bindings, and Generate and build Python bindings all succeed; noinconsistency in registered CommandLine optionsand no libc++<cstddef>failures.