Look for for homebrew packages in the right places - #3986
Conversation
| # TODO: replace gdbm, see https://github.com/Macaulay2/M2/issues/594 | ||
| find_package(GDBM REQUIRED QUIET) # See FindGDBM.cmake | ||
|
|
||
| list(APPEND CMAKE_PREFIX_PATH "$(brew --prefix libomp)") |
There was a problem hiding this comment.
I didn't read what you're doing in the autotools build but this should certainly not be hardcoded. It's up to the user to add brew directories to the path or not, and further there's a reason keg-only bottles are keg-only! e.g this may break depending on whether you build with GCC or clang.
There was a problem hiding this comment.
My goal was to make it simpler for the user with a reasonable set of default search paths so that the build should work out of the box most of the time on common systems without fiddling with flags. And we're already hardcoding several other brew paths in the cmake build: 50ea574
There was a problem hiding this comment.
Those are hints for standard libraries without multiple implementations, which are used only if a library wasn't found elsewhere on the path. Appending the prefix directory of libomp can determine whether the compiler uses libomp or libgomp.
There was a problem hiding this comment.
Ok, that makes sense -- I'll remove the cmake changes
There was a problem hiding this comment.
Side note -- do we even need to deal with OpenMP on most builds? According to a comment in configure.ac:
This is required for building the library csdp and good for building the library normaliz
Most of the time, we'll be using existing system packages for these.
There was a problem hiding this comment.
fflas-ffpack, which is a header-only library, uses it, and if it's not found correctly you get a warning when building M2:
#warning "openmp was not detected correctly at configure time, please report this bug"
Fixing this on all homebrew builds was quite a headache, and I think at some point all cmake builds here were also working correctly but I see that the ubuntu one is failing now.
306b151 to
c6884a1
Compare
Let the build systems figure them out!
|
the stock AC_OPENMP does not work on maOS, as it doesn't test for the needed flag combo Fixing it alone is not sufficient - one needs a fixed libtool (2.6.1, the latest, does contain a fix I submitted a while ago) |
|
We do have our own version of |
|
that's a bit of misnaming, IMHO, to start your macro names with AC_ And no GNU libtool issues, as it's not used? |
That way it's clear that it's not the upstream autoconf version.
Good point. I just added a commit updating it to
Not that I've seen. We don't use libtool to build M2 itself, but we do for many of its dependencies. There might be some edge case where we build one of the dependencies that needs both libtool and openmp in macOS using some older libtool that doesn't have your patch. But most of the time, we don't need to build dependencies in macOS anyway since they're available via Homebrew. (The dependency builds are mostly for RHEL, where not much is packaged.) |
Is cmake still missing openmp? |
Out of the box on macOS, yes. It needs the correct incantation of flags to find the brew libomp |
|
libomp lives in |
4 participants
We add various homebrew package directories to the appropriate flags so that we can detect them at the beginning of the build without needing to set any variables ahead of time. The cmake build was already doing a pretty good job of this (it was just missing openmp), and we bring autotools in line with it.
This is on top of #3984 so that the macOS builds will actually work.
Cc: @mikestillman