Re: [cmake-developers] [Patch] FindPkgConfig: optionally create imported target for the found libraries
Am Freitag, 13. Mai 2016, 13:38:52 schrieb Brad King:
> On 05/13/2016 12:21 PM, Rolf Eike Beer wrote:
> >> Can you just use
> >>
> >> set_property(TARGET PkgConfig::${_prefix} PROPERTY ...)
> >>
> >> in each `if()` block?
> >
> > I had this before, but I thought I avoid multiple target lookups. You
> > should only ever see that message if none of the branches is taken I
> > think. And that should never happen, or the if before is wrong. Can you
> > add a message() and show what is actually in _props there?
>
> The problem is that _libs has more than one value, so set_target_properties
> ends up with
>
> PROPERTIES PROP1 v1 PROP2 v2a v2b PROP3 v3
> ^^^
>
> Please use set_property(TARGET). It is much more robust, and this is not
> exactly a performance-critical loop.
Hopefully all better now.
Eike
signature.asc
Description: This is a digitally signed message part.
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Patch] FindPkgConfig: optionally create imported target for the found libraries
On 05/13/2016 12:21 PM, Rolf Eike Beer wrote:
>> Can you just use
>>
>> set_property(TARGET PkgConfig::${_prefix} PROPERTY ...)
>>
>> in each `if()` block?
>
> I had this before, but I thought I avoid multiple target lookups. You should
> only ever see that message if none of the branches is taken I think. And that
> should never happen, or the if before is wrong. Can you add a message() and
> show what is actually in _props there?
The problem is that _libs has more than one value, so set_target_properties
ends up with
PROPERTIES PROP1 v1 PROP2 v2a v2b PROP3 v3
^^^
Please use set_property(TARGET). It is much more robust, and this is not
exactly a performance-critical loop.
Thanks,
-Brad
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Patch] FindPkgConfig: optionally create imported target for the found libraries
Am Freitag, 13. Mai 2016, 11:16:28 schrieb Brad King:
> On 05/13/2016 11:02 AM, Rolf Eike Beer wrote:
> > Should all be done now.
>
> Thanks.
>
> The test fails for me with
>
> CMake Error at /.../Modules/FindPkgConfig.cmake:239
> (set_target_properties): set_target_properties called with incorrect number
> of arguments.
> At least part of the problem is here:
> > +if(${_prefix}_INCLUDE_DIRS)
> > + list(APPEND _props INTERFACE_INCLUDE_DIRECTORIES
> > "${${_prefix}_INCLUDE_DIRS}") +endif()
> > +if(_libs)
> > + list(APPEND _props INTERFACE_LINK_LIBRARIES "${_libs}")
> > +endif()
> > +if(${_prefix}_CFLAGS_OTHER)
> > + list(APPEND _props INTERFACE_COMPILE_OPTIONS
> > "${${_prefix}_CFLAGS_OTHER}") +endif()
> > +set_target_properties(PkgConfig::${_prefix} PROPERTIES ${_props})
>
> If any of the property values is empty the `${_props}` will
> remove it from the argument list to set_target_properties.
> Can you just use
>
> set_property(TARGET PkgConfig::${_prefix} PROPERTY ...)
>
> in each `if()` block?
I had this before, but I thought I avoid multiple target lookups. You should
only ever see that message if none of the branches is taken I think. And that
should never happen, or the if before is wrong. Can you add a message() and
show what is actually in _props there?
Greetings,
Eike
signature.asc
Description: This is a digitally signed message part.
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Patch] FindPkgConfig: optionally create imported target for the found libraries
On 05/13/2016 11:02 AM, Rolf Eike Beer wrote:
> Should all be done now.
Thanks.
The test fails for me with
CMake Error at /.../Modules/FindPkgConfig.cmake:239 (set_target_properties):
set_target_properties called with incorrect number of arguments.
At least part of the problem is here:
> +if(${_prefix}_INCLUDE_DIRS)
> + list(APPEND _props INTERFACE_INCLUDE_DIRECTORIES
> "${${_prefix}_INCLUDE_DIRS}")
> +endif()
> +if(_libs)
> + list(APPEND _props INTERFACE_LINK_LIBRARIES "${_libs}")
> +endif()
> +if(${_prefix}_CFLAGS_OTHER)
> + list(APPEND _props INTERFACE_COMPILE_OPTIONS
> "${${_prefix}_CFLAGS_OTHER}")
> +endif()
> +set_target_properties(PkgConfig::${_prefix} PROPERTIES ${_props})
If any of the property values is empty the `${_props}` will
remove it from the argument list to set_target_properties.
Can you just use
set_property(TARGET PkgConfig::${_prefix} PROPERTY ...)
in each `if()` block?
Thanks,
-Brad
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Patch] FindPkgConfig: optionally create imported target for the found libraries
Am Donnerstag, 12. Mai 2016, 13:17:42 schrieb Brad King: > On 05/12/2016 12:48 PM, Rolf Eike Beer wrote: > > Good point. I have changed this accordingly and pushed it to next. There > > are no tests yet, so maybe not immediately merge it to next but wait for > > the weekend or so. > > Okay, thanks. Please also add a Help/release/dev/FindPkgConfig-targets.rst > file with a release note for the feature. Should all be done now. Greetings, Eike signature.asc Description: This is a digitally signed message part. -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Patch] FindPkgConfig: optionally create imported target for the found libraries
On 05/12/2016 12:48 PM, Rolf Eike Beer wrote: > Good point. I have changed this accordingly and pushed it to next. There are > no tests yet, so maybe not immediately merge it to next but wait for the > weekend or so. Okay, thanks. Please also add a Help/release/dev/FindPkgConfig-targets.rst file with a release note for the feature. Thanks, -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Patch] FindPkgConfig: optionally create imported target for the found libraries
Am Donnerstag, 12. Mai 2016, 09:53:06 schrieb Brad King:
> On 05/12/2016 09:34 AM, Rolf Eike Beer wrote:
> > find_library(${_prefix}-${CMAKE_MATCH_1}
> >
> > NAMES ${CMAKE_MATCH_1}
> > ${_find_opts})
> >
> > list(APPEND _libs "${${_prefix}-${CMAKE_MATCH_1}}")
>
> Generally I try to store the CMAKE_MATCH_ values in other variables
> as soon as possible and then use those other variables. Otherwise
> refactoring may cause their values to change and break existing logic.
Good point. I have changed this accordingly and pushed it to next. There are
no tests yet, so maybe not immediately merge it to next but wait for the
weekend or so.
Greetings,
Eike
signature.asc
Description: This is a digitally signed message part.
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Patch] FindPkgConfig: optionally create imported target for the found libraries
On 05/12/2016 09:34 AM, Rolf Eike Beer wrote:
> find_library(${_prefix}-${CMAKE_MATCH_1}
> NAMES ${CMAKE_MATCH_1}
> ${_find_opts})
> list(APPEND _libs "${${_prefix}-${CMAKE_MATCH_1}}")
Generally I try to store the CMAKE_MATCH_ values in other variables
as soon as possible and then use those other variables. Otherwise
refactoring may cause their values to change and break existing logic.
Thanks,
-Brad
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Patch] FindPkgConfig: optionally create imported target for the found libraries
Am 2016-05-12 14:59, schrieb Brad King:
On 05/11/2016 05:52 PM, Rolf Eike Beer wrote:
It has always nagged me that the FindPkgConfig module requires people
to use
link_directories(). Now I created a new optional mode for
pkg_check_modules()
and pkg_search_modules() which will search the absolute paths of the
libraries
that are returned by pkg-config, and create an imported target from
that
information that also contains defines and include directories. It
restricts
searching to the directories returned by pkg-config, if none are given
the
normal search rules are used. I have manually tested this and it seems
to
work. Please have a look and tell me if I have missed something before
I put
this into next.
Great! I've long wanted to see this done.
+if (flag MATCHES "^-l(.*)")
+ set(_pkg_search "${CMAKE_MATCH_1}")
+else()
+ continue()
+endif()
+
+find_library(${_prefix}-${CMAKE_MATCH_1}
+ NAMES ${CMAKE_MATCH_1}
+ ${_find_opts})
+list(APPEND _libs "${${_prefix}-${CMAKE_MATCH_1}}")
Shouldn't these latter ${CMAKE_MATCH_1} references use ${_pkg_search}
instead?
The whole _pkg_search thing can go away, it's from an earlier version of
the patch. This should work the same:
if (NOT flag MATCHES "^-l(.*)")
continue()
endif()
find_library(${_prefix}-${CMAKE_MATCH_1}
NAMES ${CMAKE_MATCH_1}
${_find_opts})
list(APPEND _libs "${${_prefix}-${CMAKE_MATCH_1}}")
Eike
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Patch] FindPkgConfig: optionally create imported target for the found libraries
On 05/11/2016 05:52 PM, Rolf Eike Beer wrote:
> It has always nagged me that the FindPkgConfig module requires people to use
> link_directories(). Now I created a new optional mode for pkg_check_modules()
> and pkg_search_modules() which will search the absolute paths of the
> libraries
> that are returned by pkg-config, and create an imported target from that
> information that also contains defines and include directories. It restricts
> searching to the directories returned by pkg-config, if none are given the
> normal search rules are used. I have manually tested this and it seems to
> work. Please have a look and tell me if I have missed something before I put
> this into next.
Great! I've long wanted to see this done.
> +if (flag MATCHES "^-l(.*)")
> + set(_pkg_search "${CMAKE_MATCH_1}")
> +else()
> + continue()
> +endif()
> +
> +find_library(${_prefix}-${CMAKE_MATCH_1}
> + NAMES ${CMAKE_MATCH_1}
> + ${_find_opts})
> +list(APPEND _libs "${${_prefix}-${CMAKE_MATCH_1}}")
Shouldn't these latter ${CMAKE_MATCH_1} references use ${_pkg_search} instead?
Thanks,
-Brad
--
Powered by www.kitware.com
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Kitware offers various services to support the CMake community. For more
information on each offering, please visit:
CMake Support: http://cmake.org/cmake/help/support.html
CMake Consulting: http://cmake.org/cmake/help/consulting.html
CMake Training Courses: http://cmake.org/cmake/help/training.html
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/mailman/listinfo/cmake-developers
[cmake-developers] [Patch] FindPkgConfig: optionally create imported target for the found libraries
It has always nagged me that the FindPkgConfig module requires people to use
link_directories(). Now I created a new optional mode for pkg_check_modules()
and pkg_search_modules() which will search the absolute paths of the libraries
that are returned by pkg-config, and create an imported target from that
information that also contains defines and include directories. It restricts
searching to the directories returned by pkg-config, if none are given the
normal search rules are used. I have manually tested this and it seems to
work. Please have a look and tell me if I have missed something before I put
this into next.
Greetings,
Eike
From 352e6b2733995dc121fe5d80d1183fe372522124 Mon Sep 17 00:00:00 2001
From: Rolf Eike Beer
Date: Wed, 11 May 2016 23:45:26 +0200
Subject: [PATCH] FindPkgConfig: optionally create imported target for the
found libraries
---
Modules/FindPkgConfig.cmake | 81 +
1 file changed, 74 insertions(+), 7 deletions(-)
diff --git a/Modules/FindPkgConfig.cmake b/Modules/FindPkgConfig.cmake
index 447c526..fd71a7a 100644
--- a/Modules/FindPkgConfig.cmake
+++ b/Modules/FindPkgConfig.cmake
@@ -16,6 +16,7 @@
# Copyright 2006-2014 Kitware, Inc.
# Copyright 2014 Christoph Grüninger
# Copyright 2006 Enrico Scholz
+# Copyright 2016 Rolf Eike Beer
#
# Distributed under the OSI-approved BSD License (the "License");
# see accompanying file Copyright.txt for details.
@@ -119,11 +120,12 @@ macro(_pkgconfig_invoke_dyn _pkglist _prefix _varname
cleanup_regexp)
endmacro()
# Splits given arguments into options and a package list
-macro(_pkgconfig_parse_options _result _is_req _is_silent _no_cmake_path
_no_cmake_environment_path)
+macro(_pkgconfig_parse_options _result _is_req _is_silent _no_cmake_path
_no_cmake_environment_path _imp_target)
set(${_is_req} 0)
set(${_is_silent} 0)
set(${_no_cmake_path} 0)
set(${_no_cmake_environment_path} 0)
+ set(${_imp_target} 0)
if(DEFINED PKG_CONFIG_USE_CMAKE_PREFIX_PATH)
if(NOT PKG_CONFIG_USE_CMAKE_PREFIX_PATH)
set(${_no_cmake_path} 1)
@@ -147,6 +149,9 @@ macro(_pkgconfig_parse_options _result _is_req _is_silent
_no_cmake_path _no_cma
if (_pkg STREQUAL "NO_CMAKE_ENVIRONMENT_PATH")
set(${_no_cmake_environment_path} 1)
endif()
+if (_pkg STREQUAL "IMPORTED_TARGET")
+ set(${_imp_target} 1)
+endif()
endforeach()
set(${_result} ${ARGN})
@@ -154,6 +159,7 @@ macro(_pkgconfig_parse_options _result _is_req _is_silent
_no_cmake_path _no_cma
list(REMOVE_ITEM ${_result} "QUIET")
list(REMOVE_ITEM ${_result} "NO_CMAKE_PATH")
list(REMOVE_ITEM ${_result} "NO_CMAKE_ENVIRONMENT_PATH")
+ list(REMOVE_ITEM ${_result} "IMPORTED_TARGET")
endmacro()
# Add the content of a variable or an environment variable to a list of
@@ -181,8 +187,60 @@ function(_pkgconfig_add_extra_path _extra_paths_var _var)
set(${_extra_paths_var} ${${_extra_paths_var}} PARENT_SCOPE)
endfunction()
+# scan the LDFLAGS returned by pkg-config for library directories and
+# libraries, figure out the absolute paths of that libraries in the
+# given directories, and create an imported target from them
+function(_pkg_create_imp_target _prefix _no_cmake_path
_no_cmake_environment_path)
+ unset(_libs)
+ unset(_find_opts)
+
+ # set the options that are used as long as the .pc file does not provide a
library
+ # path to look into
+ if(_no_cmake_path)
+set(_find_opts "NO_CMAKE_PATH")
+ endif()
+ if(_no_cmake_environment_path)
+set(_find_opts "${_find_opts} NO_CMAKE_ENVIRONMENT_PATH")
+ endif()
+
+ foreach (flag IN LISTS ${_prefix}_LDFLAGS)
+if (flag MATCHES "^-L(.*)")
+ # only look into the given paths from now on
+ set(_find_opts "HINTS ${${CMAKE_MATCH_1}} NO_DEFAULT_PATH")
+ continue()
+endif()
+if (flag MATCHES "^-l(.*)")
+ set(_pkg_search "${CMAKE_MATCH_1}")
+else()
+ continue()
+endif()
+
+find_library(${_prefix}-${CMAKE_MATCH_1}
+ NAMES ${CMAKE_MATCH_1}
+ ${_find_opts})
+list(APPEND _libs "${${_prefix}-${CMAKE_MATCH_1}}")
+ endforeach()
+
+ # only create the target if it is linkable, i.e. no executables
+ if (NOT TARGET PkgConfig::${_prefix}
+ AND ( ${_prefix}_INCLUDE_DIRS OR _libs OR ${_prefix}_CFLAGS_OTHER ))
+add_library(PkgConfig::${_prefix} INTERFACE IMPORTED)
+if(${_prefix}_INCLUDE_DIRS)
+ set_target_properties(PkgConfig::${_prefix} PROPERTIES
+INTERFACE_INCLUDE_DIRECTORIES "${${_prefix}_INCLUDE_DIRS}")
+endif()
+if(_libs)
+ set_target_properties(PkgConfig::${_prefix} PROPERTIES
+INTERFACE_LINK_LIBRARIES "${_libs}")
+endif()
+if(${_prefix}_CFLAGS_OTHER)
+ set_property(TARGET PkgConfig::${_prefix} PROPERTY
INTERFACE_COMPILE_OPTIONS "${${_prefix}_CFLAGS_OTHER}")
+endif()
+ endif()
+endfunction()
+
###
-macro(_pkg_check_modules_internal _is_required _is_silent _no_cmake_path
_n
