You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/09/21 12:25:58 UTC
[GitHub] [logging-log4cxx] DerDakon opened a new pull request, #133: prefer pkg-config over apu-1-config executable
DerDakon opened a new pull request, #133:
URL: https://github.com/apache/logging-log4cxx/pull/133
When calling pkg-config paths that are relative to a sysroot will automatically get the correct prefix, so try using that first.
--
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
[GitHub] [logging-log4cxx] DerDakon commented on pull request #133: prefer pkg-config over apu-1-config executable
Posted by GitBox <gi...@apache.org>.
DerDakon commented on PR #133:
URL: https://github.com/apache/logging-log4cxx/pull/133#issuecomment-1254760334
Yes and no. Yes, it does, but I did not notice it because I had to patch apr-1-config to be able to build apr-util at all. Because cross-building that is it's own problem as it doesn't look into .pc files either.
Will add a follow-up patch to get that right as well.
--
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
[GitHub] [logging-log4cxx] rm5248 merged pull request #133: prefer pkg-config over apu-1-config executable
Posted by GitBox <gi...@apache.org>.
rm5248 merged PR #133:
URL: https://github.com/apache/logging-log4cxx/pull/133
--
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
[GitHub] [logging-log4cxx] rm5248 commented on a diff in pull request #133: prefer pkg-config over apu-1-config executable
Posted by 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
[GitHub] [logging-log4cxx] DerDakon commented on pull request #133: prefer pkg-config over apu-1-config executable
Posted by GitBox <gi...@apache.org>.
DerDakon commented on PR #133:
URL: https://github.com/apache/logging-log4cxx/pull/133#issuecomment-1256900714
> Using pkg-config should probably be preferred but it doesn't seem to work well with OSX for some reason. Is this a problem we can detect and fall back to using `apu-1-config` if the pkgconfig fails?
I will try using just the variables from pkg-config instead of the target. This is in no place worse than we had before, but it can't transport things like additional compile flags that the ```PkgConfig::<target>``` variant can. I would still like to find out where the thing actually is on MacOS, just for clarity. But what is clear so far is that the .pc file there contains at least one completely wrong entry.
--
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
[GitHub] [logging-log4cxx] DerDakon commented on a diff in pull request #133: prefer pkg-config over apu-1-config executable
Posted by GitBox <gi...@apache.org>.
DerDakon commented on code in PR #133:
URL: https://github.com/apache/logging-log4cxx/pull/133#discussion_r979190114
##########
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:
Yes, because ```find_program()``` will look there anyway. These PATHS are just additional search paths.
--
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
[GitHub] [logging-log4cxx] swebb2066 commented on pull request #133: prefer pkg-config over apu-1-config executable
Posted by GitBox <gi...@apache.org>.
swebb2066 commented on PR #133:
URL: https://github.com/apache/logging-log4cxx/pull/133#issuecomment-1254686454
Does the same issue arise with APR?
--
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