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

Bjoe · 2020-01-16T11:21:24Z

I've checked this Git style guide. Yes
I've checked this CMake style guide. Yes
My change will work with CMake 3.2 (minimum requirement for Hunter). Yes
I will try to keep this pull request as small as possible and will try not to mix unrelated features. Yes

pkg_check_modules() is only reparse/scan .pc files if XYZ_FOUND is 0. If XYZ_FOUND is 1, nothing happens.
This behaviour cause following problem:
If you change or update some setting, configuration or version numbers in hunter, all the hunter packages they generated the "find" config modul via pkgconfig-export-targets.cmake.in are still use the "old" libraries.

This change will force to reparse/scan the .pc files every cmake execution. Better approach is maybe to verify if the path in the CMakeCache is different then the "new one", but I think this is a bigger effort....

Force to parse again .pc file via pkg_check_modules(). After "updating" to a new hunter, toolchain or config version, CMakeCache.txt should be reinitialise with the new path.

hjmallon · 2020-01-21T07:45:16Z

I think the behaviour 'dont search a library again if found unless the CMakeCache.txt is cleared' is the same as the default FindX.cmake files included with CMake? Would this make behaviour inconsistent with those?

Bjoe · 2020-01-23T05:55:48Z

I think the behaviour 'dont search a library again if found unless the CMakeCache.txt is cleared' is the same as the default FindX.cmake files included with CMake? Would this make behaviour inconsistent with those?

Yep, I've also thought about it. But I think we should avoid to use the MODULE mode/way to add third party libs and use only CONFIG mode in find_package. I think in the CONFIG mode, there is no FindX.cmake at all ... at least what i know. The problem is that 90% of the third party libs that added via PkgConfig:: target is affected when you change for example your local "configuration" for hunter .... you never ever see that you still use a "wrong" lib.

At least we should add a warning !

Bjoe · 2020-02-03T16:39:51Z

@rbsheth I'm fully agree with @hjmallon. Let me try to add a warning/error if the path's in the CMakeCache.txt are not align with the current hunter config. So don't merge yet please.

Bjoe

Please have a look...

Bjoe · 2020-02-04T16:49:07Z

cmake/modules/hunter_pkgconfig_export_target.cmake

+      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!")
+    endif()
+  endif()
+


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

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?

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?

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 ...

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

@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

Bjoe · 2020-02-04T16:53:16Z

cmake/modules/hunter_pkgconfig_export_target.cmake

+    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!")


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

Bjoe · 2020-02-12T08:12:26Z

I thinking about to add a "verification" in hunter_finalize ... I will provide a PR for this that's why I changed this title to WIP (Work In Process) ...

rbsheth · 2020-03-06T14:35:05Z

I thinking about to add a "verification" in hunter_finalize ... I will provide a PR for this that's why I changed this title to WIP (Work In Process) ...

@Bjoe Still WIP?

Bjoe · 2020-03-10T09:04:50Z

@rbsheth Yep, I did some test with find_package(... CONFIG) ... It looks like it is not affected. But give me some more day. Currently testing an approach to verify this error in hunter_finalize()

rbsheth · 2020-03-13T13:26:55Z

@rbsheth Yep, I did some test with find_package(... CONFIG) ... It looks like it is not affected. But give me some more day. Currently testing an approach to verify this error in hunter_finalize()

Okay!

Bjoe · 2020-03-17T07:56:47Z

I created a new proposal see PR #173 ...

rbsheth · 2020-04-02T15:54:34Z

I created a new proposal see PR #173 ...

@Bjoe So is this PR obsolete?

Bjoe · 2020-04-09T12:50:07Z

@rbsheth Yep. I will close this PR because of #173

Bjoe added 2 commits January 16, 2020 12:01

Fix pkgconfig_export

6d264f1

Force to parse again .pc file via pkg_check_modules(). After "updating" to a new hunter, toolchain or config version, CMakeCache.txt should be reinitialise with the new path.

Increment internal deps id.

4e32645

Bjoe changed the title ~~Fix hunter pkgconfig export target~~ Fix bug/behaivor in imported targets via PkgConfig:: Jan 23, 2020

rbsheth approved these changes Jan 27, 2020

View reviewed changes

Verify if cache entry met hunter root path

ca76939

Bjoe commented Feb 4, 2020

View reviewed changes

Bjoe requested a review from rbsheth February 4, 2020 16:54

Bjoe mentioned this pull request Feb 5, 2020

Add "scelta" package #142

Merged

Bjoe requested a review from hjmallon February 5, 2020 15:20

hjmallon approved these changes Feb 5, 2020

View reviewed changes

Bjoe changed the title ~~Fix bug/behaivor in imported targets via PkgConfig::~~ WIP: Fix bug/behaivor in imported targets via PkgConfig:: Feb 12, 2020

Bjoe mentioned this pull request Feb 13, 2020

Add support to use cmake package configuration file from boost #154

Merged

Bjoe mentioned this pull request Mar 17, 2020

WIP: Verify if the hunter id path is the same as before #173

Open

Bjoe closed this Apr 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Bjoe commented Jan 16, 2020

hjmallon commented Jan 21, 2020

Bjoe commented Jan 23, 2020

Bjoe commented Feb 3, 2020

Bjoe left a comment

Bjoe Feb 4, 2020

hjmallon Feb 4, 2020

rbsheth Feb 4, 2020

Bjoe Feb 5, 2020 •

edited

Loading

rbsheth Feb 15, 2020

Bjoe Feb 16, 2020

Bjoe Feb 4, 2020

Bjoe commented Feb 12, 2020

rbsheth commented Mar 6, 2020

Bjoe commented Mar 10, 2020

rbsheth commented Mar 13, 2020

Bjoe commented Mar 17, 2020

rbsheth commented Apr 2, 2020

Bjoe commented Apr 9, 2020

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

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

Conversation

Bjoe commented Jan 16, 2020

hjmallon commented Jan 21, 2020

Bjoe commented Jan 23, 2020

Bjoe commented Feb 3, 2020

Bjoe left a comment

Choose a reason for hiding this comment

Bjoe Feb 4, 2020

Choose a reason for hiding this comment

hjmallon Feb 4, 2020

Choose a reason for hiding this comment

rbsheth Feb 4, 2020

Choose a reason for hiding this comment

Bjoe Feb 5, 2020 • edited Loading

Choose a reason for hiding this comment

rbsheth Feb 15, 2020

Choose a reason for hiding this comment

Bjoe Feb 16, 2020

Choose a reason for hiding this comment

Bjoe Feb 4, 2020

Choose a reason for hiding this comment

Bjoe commented Feb 12, 2020

rbsheth commented Mar 6, 2020

Bjoe commented Mar 10, 2020

rbsheth commented Mar 13, 2020

Bjoe commented Mar 17, 2020

rbsheth commented Apr 2, 2020

Bjoe commented Apr 9, 2020

Bjoe Feb 5, 2020 •

edited

Loading