Macaulay2 / M2

The primary source code repository for Macaulay2, a system for computing in commutative algebra, algebraic geometry and related fields.
https://macaulay2.com
343 stars 231 forks source link

hardcoded paths in M2/cmake/FindReadline.cmake #2218

Open haerski opened 3 years ago

haerski commented 3 years ago

I attempted to compile Macaulay2 on a Gentoo Prefix, which allows easy installation of packages in a "prefix", i.e. some directory outside the system root.

The compilation failed because readline library ended up being pulled from the base system instead of the prefix. The culprit were the following lines. Note the hardcoded paths. https://github.com/Macaulay2/M2/blob/3c3e3f7ddbc1a0ff20bd6723e41251557dea2936/M2/cmake/FindReadline.cmake#L24-L28

Letting CMake use its default lookup paths seems to do the trick. See below for a change that worked for me

find_path(READLINE_ROOT_DIR
    NAMES include/readline/readline.h
    PATHS ${HOMEBREW_PREFIX}/opt/readline
)

My experience with CMake is very limited, so I'm not sure if this breaks builds on different systems. @mahrud ?

mahrud commented 3 years ago

I think the NO_DEFAULT_PATH there was intentional to avoid CMake from picking the wrong directory on some systems.

Could you try the following alternative solutions first:

  1. Setting -DREADLINE_ROOT_DIR=... in your first CMake call.
  2. Setting -DCMAKE_PREFIX_PATH=... in your first CMake call.
  3. Setting -DCMAKE_SYSTEM_PREFIX_PATH=... in your first CMake call.

If neither of these work, we can still make other changes to make the first solution work, but I'm not sure whether that's preferable over your PR, if we're only relying on my memory that another system breaks with your PR.

Would be good if someone could try running CMake from your PR on a Mac to see which readline it finds.

haerski commented 3 years ago

Some results:

  1. Works!
  2. Doesn't work (wrong readline found)
  3. Doesn't work (wrong readline found)

I figured the PR may have been a bit naive. If readline is being fussy, I'm willing to live with solution 1, although it is a bit strange that FindReadline.cmake is the only one among all the Find*.cmake that needs hardcoded paths. In fact, looking at a few of them, it seems many of them are of the form PATH ${INCLUDE_INSTALL_DIR}, PATH ${CMAKE_INSTALL_PREFIX}, PATH ${LIB_INSTALL_DIR} etc... Maybe something like that could work for readline too?

mahrud commented 3 years ago

Readline and History are particularly problematic because Macs have a built-in version that sometimes has issues, iirc, so they both use the READLINE_ROOT_PATH variable.

If you can test a simpler solution on a Mac system and it works, then I'd be in favor of simplifying it.

I should note that if you're willing to spend time on this, it might be better to try switching to libedit instead, see #825.

Also, I'd appreciate it if you could add an entry to the CMake Build Problems wiki page with a link to this issue+solution, in case anyone else has a similar problem.

haerski commented 3 years ago

I see, thanks for the response! I don't have a Mac so I can't test it on one.

I'll add an entry to the wiki!