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.

Sign up for GitHub

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

WIP: Fix bug/behaivor in imported targets via PkgConfig:: #139

Closed
wants to merge 3 commits into from
Closed

WIP: Fix bug/behaivor in imported targets via PkgConfig:: #139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6d264f1
Fix pkgconfig_export
Bjoe Jan 16, 2020
4e32645
Increment internal deps id.
Bjoe Jan 16, 2020
ca76939
Verify if cache entry met hunter root path
Bjoe Feb 4, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions cmake/modules/hunter_pkgconfig_export_target.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@ function(hunter_pkgconfig_export_target PKG_CONFIG_MODULE PKG_GENERATE_SHARED)
if(TARGET "${target_name}")
return()
endif()

if(${PKG_CONFIG_MODULE}_FOUND)
file(READ "${CMAKE_BINARY_DIR}/_3rdParty/Hunter/install-root-dir" HUNTER_INSTALL_ROOT_DIR)
list(APPEND _copy_of_${PKG_CONFIG_MODULE}_LIBRARY_DIRS "${PKG_CONFIG_MODULE}_LIBRARY_DIRS")
list(FILTER _copy_of_${PKG_CONFIG_MODULE}_LIBRARY_DIRS EXCLUDE REGEX "${HUNTER_INSTALL_ROOT_DIR}")
list(LENGTH _copy_of_${PKG_CONFIG_MODULE}_LIBRARY_DIRS _list_length)
if(_list_length GREATER 0)
message(SEND_ERROR "CMakeCache entries for ${PKG_CONFIG_MODULE}_LIBRARY_DIRS points to ${${PKG_CONFIG_MODULE}_LIBRARY_DIRS} but the current hunter install root dir is ${HUNTER_INSTALL_ROOT_DIR}\nYou should verify and change/remove all wrong cache entries or delete your CMakeCache.txt in your build folder!")
Copy link
Author

Choose a reason for hiding this comment

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

The error message should be "corrected" ... I only write "something" ...

endif()
endif()

Copy link
Author

Choose a reason for hiding this comment

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

@hjmallon and @rbsheth What do you thing about this solution?

Copy link

Choose a reason for hiding this comment

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

Honestly I'm not sure I understand the issue still. In my experience CMake basically requires you bin your build tree (at least you CMakeCache.txt) when you change any libraries, which would include libraries changed by Hunter after a Hunter upgrade. If that is the thing you are trying to fix then I imagine it happens with non-pkgconfig libraries too? Maybe @rbsheth will have more of a clear idea?

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I don't have a clear idea. 😄

Based on the error description, it seems like using pkgconfig targets screws up CMake somehow and does not "refresh" the target if the version has changed. Wouldn't it be better to force a refresh somehow? This message is basically telling the user to nuke their CMakeCache, right?

Copy link
Author

@Bjoe Bjoe Feb 5, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Sometime an example say more than thousand words ;-)

Do following ...

Clone my example:

git clone https://github.com/Bjoe/hunter.git

Then checkout:

git checkout step-1

Build the project .... after all is fine execute the bte binary:

Version: Botan 2.10.0 (unreleased, revision unknown, distribution unspecified)

So no surprise ... :-)

Keep eyes on following cmake output:

HUNTER_INSTALL_ROOT_DIR= ...
botan-2_LIBRARY_DIRS= ...

HUNTER_INSTALL_ROOT_DIR is the current hunter path
botan-2_LIBRARY_DIRS is the "cache entry" in CMakeCache.txt which pkg_check_modules is set.

Now we update the botan version from 2.10.0 to 2.11.0-110af9494 .... checkout out ...

git checkout step-2

Now build the project again for example cmake --build <YOUR_BUILD_PATH> ...

Hunter will build botan version 2.11.0-110af9494 and you see also following cmake output like:

