-
Notifications
You must be signed in to change notification settings - Fork 727
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
CMake: improve ncurses detection; separate TINFO_LIBRARY input #1356
base: main
Are you sure you want to change the base?
Conversation
ccf739a
to
f8817e2
Compare
I found that on macOS, setting the libncurses.a and libtinfo.a into the same CURSES_LIBRARY variable does not find or link with libtinfo.a. To fix this, this commit adds a separate TINFO_LIBRARY variable. In the end, CURSES_LIBRARIES and the Curses::curses CMake TARGET will still have both libraries set, if both are provided. This fix is necessary if the ncurses was built with `--with-terminfo`. I think we got away without it on Linux because of pkg-config.
f8817e2
to
13771c9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found one other issue when re-testing this -- the summary details for the ncurses dependency is showing tinfo.a twice.
I ran this:
❯ LDFLAGS="-Wl,--copy-dt-needed-entries" cmake .. \
-D CMAKE_FIND_PACKAGE_PREFER_CONFIG=TRUE \
-D CMAKE_INSTALL_PREFIX=install \
-D CMAKE_PREFIX_PATH="$HOME/.mussels/install/host-static" \
-D CMAKE_MODULE_PATH="$HOME/.mussels/install/host-static/lib/cmake" \
-D JSONC_INCLUDE_DIR="$HOME/.mussels/install/host-static/include/json-c" \
-D JSONC_LIBRARY="$HOME/.mussels/install/host-static/lib/libjson-c.a" \
-D ENABLE_JSON_SHARED=OFF \
-D BZIP2_INCLUDE_DIR="$HOME/.mussels/install/host-static/include" \
-D BZIP2_LIBRARY_RELEASE="$HOME/bzip2-1.0.8-install/lib/libbz2.a" \
-D OPENSSL_ROOT_DIR="$HOME/.mussels/install/host-static" \
-D OPENSSL_INCLUDE_DIR="$HOME/.mussels/install/host-static/include" \
-D OPENSSL_CRYPTO_LIBRARY="$HOME/.mussels/install/host-static/lib/libcrypto.a" \
-D OPENSSL_SSL_LIBRARY="$HOME/.mussels/install/host-static/lib/libssl.a" \
-D LIBXML2_INCLUDE_DIR="$HOME/.mussels/install/host-static/include/libxml2" \
-D LIBXML2_LIBRARY="$HOME/.mussels/install/host-static/lib/libxml2.a" \
-D PCRE2_INCLUDE_DIR="$HOME/.mussels/install/host-static/include" \
-D PCRE2_LIBRARY="$HOME/.mussels/install/host-static/lib/libpcre2-8.a" \
-D CURSES_INCLUDE_DIR="$HOME/.mussels/install/host-static/include" \
-D CURSES_LIBRARY="$HOME/.mussels/install/host-static/lib/libncurses.a" \
-D TINFO_LIBRARY="$HOME/.mussels/install/host-static/lib/libtinfo.a" \
-D ZLIB_INCLUDE_DIR="$HOME/.mussels/install/host-static/include" \
-D ZLIB_LIBRARY="$HOME/.mussels/install/host-static/lib/libz.a" \
-D LIBCHECK_INCLUDE_DIR="$HOME/.mussels/install/host-static/include" \
-D LIBCHECK_LIBRARY="$HOME/.mussels/install/host-static/lib/libcheck.a" \
-D DISABLE_MPOOL=OFF \
-D ENABLE_STATIC_LIB=OFF \
-D MAINTAINER_MODE=ON \
-D ENABLE_CLAMONACC=ON \
-D ENABLE_SHARED_LIB=ON \
-D BYTECODE_RUNTIME=interpreter \
-D CMAKE_BUILD_TYPE=Debug -D ENABLE_EXAMPLES=ON \
(#-D LLVM_ROOT_DIR="$HOME/llvm/12.x-dbg") && \
make -j12 && make install && ctest -V
And in the configure-summary, I see this:
-- Application Extra Dependencies --
GUI support:
ncurses /usr/include
/home/micah/.mussels/install/host-static/lib/libncurses.a;/home/micah/.mussels/install/host-static/lib/libtinfo.a;/home/micah/.mussels/install/host-static/lib/libtinfo.a
) | ||
|
||
include(FindPackageHandleStandardArgs) | ||
find_package_handle_standard_args(TINFO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I re-tested this after a rebase to double-check my work and encountered this warning:
-- Found CURSES: /home/micah/.mussels/install/host-static/lib/libncurses.a;/home/micah/.mussels/install/host-static/lib/libtinfo.a
CMake Warning (dev) at /home/micah/.local/lib/python3.8/site-packages/cmake/data/share/cmake-3.30/Modules/FindPackageHandleStandardArgs.cmake:441 (message):
The package name passed to `find_package_handle_standard_args` (TINFO) does
not match the name of the calling package (CURSES). This can lead to
problems in calling code that expects `find_package` result variables
(e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
cmake/FindCURSES.cmake:84 (find_package_handle_standard_args)
CMakeLists.txt:551 (find_package)
This warning is for project developers. Use -Wno-dev to suppress it.
-- Found TINFO: /home/micah/.mussels/install/host-static/lib/libtinfo.a
That's... not great. It implies the correct approach is to make a separate FindTINFO.cmake
file. There is probably an alternative solution. I might investigate how CMake's own FindOpenSSL.cmake
does it considering that it finds libssl and libcrypto, neither of which are named "OpenSSL".
I found that on macOS, setting the libncurses.a and libtinfo.a into the same CURSES_LIBRARY variable does not find or link with libtinfo.a.
To fix this, this commit adds a separate TINFO_LIBRARY variable. In the end, CURSES_LIBRARIES and the Curses::curses CMake TARGET will still have both libraries set, if both are provided.
This fix is necessary if the ncurses was built with
--with-terminfo
. I think we got away without it on Linux because of pkg-config.