Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake: incorrect PKG_CONFIG_EXECUTABLE setting is used #1023

Closed
uwehermann opened this issue Dec 1, 2015 · 9 comments
Closed

cmake: incorrect PKG_CONFIG_EXECUTABLE setting is used #1023

uwehermann opened this issue Dec 1, 2015 · 9 comments
Labels

Comments

@uwehermann
Copy link
Contributor

I was under the impression that an issue we're seeing was the same as #904, but I don't think that is the case anymore. Here's what seems to have been changed and goes wrong.

The setup:

A CMakeLists.txt file for a project is using something like this to find pkg-config-using libraries:

include(FindPkgConfig)
...
list(APPEND PKGDEPS libfoo>=x.y.z)
find_package(PkgConfig)
pkg_check_modules(PKGDEPS REQUIRED ${PKGDEPS})

Old (working) mxe.conf had this:
set(PKG_CONFIG_EXECUTABLE /home/uwe/mxe-git/usr/bin/i686-w64-mingw32.static-pkg-config)

New (not working) mxe.conf has this:

file(GLOB mxe_cmake_files
    "/home/uwe/mxe-git/usr/i686-w64-mingw32.static/share/cmake/mxe-conf.d/*.cmake"
)
foreach(mxe_cmake_file ${mxe_cmake_files})
    include(${mxe_cmake_file})
endforeach()

And /home/uwe/mxe-git/usr/i686-w64-mingw32.static/share/cmake/mxe-conf.d/pkgconf.cmake contains:
set(PKG_CONFIG_EXECUTABLE /home/uwe/mxe-git/usr/bin/i686-w64-mingw32.static-pkg-config CACHE PATH "pkg-config executable")

The problem/difference is the "CACHE" here, I think. With the above PKG_CONFIG_EXECUTABLE is /usr/bin/pkg-config before and after the set line. When dropping the CACHE PATH "pkg-config executable" part, the PKG_CONFIG_EXECUTABLE after the set line is now correct again:

/home/uwe/mxe-git/usr/bin/i686-w64-mingw32.static-pkg-config

Using the correct PKG_CONFIG_EXECUTABLE the host cmake now finds the packages correctly again.

Please double-check if performing this change makes sense for MXE and all its requirements, I only checked one very specific use-case here.

@starius
Copy link
Member

starius commented Dec 1, 2015

CACHE was added in 5a81e60, #896

@tonytheodore
Copy link
Member

From cmake docs:

Since cache entries are meant to provide user-settable values this does not overwrite existing cache entries by default. Use the FORCE option to overwrite existing entries.

Is there somehow an existing cache entry for PKG_CONFIG_EXECUTABLE?

@uwehermann
Copy link
Contributor Author

Ah, yes. I've looked a bit further and in this case the root cause is this snippet in our CMakeLists.txt file:

include(FindPkgConfig)
...
find_package(PkgConfig)

This leads to cmake checking for the pkg-config executable twice. Right now this results in:

-- Found PkgConfig: /usr/bin/pkg-config (found version "0.28") 
-- checking for module 'libsigrokcxx>=0.4.0'
--   package 'libsigrokcxx>=0.4.0' not found

With one of the fixes below the correct thing happens:

-- Found PkgConfig: /usr/bin/pkg-config (found version "0.28") 
-- Found PkgConfig: /.../i686-w64-mingw32.static.posix-pkg-config (found version "0.28") 
-- checking for module 'libsigrokcxx>=0.4.0'
--   found libsigrokcxx, version 0.4.0-git-8abdf00

Or we don't do the PkgConfig init twice in our software, then it works either way:

-- Found PkgConfig: /.../i686-w64-mingw32.static.posix-pkg-config (found version "0.28") 
-- checking for module 'libsigrokcxx>=0.4.0'
--   found libsigrokcxx, version 0.4.0-git-8abdf00

So here's the possible fixes:

Fix1:

-set(PKG_CONFIG_EXECUTABLE /.../i686-w64-mingw32.static.posix-pkg-config CACHE PATH "pkg-config executable")
+set(PKG_CONFIG_EXECUTABLE /.../i686-w64-mingw32.static.posix-pkg-config)

Fix2:

-set(PKG_CONFIG_EXECUTABLE /.../i686-w64-mingw32.static.posix-pkg-config CACHE PATH "pkg-config executable")
+set(PKG_CONFIG_EXECUTABLE /.../i686-w64-mingw32.static.posix-pkg-config CACHE PATH "pkg-config executable" FORCE)

Fix3:
Do nothing, declare multiple invokations of "include(FindPkgConfig)" and "find_package(PkgConfig)" in one CMakeLists.txt file a bug.

Either way, I think the intended behaviour in MXE should be defined and documented. Fix3 is something we should probably fix in our software either way, but OTOH it would also be nice if this worked regardless, i.e. Fix2 might be a nicer solution possibly. Assuming the FORCE there doesn't cause other problems, that is.

@tonytheodore
Copy link
Member

Thanks for the investigation!

I'm thinking FORCE is what we want here, but I'm not clear on how the initial:

-- Found PkgConfig: /usr/bin/pkg-config (found version "0.28") 

