>
> >
> > Hi again,
> >
> > I've tested it. Here is my feedback.
> >
> > Both apr-1 and apr-2 works perfectly on a Unix environment.
> >
> > However, again, dependency search is done in the wrong way on Windows
> > and may work incorrectly in some cases. In cmake, find_library()
> > and find_path() are made so they handle all logic in relation to where
> > to look for each kind of artifact. They will handle system dirs,
> > -DCMAKE_INSTALL_DIR, -DCMAKE_PREFIX_PATH, <MODULENAME>_ROOT (APR_ROOT
> > in this case),
>
> Clearly, then, CMake's own FindOpenSSL.cmake is "doing it wrong",
> because it uses OPENSSL_ROOT_DIR instead of OpenSSL_ROOT... ;)
>
>
I'm not sure, but I think they have changed it at some point and currently
both of them are supported, but one of them is sort of "deprecated".
Also they call find_path() passing several additional paths including one
from the Windows Registry (which is quite weird), but don't prevent
standard library search (as I explained with CMAKE_PREFIX_PATH and
bla-bla-bla).
> > and even cmake's environment variables, and I guess vcpkg also
> > implements some custom things. This functionality is very useful for
> > people who need to set up their environments, to get builds against
> > right libraries etc...
>
> Ack
>
> > I would like to suggest fix with the serf-improve-apr-search.patch patch:
> >
> > [[[
> > cmake: Improve search of APR and APR-Util libraries by
> > calling find_library()
> > and find_path() methods without passing PATH, since cmake could
> > automatically
> > determine it based on platform-dependent factors.
> > ]]]
> >
> > After this patch, I got configuration successfully without modifying
> > settings on Windows which is good for now...
>
> Actually, your patch basically does on Windows what we do on other
> platforms, except that even there we handle *_ROOT ourselves. So it
> looks like we'd actually have to merge a lot of if(SERF_WINDOWS)
> branches into one generic search, which is good. I'll look into that
> when I have a moment.
>
> I'll just say that, when we first started writing CMakeLists.txt,
> find_package in CMake 2 didn't do half the things it does now.
>
Oh, this makes sense...
Thanks for the patch! It pointed me in the right direction.
> -- Brane
Can we actually remove the warning that cmake is an experimental feature or
not yet? I think as it is going to be released, it should not be just an
experimental build system but a secondary one.
I made an addition to the patch, because I forgot to do the same thing to
locate the APR config executable. Updated patch is in the attachments.
And I tested it on both Windows and Unix platforms (config and manual
search).
By the way, I think we should rework it a bit so we will try config first
-- no matter whether the platform is unix like or not, and then, if
pkgconfig or apr-1-config weren't found, try manual search instead (using
find_path and find_library). because technically configs could be also
available on windows. For example, I think vcpkg does something in order to
look for libs through pkgconfig on win32, so this is the case.
--
Timofei Zhakov
Index: build/FindAPR.cmake
===================================================================
--- build/FindAPR.cmake (revision 1926089)
+++ build/FindAPR.cmake (working copy)
@@ -106,8 +106,7 @@
# Windows: Find the DLL (runtime) library
function(_apru_find_dll _varname _dllname)
set(CMAKE_FIND_LIBRARY_SUFFIXES ".dll")
- find_library(${_varname} NAMES ${_dllname}
- PATHS ${ARGN} NO_DEFAULT_PATH PATH_SUFFIXES "bin" "lib")
+ find_library(${_varname} NAMES ${_dllname} PATHS ${ARGN} "bin" "lib")
endfunction(_apru_find_dll)
# Extract the main and extra static libraries
@@ -171,21 +170,15 @@
set(APR_FOUND FALSE)
if(${CMAKE_SYSTEM_NAME} MATCHES "Windows")
-
- if(DEFINED APR_ROOT)
- get_filename_component(APR_ROOT "${APR_ROOT}" REALPATH)
- else()
- message(FATAL_ERROR "APR_ROOT must be defined on Windows")
- endif()
-
include(CheckIncludeFile)
find_path(APR_INCLUDES "apr.h"
- PATHS "${APR_ROOT}/include"
- PATH_SUFFIXES "apr-2" "apr-1"
- NO_DEFAULT_PATH)
+ PATH_SUFFIXES
+ "include/apr-2"
+ "include/apr-1"
+ "include")
if(NOT APR_INCLUDES)
- message(FATAL_ERROR "apr.h was not found in ${APR_ROOT}")
+ message(FATAL_ERROR "apr.h was not found")
endif()
if(NOT EXISTS "${APR_INCLUDES}/apr_version.h")
@@ -195,21 +188,16 @@
_apru_version(APR_VERSION _apr_major _apr_minor
"${APR_INCLUDES}/apr_version.h" "APR")
set(_apr_name "apr-${_apr_major}")
- find_library(APR_LIBRARIES NAMES "lib${_apr_name}.lib"
- PATHS ${APR_ROOT} NO_DEFAULT_PATH PATH_SUFFIXES "lib")
- find_library(APR_STATIC_LIBS NAMES "${_apr_name}.lib"
- PATHS ${APR_ROOT} NO_DEFAULT_PATH PATH_SUFFIXES "lib")
+ find_library(APR_LIBRARIES NAMES "lib${_apr_name}"
+ PATH_SUFFIXES "lib")
+ find_library(APR_STATIC_LIBS NAMES "${_apr_name}"
+ PATH_SUFFIXES "lib")
_apru_find_dll(APR_RUNTIME_LIBS "lib${_apr_name}.dll" ${APR_ROOT})
else() # NOT Windows
+ find_program(APR_CONFIG_EXECUTABLE NAMES apr-2-config apr-1-config
+ PATH_SUFFIXES "bin")
- if(DEFINED APR_ROOT)
- get_filename_component(APR_ROOT "${APR_ROOT}" REALPATH)
- find_program(APR_CONFIG_EXECUTABLE NAMES apr-2-config apr-1-config
- PATHS "${APR_ROOT}/bin" NO_DEFAULT_PATH)
- else()
- find_program(APR_CONFIG_EXECUTABLE NAMES apr-2-config apr-1-config)
- endif()
mark_as_advanced(APR_CONFIG_EXECUTABLE)
macro(_apr_invoke _varname _regexp)
Index: build/FindAPRUtil.cmake
===================================================================
--- build/FindAPRUtil.cmake (revision 1926089)
+++ build/FindAPRUtil.cmake (working copy)
@@ -78,21 +78,13 @@
unset(_apru_include_only_utilities)
if(${CMAKE_SYSTEM_NAME} MATCHES "Windows")
-
- if(DEFINED APRUtil_ROOT)
- get_filename_component(APRUtil_ROOT "${APRUtil_ROOT}" REALPATH)
- else()
- message(FATAL_ERROR "APRUtil_ROOT must be defined on Windows")
- endif()
-
include(CheckIncludeFile)
find_path(APRUTIL_INCLUDES "apu.h"
- PATHS "${APRUtil_ROOT}/include"
- PATH_SUFFIXES "apr-1"
- NO_DEFAULT_PATH)
+ PATH_SUFFIXES "include/apr-1"
+ PATH_SUFFIXES "include")
if(NOT APRUTIL_INCLUDES)
- message(FATAL_ERROR "apu.h was not found in ${APRUtil_ROOT}")
+ message(FATAL_ERROR "apu.h was not found")
endif()
if(NOT EXISTS "${APRUTIL_INCLUDES}/apu_version.h")
@@ -109,11 +101,11 @@
endif()
find_library(APRUTIL_LIBRARIES NAMES "lib${_apu_name}.lib"
- PATHS ${APRUtil_ROOT} NO_DEFAULT_PATH PATH_SUFFIXES "lib")
+ PATH_SUFFIXES "lib")
find_library(_apu_static NAMES "${_apu_name}.lib"
- PATHS ${APRUtil_ROOT} NO_DEFAULT_PATH PATH_SUFFIXES "lib")
+ PATH_SUFFIXES "lib")
find_library(_apu_expat NAMES ${_apu_expat_name}
- PATHS ${APRUtil_ROOT} NO_DEFAULT_PATH PATH_SUFFIXES "lib")
+ PATH_SUFFIXES "lib")
_apru_find_dll(APRUTIL_RUNTIME_LIBS "lib${_apu_name}.dll" ${APRUtil_ROOT})
if(NOT _apu_expat AND (_apu_expat_name MATCHES "expat"))
@@ -130,14 +122,9 @@
CACHE STRING "APR-Util static libraies.")
else() # NOT Windows
+ find_program(APRUTIL_CONFIG_EXECUTABLE apu-1-config
+ PATH_SUFFIXES "bin")
- if(DEFINED APRUtil_ROOT)
- get_filename_component(APRUtil_ROOT "${APRUtil_ROOT}" REALPATH)
- find_program(APRUTIL_CONFIG_EXECUTABLE apu-1-config
- PATHS "${APRUtil_ROOT}/bin" NO_DEFAULT_PATH)
- else()
- find_program(APRUTIL_CONFIG_EXECUTABLE apu-1-config)
- endif()
mark_as_advanced(APRUTIL_CONFIG_EXECUTABLE)
macro(_apu_invoke _varname _regexp)
