Skip to content
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

ci: Fix llvm static linking on macOS CI #10879

Merged
merged 1 commit into from
Oct 2, 2022

Conversation

nirbheek
Copy link
Member

@nirbheek nirbheek commented Oct 1, 2022

Downgrade to LLVM-14 from LLVM-15 which is somewhat broken when using
static linking at present:

Homebrew/discussions#3666 (comment)

We can't use LLVM's lld instead of ld on macOS because we don't detect
it as an Apple linker and pass --as-needed etc to it. Even when that
is fixed and we set -lto_library etc correctly, the linker just hangs.

The LLVM@15 test is shared-only now and moved to the qt4 macOS job.

@nirbheek nirbheek added this to the 0.63.4 milestone Oct 1, 2022
@nirbheek nirbheek requested a review from jpakkane as a code owner October 1, 2022 23:23
@codecov
Copy link

codecov bot commented Oct 1, 2022

Codecov Report

Merging #10879 (5a166f9) into master (a58ec32) will decrease coverage by 6.90%.
The diff coverage is 87.71%.

@@            Coverage Diff             @@
##           master   #10879      +/-   ##
==========================================
- Coverage   68.09%   61.19%   -6.91%     
==========================================
  Files         404      203     -201     
  Lines       86713    42417   -44296     
  Branches    19209     8753   -10456     
==========================================
- Hits        59048    25955   -33093     
+ Misses      23190    14436    -8754     
+ Partials     4475     2026    -2449     
Impacted Files Coverage Δ
mesonbuild/backend/backends.py 79.24% <ø> (-5.05%) ⬇️
mesonbuild/backend/vs2010backend.py 0.00% <0.00%> (ø)
mesonbuild/utils/platform.py 83.33% <ø> (ø)
mesonbuild/utils/posix.py 100.00% <ø> (ø)
mesonbuild/utils/win32.py 0.00% <ø> (ø)
mesonbuild/scripts/meson_exe.py 71.08% <33.33%> (-6.03%) ⬇️
mesonbuild/mesonlib.py 60.00% <66.66%> (ø)
mesonbuild/utils/core.py 97.61% <97.61%> (ø)
mesonbuild/dependencies/dub.py 6.83% <100.00%> (-0.40%) ⬇️
mesonbuild/interpreter/type_checking.py 70.05% <100.00%> (ø)
... and 301 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@nirbheek nirbheek changed the title ci: Fix llvm detection on macOS CI WIP: ci: Fix llvm detection on macOS CI Oct 2, 2022
@eli-schwartz
Copy link
Member

eli-schwartz commented Oct 2, 2022

LLVM is detected and used just fine -- the problem is that the static archive contains contents which are effectively invalid (the toolchain errors out claiming that it has the wrong "producer").

This occurs only for static linkage -- it's a bug in homebrew's most recent LLVM update, I figured out the problem when I stumbled across a cmake project that had the same error.

A Homebrew developer (@carlocab) stated that it's intentional, because making the static libraries be bitcode-only produces speedups for the shared use case due to the intricacies of the LLVM build system. Or something. But there's a thought about post-processing that to make non-bitcode archives which can be generically linked against.

@nirbheek
Copy link
Member Author

nirbheek commented Oct 2, 2022

@eli-schwartz yes, I figured that once I saw the test failure again. According to Homebrew/discussions#3666 (comment), we should use lld but that fails because we don't detect it as an Apple linker. This PR now just does a downgrade to llvm-14, but I think not being able to detect an LLD that ships with LLVM targeting macOS as an Apple linker is an actual bug in Meson.

@nirbheek
Copy link
Member Author

nirbheek commented Oct 2, 2022

I think not being able to detect an LLD that ships with LLVM targeting macOS as an Apple linker is an actual bug in Meson.

With that fixed, it hangs while linking (statically to llvm). Very weird behaviour... I think I will add a separate job that only tests 15 llvm without static linking. Maybe club it with the qt4 one.

Opening a separate PR for the linker detection: #10880

@nirbheek nirbheek removed this from the 0.63.4 milestone Oct 2, 2022
@nirbheek nirbheek changed the title WIP: ci: Fix llvm detection on macOS CI ci: Fix llvm detection on macOS CI Oct 2, 2022
Downgrade to LLVM-14 from LLVM-15 which is somewhat broken when using
static linking at present:

