Skip to content

Commit

Permalink
Use FindCurses instead of pkg_check_modules.
Browse files Browse the repository at this point in the history 
This should resolve #217.
  • Loading branch information
@brndnmtthws
brndnmtthws committed Jan 23, 2018
1 parent 29f1c05 commit abd0be5
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions cmake/ConkyPlatformChecks.cmake
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,12 +129,12 @@ if(BUILD_HTTP)
endif(BUILD_HTTP)

if(BUILD_NCURSES)
pkg_check_modules(NCURSES ncurses)
if(NOT NCURSES_FOUND)
include(FindCurses)
if(NOT CURSES_FOUND)
message(FATAL_ERROR "Unable to find ncurses library")
endif(NOT NCURSES_FOUND)
set(conky_libs ${conky_libs} ${NCURSES_LIBRARIES})
set(conky_includes ${conky_includes} ${NCURSES_INCLUDE_DIRS})
endif(NOT CURSES_FOUND)
set(conky_libs ${conky_libs} ${CURSES_LIBRARIES})
set(conky_includes ${conky_includes} ${CURSES_INCLUDE_DIR})
endif(BUILD_NCURSES)

if(BUILD_MYSQL)
Expand Down

4 comments on commit abd0be5

@billie80
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like with this change issue #167 pops up again as FindCurses does not seem to handle a separate libtinfo :-(

See Gentoo bug for reference https://bugs.gentoo.org/648090

@brndnmtthws
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, any thoughts on how to handle this? How do other Gentoo packages deal with it?

@billie80
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Essentially this seems to be a bug in FindCurses not handling this situation correctly. There must be a bug somewhere because when looking into it there is a check for tinfo. Funny this check was added after being detected by a Gentoo bug (https://gitlab.kitware.com/cmake/cmake/commit/1f646c6ce0766f8ab59868e7cac24034e6966504).

The Gentoo solution itself was to rely on pkg-config (https://bugs.gentoo.org/457530). However it seems that this approach does not work in any case. As I got it the problem is that ncurses made the installation of pkg-config files optional. Probably the reporter of this bug did use a distribution not shipping these files. For binary distributions these may not that useful anyway. On the contrary for source based distributions like Gentoo and when trying to build packages from source on a binary distribution this files are useful as this bug cleary shows.

The proper solution would be to check if either the ncurses or tinfo library provides a certain function. For autottools this seems to be the AC_SEARCH_LIBS macro (AC_SEARCH_LIBS([cbreak], [tinfo ncurses])) see comment 12 from the above Gentoo bug. There are also references of cbreak and tinfo in the FindCurses routine which also suggests that there is something wrong in FindCurses.

I see you have already defined AC_SEARCH_LIBS in cmake/Conky.cmake. Maybe you can make use of it here too. Or other native functions like CHECK_{LIBRARY,FUNCTION,SYMBOL}_EXISTS of cmake could be of help.

What I also don't know if conky only needs functionality from tinfo so if ncurses is built with a separate tinfo only linking to tinfo is required instead of linking to ncurses and tinfo.

@billie80
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the moment I reverted this change to use pkgconfig again and I actually suggest you to do the same.

Rationale is that I don't think FindCurses will be fixed very soon. There are already bugs [1], [2] open for some time but the developers don't seem to have the resources to fix this or do not really care.

So until this is not fixed properly in cmake it is probably easier to fix this on the distribution level by installing the pkg-config files or to convince the developers of ncurses to install them by default.

[1] https://gitlab.kitware.com/cmake/cmake/issues/16354
[2] https://gitlab.kitware.com/cmake/cmake/issues/17405

Please sign in to comment.