apache-arrow: switch to llvm@17 #169354
apache-arrow: switch to llvm@17 #169354
Conversation
github-actions
bot
added
the
boost
|
libunwind linkage is everywhere 😢 |
ZhongRuoyu
force-pushed
the
apache-arrow-llvm@17
branch
from
834cdf4
to
535a00a
Compare
ZhongRuoyu
added
CI-long-timeout
github-actions
bot
removed
the
CI-long-timeout
ZhongRuoyu
force-pushed
the
apache-arrow-llvm@17
branch
from
535a00a
to
068f50b
Compare
ZhongRuoyu
force-pushed
the
apache-arrow-llvm@17
branch
from
068f50b
to
1821128
Compare
ZhongRuoyu
force-pushed
the
apache-arrow-llvm@17
branch
from
1821128
to
a586036
Compare
| if DevelopmentTools.clang_build_version >= 1500 | ||
| recursive_dependencies | ||
| .select { |d| d.name.match?(/^llvm(@\d+)?$/) } | ||
| .map { |llvm_dep| llvm_dep.to_formula.opt_lib } | ||
| .each { |llvm_lib| ENV.remove "HOMEBREW_LIBRARY_PATHS", llvm_lib } | ||
| end |
There was a problem hiding this comment.
Should probably just handle this in brew at this point.
Though, dare I ask -- why don't we want linkage with our libunwind? As long as they're all using the same one it should be ok... (Though admittedly guaranteeing that may be a tall order)
There was a problem hiding this comment.
Should probably just handle this in
brewat this point.
Yea, I agree. Was looking into this too.
Though, dare I ask -- why don't we want linkage with our
libunwind? As long as they're all using the same one it should be ok... (Though admittedly guaranteeing that may be a tall order)
Because we cannot be sure the LLVM used to build the formula (or simply present at build time) will continue to stay around until the next rebuild/update -- unless it's a direct runtime dependency.
Some formulae have a build-only dependency on llvm. The linkage test would have failed should there be libunwind linkage. This was the case when this workaround was added to apache-arrow in #146072 though LLVM became a runtime dependency later.
Some others, like the dependents here, depend on llvm indirectly via another runtime dependency (apache-arrow in this case). This makes it unnecessarily challenging for us to switch the dependency formula to a different llvm. Even a "hello world" can link to libunwind unexpectedly.
$ cat test.cc
#include <iostream>
int main() {
std::cout << "hello, world\n";
}
$ LIBRARY_PATH="$(brew --prefix llvm)/lib" "$(brew --prefix llvm)/bin/clang++" test.cc
ld: warning: reexported library with install name '/usr/local/opt/llvm/lib/libunwind.1.dylib' found at '/usr/local/Cellar/llvm/17.0.6_1/lib/libunwind.1.0.dylib' couldn't be matched with any parent library and will be linked directly
$ otool -L ./a.out
./a.out:
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1700.255.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1345.100.2)
/usr/local/opt/llvm/lib/libunwind.1.dylib (compatibility version 1.0.0, current version 1.0.0)
There was a problem hiding this comment.
Should probably just handle this in
brewat this point.
Makes sense.
Though, dare I ask -- why don't we want linkage with our
libunwind? As long as they're all using the same one it should be ok... (Though admittedly guaranteeing that may be a tall order)
In current state, dependents are mixing libs and can end up crashing, e.g. #161342 (as I recall, there should be a couple other crashes fixed by unlinking llvm's libunwind)
There was a problem hiding this comment.
Even a "hello world" can link to libunwind unexpectedly.
$ cat test.cc #include <iostream> int main() { std::cout << "hello, world\n"; } $ LIBRARY_PATH="$(brew --prefix llvm)/lib" "$(brew --prefix llvm)/bin/clang++" test.cc ld: warning: reexported library with install name '/usr/local/opt/llvm/lib/libunwind.1.dylib' found at '/usr/local/Cellar/llvm/17.0.6_1/lib/libunwind.1.0.dylib' couldn't be matched with any parent library and will be linked directly $ otool -L ./a.out ./a.out: /usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1700.255.0) /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1345.100.2) /usr/local/opt/llvm/lib/libunwind.1.dylib (compatibility version 1.0.0, current version 1.0.0)
This is really weird. Smells like a bug in ld to me. Thoughts, @Bo98?
|
🥲 If this is the only failure I think I'd build |
This leads to undesired linkage with LLVM's `libunwind` (because it shadows the system's `libunwind`). See, for example, Homebrew/homebrew-core#169354.
github-actions
bot
removed
the
CI-long-timeout
github-actions
bot
added
the
CI-published-bottle-commits
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
❌ @ZhongRuoyu bottle request for rtabmap failed. Same failure. Gonna use the self-hosted runner instead. |
`pdal` opportunistically linked to LLVM's libunwind on Linux, causing a runtime dependency on it. Instead let's properly link it to `libunwind`. Discovered in #169354.
`pdal` opportunistically linked to LLVM's libunwind on Linux, causing a runtime dependency on it. Instead let's properly link it to `libunwind`. Discovered in #169354.
|
❌ @ZhongRuoyu bottle request for rtabmap failed.
|
HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>, where<formula>is the name of the formula you're submitting?brew test <formula>, where<formula>is the name of the formula you're submitting?brew audit --strict <formula>(after doingHOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>)? If this is a new formula, does it passbrew audit --new <formula>?Cherry-picked from #165206.