Homebrew/discussions#3666 (comment)

We can't use LLVM's lld instead of ld on macOS because we don't detect
it as an Apple linker and pass --as-needed etc to it. Even when that
is fixed and we set -lto_library etc correctly, the linker just hangs.

LLVM@14 is keg-only, so we need to add CPPFLAGS / LDFLAGS to the keg
subdir inside /usr/local

The LLVM@15 test is shared-only now and moved to the qt4 macOS job.
@carlocab
Copy link
Contributor

carlocab commented Oct 2, 2022

Yep, working on a fix, but I'll need a little more time. Sorry for the pain.

In the meantime, using llvm@14 should be a fine workaround.

@nirbheek
Copy link
Member Author

nirbheek commented Oct 2, 2022

Yep, working on a fix, but I'll need a little more time. Sorry for the pain.

It's fine, comes with the territory. If you want to test with meson, you'll probably want #10880.

@nirbheek nirbheek added this to the 0.63.3 milestone Oct 2, 2022
@nirbheek nirbheek changed the title ci: Fix llvm detection on macOS CI ci: Fix llvm static linking on macOS CI Oct 2, 2022
@nirbheek nirbheek added OS:macos Issues specific to Apple Operating Systems like MacOS and iOS CI Continuous integration: Travis/Appveyor/Actions dynamic linkers Dynamic linkers (ld, link, lld-link, etc) labels Oct 2, 2022
@nirbheek
Copy link
Member Author

nirbheek commented Oct 2, 2022

This is blocking the 0.63.3 stable release, so I am merging this now to unblock myself.

@nirbheek nirbheek merged commit c20fb65 into mesonbuild:master Oct 2, 2022
@nirbheek nirbheek deleted the fix-macos-ci-llvm branch October 2, 2022 03:36
@eli-schwartz
Copy link
Member

Well, this is officially LGTM anyway. ;)

@carlocab
Copy link
Contributor

carlocab commented Oct 2, 2022

Fix in Homebrew/homebrew-core#112154.

carlocab added a commit to carlocab/homebrew-core that referenced this pull request Oct 2, 2022
In Homebrew#106925, I enabled LTO for our LLVM build. This creates static
archives that contain LLVM bitcode instead of object code. This makes
the static archives more difficult to use, requiring workarounds such as

    https://github.com/Homebrew/homebrew-core/blob/c01f1794fc3decce04b71cae03966213fc7af34d/Formula/enzyme.rb#L30

and has caused problems for multiple downstream projects.

We can fix this by converting the bitcode into object code, which is
what Fedora does with their LLVM build. They also build their toolchain
with LTO.

Alternatively, we can disable LTO, but that foregoes significant
speedups we get from enabling it.

While we're here, let's add some test coverage for features that were
recently enabled that we don't test.

Fixes:

ziglang/zig#12923
halide/Halide#7055
mesonbuild/meson#10879
Homebrew/discussions#3666
BrewTestBot pushed a commit to Homebrew/homebrew-core that referenced this pull request Oct 3, 2022
In #106925, I enabled LTO for our LLVM build. This creates static
archives that contain LLVM bitcode instead of object code. This makes
the static archives more difficult to use, requiring workarounds such as

    https://github.com/Homebrew/homebrew-core/blob/c01f1794fc3decce04b71cae03966213fc7af34d/Formula/enzyme.rb#L30

and has caused problems for multiple downstream projects.

We can fix this by converting the bitcode into object code, which is
what Fedora does with their LLVM build. They also build their toolchain
with LTO.

Alternatively, we can disable LTO, but that foregoes significant
speedups we get from enabling it.

While we're here, let's add some test coverage for features that were
recently enabled that we don't test.

Fixes:

ziglang/zig#12923
halide/Halide#7055
mesonbuild/meson#10879
Homebrew/discussions#3666

Closes #112154.

Signed-off-by: Sean Molenaar <1484494+SMillerDev@users.noreply.github.com>
Signed-off-by: BrewTestBot <1589480+BrewTestBot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration: Travis/Appveyor/Actions dynamic linkers Dynamic linkers (ld, link, lld-link, etc) OS:macos Issues specific to Apple Operating Systems like MacOS and iOS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants