macos stage3: add link support for system libc++ #23264
Conversation
There was a problem hiding this comment.
Thanks; works great. Shipped in Homebrew as Homebrew/homebrew-core#211129.
build.zig
Outdated
| const sdk = std.zig.system.darwin.getSdk(b.allocator, b.graph.host.result) orelse return error.SdkDetectFailed; | ||
| const @"libc++" = b.pathJoin(&.{ sdk, "usr/lib/libc++.tbd" }); | ||
| exe.root_module.addObjectFile(.{ .cwd_relative = @"libc++" }); |
There was a problem hiding this comment.
Note that you are implicitly assuming that the shared LLVM is linked against the system libc++. That's likely generally true in practice, but in theory one can build their own LLVM and link it against a non-system libc++.
I'm not suggesting that you need to support an LLVM built with a custom libc++. However, it may be worth documenting somewhere that you only support linking to a shared LLVM with the system libc++.
There was a problem hiding this comment.
Thank you; good advice. I think we will go the route of documenting requirement for only one copy of libc++. This problem will fully go away with #16270.
| if (static or !std.zig.system.darwin.isSdkInstalled(b.allocator)) { | ||
| mod.link_libcpp = true; |
There was a problem hiding this comment.
Per my comment here, it's probably still worth rethinking linking to the vendored libc++ when you are using a third-party LLVM (i.e. one built not using the vendored libc++).
|
Patch pushed to MacPorts: macports/macports-ports@eec7cd1 |
- activates when -DZIG_SHARED_LLVM=ON - activates when llvm_config is used and --shared-mode is shared - otherwise vendored libc++ is used closes ziglang#23189
|
| // Avoid using `mod.linkSystemLibrary()`, which: | ||
| // - is semantically equivalent to `-lc++` | ||
| // - and enables `mod.link_libcpp` | ||
| // Instead, add the full object pathname. |
There was a problem hiding this comment.
I see, thank you. This reveals a contradiction in this implementation:
- by checking for the string "c++",
linkSystemLibraryreveals an assumption that there is only one possible way to link libc++. - The entire point of this patch rests on the assumption that there are multiple ways to link libc++.
Instead of working around it, the build system should be modified to recognize that there are two choices when linking libc++, because the logic that makes my code snippet not work for this use case is based on this incorrect assumption.
Then, my simpler code snippet which does not rely on something that requires access outside of a build sandbox (#14286) will work correctly.
closes #23189