happens if cmake is invoked with the toolchain file. Have you run cmake without the toolchain file previously? If so, then FORCE will likely break that workflow as you won't be able to switch back.

@uwehermann
Copy link
Contributor Author

No, cmake is always run with the same toolchain file. Here's how to reproduce:

$ ls
CMakeLists.txt

$ cat CMakeLists.txt
cmake_minimum_required(VERSION 2.8.6)
include(FindPkgConfig)
project(foo)
list(APPEND PKGDEPS libsigrokcxx>=0.4.0)
find_package(PkgConfig)
pkg_check_modules(PKGDEPS REQUIRED ${PKGDEPS})

$ PKG_CONFIG_PATH_i686_w64_mingw32_static_posix=/home/uwe/sr_mingw/lib/pkgconfig:/home/uwe/mxe-git/usr/i686-w64-mingw32.static.posix/lib/pkgconfig cmake -DCMAKE_TOOLCHAIN_FILE=/home/uwe/mxe-git/usr/i686-w64-mingw32.static.posix/share/cmake/mxe-conf.cmake .
-- Found PkgConfig: /usr/bin/pkg-config (found version "0.28") 
-- The C compiler identification is GNU 4.9.3
-- The CXX compiler identification is GNU 4.9.3
-- Check for working C compiler: /home/uwe/mxe-git/usr/bin/i686-w64-mingw32.static.posix-gcc
-- Check for working C compiler: /home/uwe/mxe-git/usr/bin/i686-w64-mingw32.static.posix-gcc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /home/uwe/mxe-git/usr/bin/i686-w64-mingw32.static.posix-g++
-- Check for working CXX compiler: /home/uwe/mxe-git/usr/bin/i686-w64-mingw32.static.posix-g++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- checking for module 'libsigrokcxx>=0.4.0'
--   package 'libsigrokcxx>=0.4.0' not found
CMake Error at /usr/share/cmake-3.3/Modules/FindPkgConfig.cmake:340 (message):
  A required package was not found
Call Stack (most recent call first):
  /usr/share/cmake-3.3/Modules/FindPkgConfig.cmake:502 (_pkg_check_modules_internal)
  CMakeLists.txt:6 (pkg_check_modules)


-- Configuring incomplete, errors occurred!

The first "include(FindPkgConfig)" seems to do a pkg-config check (as does the second), but for some reason it finds another pkg-config and/or doesn't honor the toolchain file settings. I haven't really looked into that any further.

(Put any random *.pc file into ~/sr_mingw/lib/pkgconfig for testing and replace libsigrokcxx accordingly above)

@tonytheodore
Copy link
Member

@ChristianFrisson, do you have any insights on this? Adding FORCE seems the way to go.

@ChristianFrisson
Copy link
Contributor

Please remove include(FindPkgConfig)!

find_package(PkgConfig) is sufficient for the job.
cmake already knows where to find FindPkgConfig.cmake especially since it is a defaut search module, from the prefix where cmake is installed: share/cmake-*/Modules/ (cmake version wildcarded here).

For more info check https://cmake.org/cmake/help/latest/command/find_package.html

I actually issued #896 otherwise multiple/first calls to find_package(PkgConfig) may fail.

Hoping this will help!

abraxa pushed a commit to abraxa/pulseview that referenced this issue Dec 12, 2015
This causes the check for the pkg-config tool to be performed twice
since it is followed by another "find_package(PkgConfig)".

This can cause issues in some situation, see e.g.
mxe/mxe#1023 (comment)

This fixes bug #663.
starius added a commit to LuaAndC/mxe that referenced this issue Jan 1, 2016
@tonytheodore
Copy link
Member

The first "include(FindPkgConfig)" seems to do a pkg-config check (as does the second), but for some reason it finds another pkg-config and/or doesn't honor the toolchain file settings.

Oddly, the behaviour seems to change if you put it after the project(foo) command. Setting the env var PKG_CONFIG works with later versions of cmake (but not 3.0.1 used by mxe) - I think FORCE is the way to go.

tonytheodore added a commit to tonytheodore/mxe that referenced this issue Jan 21, 2016
* use templates instead of `echo` for readability
* ensure `pkg-config` executable is set correctly
    - closes mxe#1023
    - avoids future cases of mxe#1108 (comment)
* moves `cmake_policy` to toolchain (closes mxe#971)
* use `CACHE ... FORCE` for other variables (BUILD_SHARED_LIBS etc.)
tonytheodore added a commit to tonytheodore/mxe that referenced this issue Jan 21, 2016
* use templates instead of `echo` for readability
* ensure `pkg-config` executable is set correctly
    - closes mxe#1023
    - avoids future cases of mxe#1108 (comment)
* moves `cmake_policy` to toolchain (closes mxe#971)
* use `CACHE ... FORCE` for other variables (BUILD_SHARED_LIBS etc.)
fiesh pushed a commit to fiesh/mxe that referenced this issue Feb 5, 2016
@tonytheodore
Copy link
Member

I think FORCE is the way to go.

Now I think that's wrong - FORCE causes other issues and doesn't seem to be something we should put in the toolchain.

#2097 simply errors if PKG_CONFIG_FOUND is set in any way before the toolchain is read.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants