[
https://issues.apache.org/jira/browse/LOGCXX-510?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17189028#comment-17189028
]
Thorsten Schöning edited comment on LOGCXX-510 at 9/2/20, 7:04 AM:
-------------------------------------------------------------------
> 2. The directories for APR and APR-Util are found using the FindAPR.cmake and
> FindAPR-Util.cmake in src/cmake.
After comparing those files, I have the feeling the handling of XMLLIB_DLL in
APU was simply wrong: APR_LIBRARIES and APR_DLL and the same for APU are
defined in different branches than the same for XML. If I change that order, I
get the expected config options in Visual Studio and am able to run the tests.
The wrong order easily explains why
"get_filename_component(XMLLIB_DLL_DIR[...])" has been available at all. Look
at the following commit:
https://github.com/apache/logging-log4cxx/commit/37d3da3cadfc46f1d4414a9ffa14c92fd0f9701b
{noformat}
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)
find_library(XMLLIB_LIBRARIES NAMES expat)
else()
find_library(APR_UTIL_LIBRARIES NAMES libaprutil-1)
find_program(APR_UTIL_DLL libaprutil-1.dll)
find_library(XMLLIB_LIBRARIES NAMES libexpatd)
find_program(XMLLIB_DLL libexpatd.dll)
endif()
{noformat}
> 1. Provide a configuration option to allow the user to input a custom path
> that will be prepended to the PATH before running the tests(this is likely
> needed for the tests to work with zip/grep/sed anyway).
That's not needed with the above change anymore, APR+APU+XML are handled the
exact same way now. So either things work automatically for all three or need
to be configured manually for all three. That totally makes sense in my opinion.
This only leaves handling of SED etc. and one can easily argue that those need
to be available in PATH of the executing user. That is easy to do on the shell
if necessary and on many systems this might be the case by default already
anyway. So if at all, one would only need some TEST_HELPER_PATH or something to
add to PATH. But in that case it would be important that this variable gets
"asked for" by Visual Studio like the ones above. This would allow exceptions
form the rule like me to configure that path in VS, things get saved in
CMakeSettings.json and everything would be fine.
was (Author: tschoening):
> 2. The directories for APR and APR-Util are found using the FindAPR.cmake and
> FindAPR-Util.cmake in src/cmake.
After comparing those files, I have the feeling the handling of XMLLIB_DLL in
APU was simply wrong: APR_LIBRARIES and APR_DLL and the same for APU are
defined in different branches than the same for XML. If I change that order, I
get the expected config options in Visual Studio and am able to run the tests.
The wrong order easily explains why
"get_filename_component(XMLLIB_DLL_DIR[...])" has been available at all. Look
at the following commit:
https://github.com/apache/logging-log4cxx/commit/37d3da3cadfc46f1d4414a9ffa14c92fd0f9701b
{noformat}
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)
find_library(XMLLIB_LIBRARIES NAMES expat)
else()
find_library(APR_UTIL_LIBRARIES NAMES libaprutil-1)
find_program(APR_UTIL_DLL libaprutil-1.dll)
find_library(XMLLIB_LIBRARIES NAMES libexpatd)
find_program(XMLLIB_DLL libexpatd.dll)
endif()
{noformat}
> 1. Provide a configuration option to allow the user to input a custom path
> that will be prepended to the PATH before running the tests(this is likely
> needed for the tests to work with zip/grep/sed anyway).
That's not needed with the above change anymore, APR+APU+XML are handled the
exact same way now. So either things work automatically for all three or need
to be configured manually for all three. That totally makes sense in my opinion.
This only leaves handling of SED etc. and one can easily argue that those need
to be available in PATH of the executing user. That is easy to do on the shell
if necessary and on many systems this might be the case by default already
anyway. So if at all, one would only need some TEST_HELPER_PATH or something to
add to PATH. But in that case it would be important that this variable gets
"asked for" by Visual Studio like the ones above. This would allow exceptions
form the rule like me to configure that path in VS, things get saved in
CMakeSettings.json and all would be fine.
> Build problems using CMAKE and Visual Studio 2019 Community
> -----------------------------------------------------------
>
> Key: LOGCXX-510
> URL: https://issues.apache.org/jira/browse/LOGCXX-510
> Project: Log4cxx
> Issue Type: Bug
> Components: Build
> Affects Versions: 0.11.0
> Reporter: Thorsten Schöning
> Assignee: Thorsten Schöning
> Priority: Major
> Attachments: CMP00079.png, CMakeSettings.json,
> cmake_settings_using_vs_gui.zip, missing_log4cxx_rc.png,
> set_target_properties.png
>
>
> I just tested building log4cxx using CMAKE and Visual Studio 2019 Community.
> Things failed and I want to use this bug to document everything that I've
> found, because for some things I'm not sure how to handle them properly. Will
> create a new branch to maintain the fixes I came along, so that those can at
> best be later reviewed by others. The branch of interest is currently named
> "logcxx_510_cmake_vs2019_compat".
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
