MantisBT - CMake
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0015903CMakeModulespublic2016-01-04 11:492016-06-10 14:21
ReporterBernd Lörwald 
Assigned ToBen Boeckel 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product VersionCMake 3.5 
Target VersionCMake 3.5.1Fixed in VersionCMake 3.5.1 
Summary0015903: pkg_search_module no longer caches _PREFIX, _LIBDIR, _INCLUDEDIR
DescriptionBeginning with https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=70e8db6e20749a484dd677d7094780c5f4b451c6, [^] the pkgconfig module no longer caches the _PREFIX, _LIBDIR, _INCLUDEDIR variables. This was introduced by using pkg_get_variable which sets for PARENT_SCOPE but not CACHE as _pkgconfig_set did previously. Instead, a helper variable is cached.
Steps To Reproduce$ cat > CMakeLists.txt
cmake_minimum_required (VERSION 3.3)
find_package (PkgConfig)
pkg_search_module (PREFIX REQUIRED gl)
if (NOT PREFIX_PREFIX)
  message (FATAL_ERROR "BUMMER")
endif()
^D

$ cmake-3.4.1 .
-- […snip]
-- Found PkgConfig: /usr/bin/pkg-config (found version "0.28")
-- Checking for one of the modules 'gl'
-- Configuring done
-- Generating done
-- Build files have been written to: […snip]
$ cmake-3.4.1 .
CMake Error at CMakeLists.txt:5 (message):
  BUMMER
$ grep -E "prefix_result|PREFIX_PREFIX" CMakeCache.txt
PREFIX_PREFIX:INTERNAL=
prefix_result:INTERNAL=/usr/lib64

$ cmake-3.3.2 .
-- […snip]
-- Found PkgConfig: /usr/bin/pkg-config (found version "0.28")
-- checking for one of the modules 'gl'
-- Configuring done
-- Generating done
-- Build files have been written to: […snip]
$ cmake-3.3.2 .
-- Configuring done
-- Generating done
-- Build files have been written to: […snip]
$ grep -E "prefix_result|PREFIX_PREFIX" CMakeCache.txt
PREFIX_PREFIX:INTERNAL=/usr
Additional Information
TagsNo tags attached.
Relationships
related to 0015910closed  pkg_check_modules() should add PREFIX/share/pkgconfig to PKG_CONFIG_PATH 
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2016-01-04 11:49Bernd LörwaldNew Issue
2016-01-04 12:39Ben BoeckelAssigned To => Ben Boeckel
2016-01-04 12:39Ben BoeckelStatusnew => assigned
2016-01-04 12:40Ben BoeckelNote Added: 0040135
2016-01-04 12:47Bernd LörwaldNote Added: 0040136
2016-01-04 12:56Ben BoeckelNote Added: 0040137
2016-01-07 16:06Brad KingProduct VersionCMake 3.4.1 => CMake 3.5
2016-01-07 16:06Brad KingTarget Version => CMake 3.5
2016-01-12 09:58Brad KingRelationship addedrelated to 0015910
2016-01-21 10:41Ben BoeckelNote Added: 0040286
2016-01-22 09:05Brad KingNote Added: 0040304
2016-01-22 09:05Brad KingStatusassigned => resolved
2016-01-22 09:05Brad KingResolutionopen => fixed
2016-01-22 09:05Brad KingFixed in Version => CMake 3.5
2016-03-15 13:28Bernd LörwaldNote Added: 0040695
2016-03-15 13:28Bernd LörwaldNote Edited: 0040695bug_revision_view_page.php?bugnote_id=40695#r2058
2016-03-21 09:07Brad KingNote Added: 0040735
2016-03-21 09:07Brad KingFixed in VersionCMake 3.5 => CMake 3.5.1
2016-03-21 09:07Brad KingTarget VersionCMake 3.5 => CMake 3.5.1
2016-06-10 14:21Kitware RobotNote Added: 0041220
2016-06-10 14:21Kitware RobotStatusresolved => closed

Notes
(0040135)
Ben Boeckel   
2016-01-04 12:40   
These were never documented before, but yes, they should be cached still (and documented).
(0040136)
Bernd Lörwald   
2016-01-04 12:47   
> These were never documented before

The documentation seems to list them at

https://cmake.org/gitweb?p=cmake.git;a=blob;f=Modules/FindPkgConfig.cmake;h=e822b9c43bcbda8cb82f05cc0e9921f6fc9d690a;hb=fd7180f0c0c2554c31afda235469df986a109fe4#l473 [^]

which is where we got the names from to begin with. Did I miss something there?
(0040137)
Ben Boeckel   
2016-01-04 12:56   
Indeed it does; I looked in the wrong place.
(0040286)
Ben Boeckel   
2016-01-21 10:41   
Pushed to stage and next as fix-pkg_search_module-cache.
(0040304)
Brad King   
2016-01-22 09:05   
For reference, the commit mentioned in 0015903:0040286 is:

 FindPkgConfig: set standard variables in the cache
 https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=40249bcc [^]
(0040695)
Bernd Lörwald   
2016-03-15 13:28   
[apparently mantis has a timeout for how long you can take to write tickets so I lost the entire bigger and properly written version of this. this might not be entirely perfect.]

