cgiboudeaux added inline comments. INLINE COMMENTS
> FindGLIB2.cmake:10 > +# True if the GLib2 library is available > +# ``GLIB2_INCLUDE_DIR`` > +# The GLib2 include directory Call it GLIB2_INCLUDE_DIRS. > FindGLIB2.cmake:43 > +# > +# For details see the accompanying COPYING-CMAKE-SCRIPTS file. > +#============================================================================= Not needed > FindGLIB2.cmake:46-49 > +if(GLIB2_INCLUDE_DIR AND GLIB2_LIBRARIES) > + # Already in cache, be silent > + set(GLIB2_FIND_QUIETLY TRUE) > +endif() Remove this block > FindGLIB2.cmake:52 > +find_package(PkgConfig) > +pkg_check_modules(PC_LibGLIB2 QUIET glib-2.0) > + nitpick : the pkgconfig file is called glib-2.0.pc and the module FindGLIB2.cmake. What about using PC_GLIB2 instead ? > FindGLIB2.cmake:54 > + > +find_path(GLIB2_MAIN_INCLUDE_DIR > + NAMES glib.h Rename to GLIB2_INCLUDE_DIRS > FindGLIB2.cmake:59-64 > +find_library(GLIB2_LIBRARY > + NAMES glib-2.0 > + HINTS ${PC_LibGLIB2_LIBDIR} > +) > + > +set(GLIB2_LIBRARIES ${GLIB2_LIBRARY}) Do the opposite : use GLIB2_LIBRARIES everywhere and add a comment about GLIB2_LIBRARY being deprecated. > FindGLIB2.cmake:73 > + > +set(GLIB2_INCLUDE_DIR "${GLIB2_MAIN_INCLUDE_DIR}") > + Drop this line > FindGLIB2.cmake:78 > +if(GLIB2_INTERNAL_INCLUDE_DIR) > + set(GLIB2_INCLUDE_DIR ${GLIB2_INCLUDE_DIR} "${GLIB2_INTERNAL_INCLUDE_DIR}") > +endif() list(APPEND GLIB2_INCLUDE_DIRS "${GLIB2_INTERNAL_INCLUDE_DIR}") > FindGLIB2.cmake:87 > + URL "https://wiki.gnome.org/Projects/GLib" > + DESCRIPTION "Event loop and utility library") set and add a comment about GLIB2_INCLUDE_DIR being deprecated and add a target eg: if(GLIB2_FOUND AND NOT TARGET GLIB2::GLIB2) add_library(GLIB2::GLIB2 UNKNOWN IMPORTED) set_target_properties(GLIB2::GLIB2 PROPERTIES IMPORTED_LOCATION "${GLIB2_LIBRARIES}" INTERFACE_INCLUDE_DIRECTORIES "${GLIB2_INCLUDE_DIRS}" ) endif() (+ doc about it) > FindPulseAudio.cmake:8-19 > +# ``PULSEAUDIO_FOUND`` > +# True if the system has the PulseAudio library of at least > +# the minimum version specified by either the version parameter > +# to find_package() or the variable PULSEAUDIO_MINIMUM_VERSION > +# ``PULSEAUDIO_INCLUDE_DIR`` > +# The PulseAudio include directory > +# ``PULSEAUDIO_LIBRARY`` It's a new module, change PULSEAUDIO to PulseAudio to match the module name. > FindPulseAudio.cmake:12 > +# to find_package() or the variable PULSEAUDIO_MINIMUM_VERSION > +# ``PULSEAUDIO_INCLUDE_DIR`` > +# The PulseAudio include directory PulseAudio_INCLUDE_DIRS > FindPulseAudio.cmake:14 > +# The PulseAudio include directory > +# ``PULSEAUDIO_LIBRARY`` > +# The PulseAudio libraries for linking PulseAudio_LIBRARIES > FindPulseAudio.cmake:50 > +# > +# For details see the accompanying COPYING-CMAKE-SCRIPTS file. > +#============================================================================= not needed > FindPulseAudio.cmake:63 > + > +if (NOT WIN32) > + include(FindPkgConfig) Remove this line > FindPulseAudio.cmake:64 > +if (NOT WIN32) > + include(FindPkgConfig) > + pkg_check_modules(PC_PULSEAUDIO QUIET > libpulse>=${PulseAudio_FIND_VERSION}) find_package(PkgConfig) > FindPulseAudio.cmake:67 > + pkg_check_modules(PC_PULSEAUDIO_MAINLOOP QUIET libpulse-mainloop-glib) > +endif() > + and this one :) > FindPulseAudio.cmake:108 > + URL "https://www.freedesktop.org/wiki/Software/PulseAudio" > + DESCRIPTION "Sound server, for sound stream routing and mixing") Add an imported target REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D7823 To: marten, #frameworks, #build_system Cc: cgiboudeaux, cfeck, heikobecker