On Wed, Jun 11, 2025 at 5:08 PM Branko Čibej <br...@apache.org> wrote:
> On 11. 6. 25 00:07, rin...@apache.org wrote:
>
> Author: rinrab
> Date: Tue Jun 10 22:07:50 2025
> New Revision: 1926350
>
> URL: http://svn.apache.org/viewvc?rev=1926350&view=rev
> Log:
> cmake: Prevent re-definition of APR and Serf targets by using different
> prefixes for version '1' and '2'.
>
> * CMakeLists.txt
> (APR, Serf): Bunch of cmake magic.
>
> Modified:
> subversion/trunk/CMakeLists.txt
>
>
>
> After the pkg-config related changes, cmake will no longer find a Serf on
> macOS. This worked before, but now Serf_ROOT is ignored if we happen to
> find pkg-config. That's because you use either find_package or
> pkg_find_package, but never both. I have to explicitly disable
> SVN_USE_PKG_CONFIG in order to build again. Looks like a regression to me.
>
> -- Brane
>
Check this patch out: *attached*.
In there I've moved pkg-config handling into the module and made it
manually look for the .pc file (for now, only for APR). This means that
find_file will also look in Serf_ROOT as well as the other cmake specific
behaviour will be in place.
However, it has its own disadvantages and may cause several regressions:
- pkgconfig doesn't really like processing absolute paths. It works, but
they may want to maybe look in some platform specific paths which only the
pkgconfig executable may know about.
- The documentation (
https://cmake.org/cmake/help/latest/module/FindPkgConfig.html) says
that moduleSpec should be a *bare module name.* Nothing about a path.
- But our autoconf build system does rely on this behaviour already
(<svn-trunk>/build/ac-macros/serf.m4:153 → serf.m4:163). So, assuming the
cmake module doesn't do anything unusual, we should be fine.
IMHO, there is a place to experiment.
--
Timofei Zhakov
Index: build/cmake/FindAPR.cmake
===================================================================
--- build/cmake/FindAPR.cmake (revision 1926385)
+++ build/cmake/FindAPR.cmake (working copy)
@@ -62,11 +62,17 @@
PATH_SUFFIXES bin
)
+find_path(APR_PKG_CONFIG
+ NAMES "apr-1.pc" "apr-2.pc"
+ PATH_SUFFIXES "pkgconfig"
+)
+
mark_as_advanced(
APR_INCLUDE_DIR
APR_LIBRARY_SHARED
APR_LIBRARY_STATIC
APR_DLL
+ APR_PKG_CONFIG
)
if(APR_LIBRARY_SHARED)
@@ -77,34 +83,40 @@
include(FindPackageHandleStandardArgs)
-FIND_PACKAGE_HANDLE_STANDARD_ARGS(
- APR
- REQUIRED_VARS
- APR_LIBRARY
- APR_INCLUDE_DIR
- VERSION_VAR
- APR_VERSION
-)
+if(APR_PKG_CONFIG AND PKG_CONFIG_FOUND)
+ pkg_check_modules(apr REQUIRED IMPORTED_TARGET ${APR_PKG_CONFIG})
+ add_library(apr::apr INTERFACE IMPORTED)
+ target_link_libraries(apr::apr INTERFACE PkgConfig::apr)
+else()
+ FIND_PACKAGE_HANDLE_STANDARD_ARGS(
+ APR
+ REQUIRED_VARS
+ APR_LIBRARY
+ APR_INCLUDE_DIR
+ VERSION_VAR
+ APR_VERSION
+ )
-if(APR_FOUND AND NOT TARGET apr::apr)
- if (APR_LIBRARY_SHARED)
- add_library(apr::apr SHARED IMPORTED)
- set_target_properties(apr::apr PROPERTIES
- IMPORTED_LOCATION ${APR_DLL}
- IMPORTED_IMPLIB ${APR_LIBRARY}
- INTERFACE_COMPILE_DEFINITIONS "APR_DECLARE_IMPORT"
- )
- else()
- add_library(apr::apr STATIC IMPORTED)
- set_target_properties(apr::apr PROPERTIES
- IMPORTED_LOCATION ${APR_LIBRARY}
- INTERFACE_COMPILE_DEFINITIONS "APR_DECLARE_STATIC"
- )
- endif()
+ if(APR_FOUND AND NOT TARGET apr::apr)
+ if (APR_LIBRARY_SHARED)
+ add_library(apr::apr SHARED IMPORTED)
+ set_target_properties(apr::apr PROPERTIES
+ IMPORTED_LOCATION ${APR_DLL}
+ IMPORTED_IMPLIB ${APR_LIBRARY}
+ INTERFACE_COMPILE_DEFINITIONS "APR_DECLARE_IMPORT"
+ )
+ else()
+ add_library(apr::apr STATIC IMPORTED)
+ set_target_properties(apr::apr PROPERTIES
+ IMPORTED_LOCATION ${APR_LIBRARY}
+ INTERFACE_COMPILE_DEFINITIONS "APR_DECLARE_STATIC"
+ )
+ endif()
- target_include_directories(apr::apr INTERFACE ${APR_INCLUDE_DIR})
+ target_include_directories(apr::apr INTERFACE ${APR_INCLUDE_DIR})
- if (WIN32)
- target_link_libraries(apr::apr INTERFACE ws2_32 rpcrt4)
+ if (WIN32)
+ target_link_libraries(apr::apr INTERFACE ws2_32 rpcrt4)
+ endif()
endif()
endif()
Index: CMakeLists.txt
===================================================================
--- CMakeLists.txt (revision 1926386)
+++ CMakeLists.txt (working copy)
@@ -262,31 +262,14 @@
### APR and APR-Util
-if(SVN_USE_PKG_CONFIG)
- pkg_check_modules(apr1 IMPORTED_TARGET apr-1)
+find_package(APR REQUIRED)
+add_library(external-apr ALIAS apr::apr)
- if(apr1_FOUND)
- # apr-1
- add_library(external-apr ALIAS PkgConfig::apr1)
-
- pkg_check_modules(aprutil-1 REQUIRED IMPORTED_TARGET apr-util-1)
- add_library(external-aprutil ALIAS PkgConfig::aprutil-1)
- else()
- # apr-2
- pkg_check_modules(apr2 REQUIRED IMPORTED_TARGET apr-2)
- add_library(external-apr ALIAS PkgConfig::apr2)
- add_library(external-aprutil ALIAS PkgConfig::apr2)
- endif()
+if(APR_VERSION VERSION_LESS 2.0.0)
+ find_package(APRUtil REQUIRED)
+ add_library(external-aprutil ALIAS apr::aprutil)
else()
- find_package(APR REQUIRED)
- add_library(external-apr ALIAS apr::apr)
-
- if(APR_VERSION VERSION_LESS 2.0.0)
- find_package(APRUtil REQUIRED)
- add_library(external-aprutil ALIAS apr::aprutil)
- else()
- add_library(external-aprutil ALIAS apr::apr)
- endif()
+ add_library(external-aprutil ALIAS apr::apr)
endif()
### ZLIB