...
-- [hunter] BOTAN_ROOT: /home/developer/.hunter/_Base/36e7f44/f49d2cd/8d03466/Install (ver.: 2.11.0-110af9494)
...

So all looks fine, but when you execute bte it will still print:

Version: Botan 2.10.0 (unreleased, revision unknown, distribution unspecified)

If you verify the output from HUNTER_INSTALL_ROOT_DIR and botan-2_LIBRARY_DIRS ... then you will have some output like:

-- HUNTER_INSTALL_ROOT_DIR=/home/developer/.hunter/_Base/36e7f44/f49d2cd/8d03466/Install
-- botan-2_LIBRARY_DIRS=/home/developer/.hunter/_Base/36e7f44/f49d2cd/5f3f694/Install/lib

You see .... botan-2_LIBRARY_DIRS is still pointing to the "build" before ... this is really bad and nobody will see this!

This happen, because if botan-2_FOUND is 1 then pkg_check_modules reads only the environemnt from the CMakeCache.txt. This behaivor is like find_library see https://cmake.org/cmake/help/v3.16/command/find_library.html?highlight=find_library

You can test my solution ... checkout

git checkout step-3

Now you will get the error message (again should be more "proper").

As @hjmallon comment all the FindX.cmake working somehow with find_library, find_path or find_program ... all they have the same behaivor.
Hunter also provides FindX.cmake with for example cmake/find/FindAvahi.cmake line 51 or cmake/find/FindBoost.cmake line 326 ....
Btw. FindBoost.cmake is also used when you add the config module with find_package(Boost CONFIG REQUIRED) ! See cmake/templates/BoostConfig.cmake.in line 51 find_package(Boost MODULE REQUIRED ${Boost_FIND_COMPONENTS}) ... this will fixed in the use boost config branch ..

Anyway, I will verify if there are more "pitfalls" for example with find_package ...

Copy link
Member

Choose a reason for hiding this comment

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

Wow, thanks for the detailed explanation. I see what is going on now. Is this PR still WIP?

Copy link
Author

Choose a reason for hiding this comment

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

@rbsheth Yes, still WIP, since I will check whether the "find_package ()" is also affected. Then I want to try to check "this" in "hunter_finalize ()" ... that's my plan. I'm trying to get ready this week

pkg_check_modules(${PKG_CONFIG_MODULE} ${PKG_CONFIG_MODULE})
if(NOT ${PKG_CONFIG_MODULE}_FOUND)
hunter_internal_error(
Expand Down
2 changes: 1 addition & 1 deletion cmake/projects/x11/hunter.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ hunter_cmake_args(
hunter_cacheable(x11)
hunter_download(
PACKAGE_NAME x11
PACKAGE_INTERNAL_DEPS_ID "7"
PACKAGE_INTERNAL_DEPS_ID "8"
PACKAGE_UNRELOCATABLE_TEXT_FILES
"lib/libX11-xcb.la"
"lib/libX11.la"
Expand Down
2 changes: 1 addition & 1 deletion cmake/projects/x264/hunter.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ hunter_cmake_args(x264 CMAKE_ARGS PKGCONFIG_EXPORT_TARGETS=x264)
hunter_cacheable(x264)
hunter_download(
PACKAGE_NAME x264
PACKAGE_INTERNAL_DEPS_ID "3"
PACKAGE_INTERNAL_DEPS_ID "4"
PACKAGE_UNRELOCATABLE_TEXT_FILES "lib/pkgconfig/x264.pc"
)
2 changes: 1 addition & 1 deletion cmake/projects/xcb/hunter.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,6 @@ hunter_pick_scheme(DEFAULT xcb)
hunter_cacheable(xcb)
hunter_download(
PACKAGE_NAME xcb
PACKAGE_INTERNAL_DEPS_ID "6"
PACKAGE_INTERNAL_DEPS_ID "7"
PACKAGE_UNRELOCATABLE_TEXT_FILES "${_xcb_text_files}"
)