[GitHub] [logging-log4cxx] rm5248 commented on a diff in pull request #133: prefer pkg-config over apu-1-config executable

[GitBox <gi...@apache.org>]

rm5248 commented on code in PR #133:
URL: https://github.com/apache/logging-log4cxx/pull/133#discussion_r979056630


##########
src/cmake/FindAPR-Util.cmake:
##########
@@ -27,30 +27,40 @@ macro(_apu_invoke _varname)
     endif(_apr_failed)
 endmacro(_apu_invoke)
 
-find_program(APR_UTIL_CONFIG_EXECUTABLE
-    apu-1-config
-    PATHS /usr/local/bin    /usr/local/opt/apr-util/bin    /usr/bin    $ENV{ProgramFiles}/apr-util/bin
-    )
-mark_as_advanced(APR_UTIL_CONFIG_EXECUTABLE)
-if(EXISTS ${APR_UTIL_CONFIG_EXECUTABLE})
-    _apu_invoke(APR_UTIL_INCLUDE_DIR   --includedir)
-    if (APU_STATIC OR NOT BUILD_SHARED_LIBS)
-      _apu_invoke(_apu_util_link_args  --link-ld)
-      string(REGEX MATCH "-L([^ ]+)" _apu_util_L_flag ${_apu_util_link_args})
-      find_library(APR_UTIL_LIBRARIES NAMES libaprutil-1.a PATHS "${CMAKE_MATCH_1}")
-      set(APR_UTIL_COMPILE_DEFINITIONS APU_DECLARE_STATIC)
-    else()
-      _apu_invoke(APR_UTIL_LIBRARIES   --link-ld)
-    endif()
+find_package(PkgConfig)
+pkg_check_modules(APR_UTIL IMPORTED_TARGET apr-util-1)
+
+if(TARGET PkgConfig::APR_UTIL)
+  message(STATUS "APR_UTIL debug: libs ${APR_UTIL_LIBRARIES} include ${APR_UTIL_INCLUDE_DIRS}")
+  execute_process(COMMAND ${PKG_CONFIG_EXECUTABLE} --debug --cflags apr-util-1)
+  execute_process(COMMAND ls -lR ${APR_UTIL_INCLUDE_DIRS})
+  # set(APR_UTIL_LIBRARIES PkgConfig::APR_UTIL)
 else()
-    find_path(APR_UTIL_INCLUDE_DIR apu.h PATH_SUFFIXES apr-1)
-    if (APU_STATIC OR NOT BUILD_SHARED_LIBS)
-      set(APR_UTIL_COMPILE_DEFINITIONS APU_DECLARE_STATIC)
-      find_library(APR_UTIL_LIBRARIES NAMES aprutil-1)
-    else()
-      find_library(APR_UTIL_LIBRARIES NAMES libaprutil-1)
-      find_program(APR_UTIL_DLL libaprutil-1.dll)
-    endif()
+  find_program(APR_UTIL_CONFIG_EXECUTABLE
+      apu-1-config
+      PATHS /usr/local/opt/apr-util/bin    $ENV{ProgramFiles}/apr-util/bin

Review Comment:
   Is there a reason to remove the other PATHS from here?  If we need to fallback to using apu-1-config(and apr-1-config) it seems like we should check on the normal PATH directories and not limit ourselves to just `/usr/local/opt/...`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org