cmake: Install cmake files in a more common path #366
Conversation
Install cmake files in prefix/lib/cmake/pcre2 rather than prefix/cmake
| @@ -1124,7 +1124,7 @@ configure_file(${PCRE2_CONFIG_IN} ${PCRE2_CONFIG_OUT} @ONLY) | |||
| set(PCRE2_CONFIG_VERSION_IN ${CMAKE_CURRENT_SOURCE_DIR}/cmake/pcre2-config-version.cmake.in) | |||
| set(PCRE2_CONFIG_VERSION_OUT ${CMAKE_CURRENT_BINARY_DIR}/cmake/pcre2-config-version.cmake) | |||
| configure_file(${PCRE2_CONFIG_VERSION_IN} ${PCRE2_CONFIG_VERSION_OUT} @ONLY) | |||
| install(FILES ${PCRE2_CONFIG_OUT} ${PCRE2_CONFIG_VERSION_OUT} DESTINATION cmake) | |||
| install(FILES ${PCRE2_CONFIG_OUT} ${PCRE2_CONFIG_VERSION_OUT} DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/pcre2) | |||
There was a problem hiding this comment.
there used to be a time (at least in Linux) when CMAKE_INSTALL_LIBDIR was lib64 for 64bit builds, and I presume avoiding that variance made using a "bitness" agnostic directory, attractive.
I also wonder how that would work in Windows, where the path is usually just "cmake" (usually with some funny case ignoring but preserving variant).
could we check with the rest of the "cmake communitiy" around #115 for advice before merging this backward incompatible change?
There was a problem hiding this comment.
https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-search-procedure suggests that this is viable on both Windows and Unix
There was a problem hiding this comment.
agree it is "viable", the point I was making was how to minimize "backward compatibility" issues
ex: currently it "might" fail to allow locatiing the cmake files if ${CMAKE_INSTALL_LIBDIR} is (arguibly incorrectly) set to lib64 and the system is Windows. Making this change ONLY if not Windows would "solve" that and IMHO was simple enough to do as shown by:
There was a problem hiding this comment.
I'm not sure what you really want as several have suggested the same thing by now
https://github.com/PCRE2Project/pcre2/pull/260/files#r1225428249
There was a problem hiding this comment.
funny enough you linked to my "ask", which was "what can we put in the release notes to make sure whoever might be affected by this change fixes their setup".
I don't maintan any software that uses pcre2 and that would be using this code, so I am not the best person to provide that.
Indeed, I dont't even know how to create an application that could be affected, but assume one running Windows that uses cmake and linking with a PCRE2 that provides this file (meaning it was installed by running cmake --install instead of the more common way to get PCRE2 installed which is calling make install as "root" while using a POSIX compatible shell), hence maybe mingw64 bash.
There was a problem hiding this comment.
Not sure if this answers your question but vcpkg does this too
https://github.com/microsoft/vcpkg/blob/master/ports/pcre2/portfile.cmake#L54
There was a problem hiding this comment.
some of the patches we are discusing were tested with vcpkg but had to be reverted before 10.43RC1 because they broke autotools bootstrapping.
it doesn't answer though how to document the hopefully small backward incompatibility that they introduce in their current form.
|
As long as you're making a change like this, you might as well make the path configurable via a cache variable like include(GNUInstallDirs)
set(PCRE2_INSTALL_CMAKEDIR "${CMAKE_INSTALL_LIBDIR}/cmake/pcre2"
CACHE STRING "Relative path to PCRE2's CMake config files from the installation root")
install(
FILES ${PCRE2_CONFIG_OUT} ${PCRE2_CONFIG_VERSION_OUT}
DESTINATION "${PCRE2_INSTALL_CMAKEDIR}"
)I go into a lot more detail on CMake packaging on my blog here: https://alexreinking.com/blog/building-a-dual-shared-and-static-library-with-cmake.html |
…#428) Historically, pcre2grep has done minor processing of the patterns that were read through the `-f` option. The end result is that for some patterns there are different results depending if they were provided through `-e`, `-f` or as a parameter in the command line. Add a flag that could be provided to skip that processing so that the same pattern file used with other grep implementations could be used directly for the same result.
Install cmake files in prefix/lib/cmake/pcre2 rather than prefix/cmake