On 05/03/14 17:36, Brad King wrote: > 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`
Done. I noticed that the CMAKE_FRAMEWORK_PATH and CMAKE_APPBUNDLE_PATH don't have documentation, but to be honest I don't know exactly what's the meaning of these two variables, I just saw that find_package was using them, so I thought it was reasonable to use them here too. Am I supposed to add documentation for the variable PKG_CONFIG_USE_CMAKE_PREFIX_PATH, that is checked by this patch? If yes, in Help/variable/PKG_CONFIG_USE_CMAKE_PREFIX_PATH.rst, Help/prop_gbl/PKG_CONFIG_USE_CMAKE_PREFIX_PATH.rst or in which other file? > 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... 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? 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
