-
-
Notifications
You must be signed in to change notification settings - Fork 12.8k
zig 0.14.0: re-enable static llvm for macos only #210173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This likely just silences the warnings, and doesn't fix the underlying issue. Note that we still have dynamic linkage to LLD:
And LLD itself links dynamically to libLLVM and libc++:
So the problem of loading possibly conflicting versions of libc++ remains. To add to that, this also introduces the possibility of loading conflicting versions of
libLLVM
. One version is the one that is statically linked, and the other is the one loaded by LLD.The only real fix here is to allow
zig
itself to link dynamically to the system libc++ instead of forcing linkage to a bundled static version.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand, but as I have already specified this is only a workaround and, from some research I have done, this would seem to be a long-standing problem and not yet solved or at least made known to zig itself (https://ziggit.dev/t/linking-stage3-zig-dynamically-with-system-libc/5726).
I have also seen how Macports solves this problem and they too have implemented the same workaround:
(https://github.com/macports/macports-ports/blob/master/lang/zig/Portfile)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you report it to Zig then and see what their opinion is on the fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand the temptation to blindly follow what MacPorts does here, but their workaround is not applicable for us.
MacPorts almost certainly does not ship shared libraries for LLD, and so Zig can only link with the static versions. That makes using
ZIG_STATIC_LLVM
somewhat safe, because the static libraries will not load a differentlibLLVM
orlibc++
.However, we do not ship the LLD static libraries, and only ship the shared ones. As a consequence, using
ZIG_STATIC_LLVM
here will still result in your loading two different versions oflibc++
(and possibly two different versions oflibLLVM
). This is not safe, and is exactly what the warning you're currently seeing is about.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SMillerDev I just reported to zig: ziglang/zig#23189.
Let's hope to get an answer as soon as possible but I don't think there will be a definitive answer given these discussions:
ziglang/zig#23127
ziglang/zig#20450
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just want to add that I don't endorse this statement. Zig 0.14.0 has these build dependencies so of course we provide support for building Zig 0.14.0 from source with these dependencies.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I looked into this, build.zig didn't allow linking to system libc++ on macOS (ziglang/zig#20450 (comment)).
mod.link_libcpp = true
(andmod.linkSystemLibrary("c++", .{});
) link the bundled libc++.a, which is visible in logs when building with-DZIG_EXTRA_BUILD_ARGS=--verbose;--verbose-link
, e.g. (with some flags collapsed into "..." for readability)A quick hack I tried was patching
mod.link_libcpp = true
with something liketest/link/macho.zig
1 but this won't be correct if LLVM was linked to another libc++:Footnotes
https://github.com/ziglang/zig/blob/master/test/link/macho.zig#L886 ↩
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd love to do that, and that's exactly what we've been trying to do in the first place. Unfortunately there does not appear to be a way to tell Zig to use the system-provided libc++ without hacking up the source.
See also ziglang/zig#23189.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please take this patch from @mikdusan for a spin with shared LLVM/LLD: ziglang/zig#23264
Would be nice if someone could also ping the relevant folks from MacPorts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks; have opened #211129. Have also left a link to ziglang/zig#23264 at ziglang/zig#20450 and macports/macports-ports#24430 for the MacPorts folks.