Re: [cmake-developers] [Review request] Topic FindPkgConfig_Extend-PKG_CONFIG_PATH
On 11/03/14 14:51, Brad King wrote: Please add test cases for the FindPkgConfig interface and argument parsing errors. The test can just fake the location of pkg-config with a dummy if necessary. I added some unit test. I'm not done yet, but I would like to see if it works on all the architectures... Can I merge it to next to check the results on the dashboard? I will obviously take care to fix the errors tomorrow... Cheers, Daniele -- 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/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Review request] Topic FindPkgConfig_Extend-PKG_CONFIG_PATH
On 03/13/2014 02:54 PM, Daniele E. Domenichelli wrote: I added some unit test. I'm not done yet, but I would like to see if it works on all the architectures... Can I merge it to next to check the results on the dashboard? I will obviously take care to fix the errors tomorrow... Yes. It should be just one independent test failure. In the worst case you can just revert the commit. Don't worry about noise commits in the middle of a topic. We can squash them out before merging to master. 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/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Review request] Topic FindPkgConfig_Extend-PKG_CONFIG_PATH
On 03/10/2014 10:38 AM, Brad King wrote:
Thanks. Now the docs look good too. Please rebase on master
and merge to 'next' for testing.
Please add test cases for the FindPkgConfig interface and argument
parsing errors. The test can just fake the location of pkg-config
with a dummy if necessary.
Also the hunks:
+if(UNIX)
+ string(REPLACE \\: ; _path ${_path})
+endif()
should not be necessary because TO_CMAKE_PATH already does that.
It looks like TO_NATIVE_PATH does not do the reveres though so that
explicit replacement is still needed.
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/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Review request] Topic FindPkgConfig_Extend-PKG_CONFIG_PATH
On 11/03/14 14:51, Brad King wrote:
Also the hunks:
+if(UNIX)
+ string(REPLACE \\: ; _path ${_path})
+endif()
should not be necessary because TO_CMAKE_PATH already does that.
It looks like TO_NATIVE_PATH does not do the reveres though so that
explicit replacement is still needed.
Done. I also added an extra commit to remove empty values from the list.
I'll add the unit tests as soon as possible.
It is a quite common use case to get and set path environment
variables and taking care of TO_CMAKE_PATH/TO_NATIVE_PATH windows/linux
every time is a bit annoying... It would be really nice to have an easy
way to do this replacement in both ways that automatically takes care of
unix and windows
Perhaps having a syntax like this:
get: $ENV:PATH{VAR}
set: ENV:PATH{VAR}
That automatically convert to local/native path and replaces the ; and
: as necessary on the system.
Do you think this is doable?
Cheers,
Daniele
--
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/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Review request] Topic FindPkgConfig_Extend-PKG_CONFIG_PATH
On 03/11/2014 10:43 AM, Daniele E. Domenichelli wrote: It is a quite common use case to get and set path environment It is not very common within CMake's own modules: $ git grep TO_NATIVE_PATH v3.0.0-rc1 -- Modules |wc -l 2 $ git grep TO_CMAKE_PATH v3.0.0-rc1 -- Modules |wc -l 24 # (and 6 of these are in one module) We don't like to have behavior depend on environment variables more than necessary. They may not still be set when CMake re-runs inside the build system. That is one reason we CACHE the find results. Setting an environment variable for a tool that runs under CMake is a pretty special case in this topic. -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/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Review request] Topic FindPkgConfig_Extend-PKG_CONFIG_PATH
On 03/06/2014 08:15 PM, Daniele E. Domenichelli wrote: I pushed a doc-osx-path-variables topic to the stage to document these variables. You may rebase on that to reference them. Rebased. Thanks. Now the docs look good too. Please rebase on master and merge to 'next' for testing. Thanks, -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Review request] Topic FindPkgConfig_Extend-PKG_CONFIG_PATH
On 03/05/2014 07:47 PM, Daniele E. Domenichelli wrote: I noticed that the CMAKE_FRAMEWORK_PATH and CMAKE_APPBUNDLE_PATH don't have documentation Just leave them as inline-literals for now if there is no doc to link. They can be added later. Am I supposed to add documentation for the variable PKG_CONFIG_USE_CMAKE_PREFIX_PATH, that is checked by this patch? It looks like you've updated the docs further to use the variable directive. Good. Do you mind flipping the order of the patches to revise the documentation formatting first and then make the functional change? That will make it easier to see the actual change. Also it looks like some logic is taken from GNUInstallDirs. Is there enough in common to try to factor that out into a helper module? I just noticed that there is already a global property FIND_LIBRARY_USE_LIB64_PATHS. Perhaps there should be one similar for debian and derivatives (CMAKE_LIBRARY_USE_LIB_ARCHITECTURE_PATHS?) Both GNUInstallDirs and FindPkgConfig could then benefit from them... Perhaps. The global property was from a brief design era when we thought about moving all platform information from variables to properties. It never happened but FIND_LIBRARY_USE_LIB64_PATHS ended up that way. The new one you propose could be a plain variable. The logic if(CMAKE_SYSTEM_NAME MATCHES ^(Linux|kFreeBSD|GNU)$ AND NOT CMAKE_CROSSCOMPILING) if (EXISTS /etc/debian_version) # is this a debian system ? could then be factored over into the platform info modules. Also at the moment on platforms that use lib64 the patch check both lib and lib64, perhaps it should check lib64 only? But locally installed libraries usually just install in prefix/lib, not lib64... But perhaps lib64 be searched first... What do you think? The find_library and find_package commands implement FIND_LIBRARY_USE_LIB64_PATHS by searching lib64 and then lib. This should honor FIND_LIBRARY_USE_LIB64_PATHS too. Look at the find_package implementation: http://cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmFindPackageCommand.cxx;hb=v3.0.0-rc1#l2077 for how it does lib/arch, then lib64, then lib. Thanks, -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Review request] Topic FindPkgConfig_Extend-PKG_CONFIG_PATH
On 06/03/14 17:25, Brad King wrote: Am I supposed to add documentation for the variable PKG_CONFIG_USE_CMAKE_PREFIX_PATH, that is checked by this patch? It looks like you've updated the docs further to use the variable directive. Good. Do you mind flipping the order of the patches to revise the documentation formatting first and then make the functional change? That will make it easier to see the actual change. Done. The find_library and find_package commands implement FIND_LIBRARY_USE_LIB64_PATHS by searching lib64 and then lib. This should honor FIND_LIBRARY_USE_LIB64_PATHS too. Look at the find_package implementation: http://cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmFindPackageCommand.cxx;hb=v3.0.0-rc1#l2077 for how it does lib/arch, then lib64, then lib. For now I reverted the order, but I'm still using the same checks and logic from GNUInstallDirs. Should I change it and use FIND_LIBRARY_USE_LIB64_PATHS instead? It is now something like this: if (debian) add lib/arch elseif (64 bit or unknown) add lib64 add lib (The reason for the unknown is that if project(foo NONE) is used, CMAKE_SIZEOF_VOID_P is not defined, and therefore we cannot use it to know if we are on a 64 bit platform.) The logic is slightly different from find_package that just does if (defined arch) add lib/arch if (UseLib64Paths) add lib64 add lib Should I change the logic as well to follow find_package? Cheers, Daniele -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Review request] Topic FindPkgConfig_Extend-PKG_CONFIG_PATH
On 03/06/2014 01:22 PM, Daniele E. Domenichelli wrote: For now I reverted the order, but I'm still using the same checks and logic from GNUInstallDirs. Should I change it and use FIND_LIBRARY_USE_LIB64_PATHS instead? Yes, I think so, because GNUInstallDirs is about install destinations and FIND_LIBRARY_USE_LIB64_PATHS is about search locations. In this case we want to influence search locations. if (debian) add lib/arch elseif (64 bit or unknown) add lib64 add lib That looks good except for FIND_LIBRARY_USE_LIB64_PATHS. The logic is slightly different from find_package that just does if (defined arch) add lib/arch if (UseLib64Paths) add lib64 add lib Actually UseLib64Paths depends on CMAKE_SIZEOF_VOID_P too. See the call to PlatformIs64Bit when setting it. Thanks, -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Review request] Topic FindPkgConfig_Extend-PKG_CONFIG_PATH
On 03/06/2014 01:22 PM, Daniele E. Domenichelli wrote: On 06/03/14 17:25, Brad King wrote: revise the documentation formatting first Done. Thanks. Here are a couple comments: +.. command:: pkg_check_modules + +Checks for all the given modules. The text defining the command needs to be indented to be part of the explicit markup block. +.. code-block:: cmake + + pkg_check_modules(PREFIX [REQUIRED] [QUIET] MODULE [MODULE]*) The documentation style guide: http://www.cmake.org/cmake/help/v3.0/manual/cmake-developer.7.html#style says that command signatures should be plain literal blocks and not cmake code-blocks. Similar fixes will be needed throughout the module. Thanks, -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Review request] Topic FindPkgConfig_Extend-PKG_CONFIG_PATH
On 03/06/2014 11:25 AM, Brad King wrote: On 03/05/2014 07:47 PM, Daniele E. Domenichelli wrote: I noticed that the CMAKE_FRAMEWORK_PATH and CMAKE_APPBUNDLE_PATH don't have documentation Just leave them as inline-literals for now if there is no doc to link. They can be added later. I pushed a doc-osx-path-variables topic to the stage to document these variables. You may rebase on that to reference them. Thanks, -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Review request] Topic FindPkgConfig_Extend-PKG_CONFIG_PATH
if (debian) add lib/arch elseif (64 bit or unknown) add lib64 add lib That looks good except for FIND_LIBRARY_USE_LIB64_PATHS. Fixed. Now the logic is if (debian) add lib/arch elseif (FIND_LIBRARY_USE_LIB64_PATHS) add lib64 add lib The text defining the command needs to be indented to be part of the explicit markup block. [...] command signatures should be plain literal blocks and not cmake code-blocks. Done and done. I pushed a doc-osx-path-variables topic to the stage to document these variables. You may rebase on that to reference them. Rebased. Cheers, Daniele -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
Re: [cmake-developers] [Review request] Topic FindPkgConfig_Extend-PKG_CONFIG_PATH
On 03/04/2014 10:07 PM, Daniele E. Domenichelli wrote: Follow up to this thread: http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/8369 Please review the topic FindPkgConfig_Extend-PKG_CONFIG_PATH. Nice! Please revise the documentation to use proper cross-reference syntax to link to other variables instead of just inline literals: ``SOME_VARIABLE`` = :variable:`SOME_VARIABLE` Also it looks like some logic is taken from GNUInstallDirs. Is there enough in common to try to factor that out into a helper module? Thanks, -Brad -- Powered by www.kitware.com Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Follow this link to subscribe/unsubscribe: http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
[cmake-developers] [Review request] Topic FindPkgConfig_Extend-PKG_CONFIG_PATH
Dear CMake developers,
Follow up to this thread:
http://thread.gmane.org/gmane.comp.programming.tools.cmake.devel/8369
Please review the topic FindPkgConfig_Extend-PKG_CONFIG_PATH.
FindPkgConfig: Extend PKG_CONFIG_PATH using CMake variables
Use CMAKE_PREFIX_PATH, CMAKE_FRAMEWORK_PATH, and CMAKE_APPBUNDLE_PATH
cache and environment variables to extend PKG_CONFIG_PATH before calling
pkg-config.
In each of the path in these variables it searches for lib/pkgconfig.
Then, depending on the system, it searches for
lib/${CMAKE_LIBRARY_ARCHITECTURE}/pkgconfig (debian) or for
lib64/pkgconfig (other 64 bit unixes). If any of these path is found,
it is appended to the PKG_CONFIG_PATH enviromnent variable.
Add two new arguments to the pkg_check_module and pkg_search_module
macro, NO_CMAKE_PATH and NO_CMAKE_ENVIRONMENT_PATH. The new signature
are therefore:
pkg_check_modules(PREFIX [REQUIRED] [QUIET]
[NO_CMAKE_PATH] [NO_CMAKE_ENVIRONMENT_PATH]
MODULE [MODULE]*)
pkg_search_module(PREFIX [REQUIRED] [QUIET]
[NO_CMAKE_PATH] [NO_CMAKE_ENVIRONMENT_PATH]
MODULE [MODULE]*)
By default, if CMAKE_MINIMUM_REQUIRED_VERSION is 3.1 or later (in
order to keep compatibility with the previous behavior), or if
PKG_CONFIG_USE_CMAKE_PREFIX_PATH is set, the CMAKE_PREFIX_PATH,
CMAKE_FRAMEWORK_PATH, and CMAKE_APPBUNDLE_PATH cache and environment
variables will be added to pkgconfig search path.
The NO_CMAKE_PATH and NO_CMAKE_ENVIRONMENT_PATH arguments disable this
behavior for the cache variables and the environment variables,
respectively, similarly to the find_package() command.
Thanks.
Regards,
Daniele
--
Powered by www.kitware.com
Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html
Please keep messages on-topic and check the CMake FAQ at:
http://www.cmake.org/Wiki/CMake_FAQ
Follow this link to subscribe/unsubscribe:
http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers
