- User Since
- Aug 12 2020, 12:14 AM (63 w, 1 d)
Jun 4 2021
Is @tra able to land this? I'm not
Jun 3 2021
Accepting this, with the comment that I think it's better to follow up with a patch that reverts autodetecting hip from clang based on the spack directory structure.
May 24 2021
May 22 2021
I've created a pull request to spack here: https://github.com/spack/spack/pull/23859, hopefully that's enough to revert this patch.
Feb 24 2021
I don't have commit access, would be great if you could do that for me!
Should this be merged?
Jan 28 2021
I dropped the awk bits, since there's no portable way to do it there, and replaced all perl -w with perl and use warnings.
Jan 27 2021
@JDevlieghere, would you care to review? This is another instance of moving away from system binaries to whatever the user / package manager wants the build to use.
Jan 21 2021
Sep 15 2020
Also note that the changes are not likely to be backported to 11 even: https://reviews.llvm.org/rG31e5f7120bdd2f76337686d9d169b1c00e6ee69c#942622.
Aug 30 2020
Another take on this isuse, incorporating @phosek's comment on making find_library not find shared libs when compiling with LLVM_BUILD_STATIC=ON, and making sure that get_system_libname does not add empty items to a regex capture group, as CMake's regex engine does not support this.
Aug 27 2020
@phosek it would be great to get that patch in, but the truth is it is difficult testing across multiple platforms for me; I can only test on Linux and macOS, not Windows (GNU). Since that patch would touch predefined, platform-dependent cmake variables, I would want to test it properly before submitting a patch, which might take a bit more time. So for me it would be best to temporarily accept this reverting patch, and submit a properly tested patch afterwards.
Aug 25 2020
@gkistanova It's true that this change has lead to more issues I could ever imagine, but I think the link you provided is the last remaining problem.
Aug 24 2020
@mati865 can you test this on your platform? You only have to run cmake ../llvm and check whether tools/llvm-config/BuildVariables.inc: has a proper #define LLVM_SYSTEM_LIBS "-lrt -ldl -lpthread -lm -lz -ltinfo"
Aug 21 2020
@phosek do you know what is required to backport this as well as the ncurses detection to the LLVM 11 release branch?
Aug 19 2020
@aeubanks and @phosek: see https://reviews.llvm.org/D86245 for a last fix that checks whether CMAKE_FIND_LIBRARY_PREFIXES and CMAKE_FIND_LIBRARY_SUFFIXES are non-empty before applying the regex to fix this window issue
Aug 18 2020
Move the output variable to the end and use lowercase for arguments.
Aug 17 2020
Aug 16 2020
DRY in find_library
Aug 14 2020
use REQUIRED when FORCE_ON
Aug 13 2020
Use LLVM_ENABLE_TERMINFO and make it respect FORCE_ON.
Aug 12 2020
I also considered set(CURSES_NEED_NCURSES true) and then find_package(Curses), but indeed CURSES_LIBRARIES is the only thing to work with, which lists too many libraries. I think find_library is cleanest. Thanks for the LLVM_ENABLE_TERMINFO, I can add that change.
Simplified detection a bit
Great, one benefit of this is that zlib can now be detected in non-system libs. Maybe we should handle ncurses / TERMINFO in a similar manner? It currently has similar logic as finding zlib had