* it is still broken in 3.5.0, the test case does not execute cmake twice which is what triggers the issue
* it is setting the wrong variable in the cache

taking the test case, putting it into a cmakelists.txt, adding a line of debug output and changing to use gl instead of ncurses (due to not having ncurses here) with the diff

modified CMakeLists.txt
@@ -1,16 +1,20 @@
-find_package(PkgConfig REQUIRED)
-pkg_check_modules(NCURSES QUIET ncurses)
+cmake_minimum_required (VERSION 3.3)
 
-if (NCURSES_FOUND)
+include (${CMAKE_SOURCE_DIR}/FindPkgConfig.cmake)
+
+pkg_check_modules(GL QUIET gl)
+
+if (GL_FOUND)
   foreach (variable IN ITEMS PREFIX INCLUDEDIR LIBDIR)
     get_property("${variable}"
- CACHE "NCURSES_${variable}"
+ CACHE "GL_${variable}"
       PROPERTY TYPE
       SET)
+ message (STATUS "${variable}: ${GL_${variable}} ${${variable}}")
     if (NOT ${variable})
- message(FATAL_ERROR "Failed to set cache entry for NCURSES_${variable}")
+ message(FATAL_ERROR "Failed to set cache entry for GL_${variable}")
     endif ()
   endforeach ()
 else ()
- message(STATUS "skipping test; ncurses not found")
+ message(STATUS "skipping test; gl not found")
 endif ()

running

$ cmake-3.5.0 .
$ grep -E "prefix_result|GL_PREFIX
$ cmake-3.5.0 .
$ grep -E "prefix_result|GL_PREFIX

yields

> -- PREFIX: /usr 1
> -- INCLUDEDIR: /usr/include 1
> -- LIBDIR: /usr/lib64 1
> GL_PREFIX:INTERNAL=
> prefix_result:INTERNAL=/usr/lib64
> -- PREFIX: 1
> -- INCLUDEDIR: 1
> -- LIBDIR: 1
> GL_PREFIX:INTERNAL=
> prefix_result:INTERNAL=/usr/lib64

where the second invokation is missing the variables, but still claims to have it set in the cache, which shows that the test is insufficient.

doing the same with cmake-3.3.2 as a cross check yields

> -- PREFIX: /usr 1
> -- INCLUDEDIR: /usr/include 1
> -- LIBDIR: /usr/lib64 1
> GL_PREFIX:INTERNAL=/usr
> -- PREFIX: /usr 1
> -- INCLUDEDIR: /usr/include 1
> -- LIBDIR: /usr/lib64 1
> GL_PREFIX:INTERNAL=/usr

debugging FindPkgConfig shows that it is setting the wrong variable: gl_PREFIX rather than GL_PREFIX (or GL_gl_PREFIX in the case of searching multiple modules (<XPREFIX>_PREFIX as described in the documentation)).

A quick hack of

diff --git FindPkgConfig.cmake FindPkgConfig.cmake
@@ -383,7 +390,7 @@ macro(_pkg_check_modules_internal _is_required _is_silent _no_cmake_path _no_cma
         pkg_get_variable("${_pkg_check_prefix}_INCLUDEDIR" ${_pkg_check_modules_pkg} "includedir")
         pkg_get_variable("${_pkg_check_prefix}_LIBDIR" ${_pkg_check_modules_pkg} "libdir")
         foreach (variable IN ITEMS PREFIX INCLUDEDIR LIBDIR)
- _pkgconfig_set("${_pkg_check_modules_pkg}_${variable}" "${${_pkg_check_modules_pkg}_${variable}}")
+ _pkgconfig_set("${_pkg_check_prefix}_${variable}" "${${_pkg_check_prefix}_${variable}}")
         endforeach ()
 
         if (NOT ${_is_silent})

shows that at least for the test case used in this issue as well as the test case added in 40249bcc, it fixes the issue:

> -- PREFIX: /usr 1
> -- INCLUDEDIR: /usr/include 1
> -- LIBDIR: /usr/lib64 1
> GL_PREFIX:INTERNAL=/usr
> prefix_result:INTERNAL=/usr/lib64
> -- PREFIX: /usr 1
> -- INCLUDEDIR: /usr/include 1
> -- LIBDIR: /usr/lib64 1
> GL_PREFIX:INTERNAL=/usr
> prefix_result:INTERNAL=/usr/lib64

leaving only the clobbering with prefix_result which is due to pkg_get_variable using _pkgconfig_invoke which automatically assembles the output variable name instead of letting it be passed in.
(0040735)
Brad King   
2016-03-21 09:07   
Re 0015903:0040695: Thanks. Fixed:

 FindPkgConfig: set correctly named variables in cache
 https://cmake.org/gitweb?p=cmake.git;a=commitdiff;h=6f4f9054 [^]

I've queued this for merge to 'release' for 3.5.1.
(0041220)
Kitware Robot   
2016-06-10 14:21   
This issue tracker is no longer used. Further discussion of this issue may take place in the current CMake Issues page linked in the banner at the top of this page.