cmake: Fix paths in pkg-config file #563
Conversation
It is not generally true that `CMAKE_INSTALL_<dir>` variables are relative paths: https://github.com/jtojnar/cmake-snips#concatenating-paths-when-building-pkg-config-files Absolute paths will cause things like this appear: includedir=${prefix}//nix/store/87zhscw1p08nqm73ls2sd82zmr1a8rni-libiio-0.21-dev/include Signed-off-by: Jan Tojnar <[email protected]>
| @@ -1,7 +1,7 @@ | |||
| prefix=@CMAKE_INSTALL_PREFIX@ | |||
| exec_prefix=${prefix} | |||
| libdir=${prefix}/@CMAKE_INSTALL_LIBDIR@ | |||
| includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@ | |||
| libdir=@libdir_for_pc_file@ | |||
There was a problem hiding this comment.
what if we do:
libdir=@CMAKE_INSTALL_LIBDIR@
includedir=@CMAKE_INSTALL_INCLUDEDIR@
and make sure that CMAKE_INSTALL_xxxx are absolute paths?
and if they aren't absolute maybe we cal do CMAKE_INSTALL_LIBDIR=CMAKE_INSTALL_PREFIX/CMAKE_INSTALL_LIBDIR
the overall join_paths() looks nice, but it's a bit more logic than is required to solve the actual problem;
and it looks to me that CMAKE_INSTALL_LIBDIR (and includedir) is used in CMakeLists.txt as well;
so there can be a minor inconsistency between the values of CMAKE_INSTALL_LIBDIR use in libiio.pc.cmakein and the ones used in CMakeLists.txt
so, maybe something like?:
if(NOT IS_ABSOLUTE "${CMAKE_INSTALL_LIBDIR}")
CMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}
endif
There was a problem hiding this comment.
and make sure that
CMAKE_INSTALL_xxxxare absolute paths?
There are FULL variants of the variables, see the docs so that could be used if we decide to go that way.
The reason why there is ${prefix} is to make it overridable (at least in the cases when it is not absolute), see https://www.bassi.io/articles/2018/03/15/pkg-config-and-paths/.
There was a problem hiding this comment.
hmm, you are right;
i am just a bit hesitant about having an extra level of indirection for changing from includedir=${prefix}/@CMAKE_INSTALL_INCLUDEDIR@ to constructing ${prefix}/@CMAKE_INSTALL_INCLUDEDIR@ in CMake via some function;
if libiio.pc.cmakein will only be generated from CMake, then this code seems a bit much;
i'm not fully against it, i just prefer something simpler to read/review
There was a problem hiding this comment.
I guess the absolute paths are fine if libiio does not expect other software to ever install to their libdir or includedir.
There was a problem hiding this comment.
if this would fix your case as well, i would prefer to do it simpler for now;
if we need to add the complexity of join_paths() we can add it later;
There was a problem hiding this comment.
Depends on whether libiio is extensible and the extension points are the installation paths. If not, I was not able to come up with a reason #564 should fail.
There was a problem hiding this comment.
oh; i did not notice the other PR;
too many emails i think
| include(cmake/JoinPaths.cmake) | ||
|
|
||
| join_paths(libdir_for_pc_file "\${prefix}" "${CMAKE_INSTALL_LIBDIR}") | ||
| join_paths(includedir_for_pc_file "\${prefix}" "${CMAKE_INSTALL_INCLUDEDIR}") |
There was a problem hiding this comment.
continuing from comment 48e1606#r449998108
this is where we could do:
if(NOT IS_ABSOLUTE "${CMAKE_INSTALL_LIBDIR}")
CMAKE_INSTALL_LIBDIR=${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_LIBDIR}
endif
|
thanks for the find :) |
Alternative fix for: analogdevicesinc#563 It is not generally true that `CMAKE_INSTALL_<dir>` variables are relative paths: https://github.com/jtojnar/cmake-snips#concatenating-paths-when-building-pkg-config-files Absolute paths will cause things like this to appear: includedir=${prefix}//nix/store/87zhscw1p08nqm73ls2sd82zmr1a8rni-libiio-0.21-dev/include It would be best if the paths were relative to `${prefix}` pkg-config variable so that they could be overridden: https://www.bassi.io/articles/2018/03/15/pkg-config-and-paths/ but since that is not easy to do in CMake, we will just use absolute paths. libiio does not expect other project to install artefacts to its installation paths so non-overriddable paths should no do much harm. Signed-off-by: Jan Tojnar <[email protected]>
|
#564 was merged instead of this - so closing. |
It is not generally true that
CMAKE_INSTALL_<dir>variables are relative paths:https://github.com/jtojnar/cmake-snips#concatenating-paths-when-building-pkg-config-files
Absolute paths will cause things like this appear:
Signed-off-by: Jan Tojnar [email protected]