Re: APR-util bundled vcpkg port

2025-05-18 Thread Ivan Zhakov
On Sun, 18 May 2025 at 14:18, Timofei Zhakov  wrote:

> On Sun, May 18, 2025 at 12:49 PM Timofei Zhakov  wrote:
>
>> On Sat, 17 May 2025 at 12:07 PM, Graham Leggett  wrote:
>>
>>> On 16 May 2025, at 17:53, Timofei Zhakov  wrote:
>>>
>>> > I've made a patch for apr util where I added a bundled vcpkg port to
>>> test that the latest version can be built correctly according to vcpkg
>>> requirements. The similar thing already exists in the trunk (apr-2) and
>>> stable apr, but doesn't in the apr util.
>>> >
>>> > However, there would be a problem when compiling against apr 1.7.5,
>>> due to the missing header. I PR-ed to the official registry update to
>>> 1.7.6, but they will publish it after approximately a few weeks, so the
>>> github workflow won't work. I did not actually test the workflow, but it
>>> should work, since I copied it from the trunk.
>>>
>>> Would it be possible to add the LDAP module to the apr-util build?
>>
>>
>>
>> Yeah, it can be done through APR_HAS_LDAP cmake option. However, there is
>> the same problem existing in the trunk, so the same has to be done there as
>> well.
>>
>> I will make a new patch soonly…
>>
>
> This worked for me. Patch is in the attachments.
>
> However, I did notice some kind of strange behaviour of module builds; It
> seems like cmake tries to install their DLLs into the `lib` directory
> instead of `bin`. The same problem existed in for example
> apr_dbd_odbc-1.dll.
>
> [[[
> ...
> -- Performing post-build validation
> D:\tmp\apr-util\build\vcpkg\apr-util\portfile.cmake: warning: The
> following dlls were found in ${CURRENT_PACKAGES_DIR}/lib or
> ${CURRENT_PACKAGES_DIR}/debug/lib. Please move them to
> ${CURRENT_PACKAGES_DIR}/bin or ${CURRENT_PACKAGES_DIR}/debug/bin,
> respectively.
> D:\vcpkg\vcpkg-apr\packages\apr-util_x64-windows: note: the DLLs are
> relative to ${CURRENT_PACKAGES_DIR} here
> note: debug/lib/apr_dbd_odbc-1.dll
> note: lib/apr_dbd_odbc-1.dll
> ...
> ]]]
>
> I remember I had to do something with this in subversion. Let me dig down
> a little bit...
>
> Committed adapted patch to apr-trunk in r1925677 and backported to
apr-util/1.7.x in r1925678.

Thanks!

-- 
Ivan Zhakov


Re: APR-util bundled vcpkg port

2025-05-18 Thread Ivan Zhakov
On Sun, 18 May 2025 at 15:21, Timofei Zhakov  wrote:

> [...]
>
>
>> I remember I had to do something with this in subversion. Let me dig down
>> a little bit...
>>
>
> In the current scenario the problem seems to be due to incorrect
> installation of targets in cmake. Currently, the install is called with
> custom bin and lib directories. However, apr modules are declared as MODULE
> libraries [1] in cmake. The thing is the artifacts this kind of targets
> work in a way so when install is called, the LIBRARY artifacts actually
> include DLL files of the targets. See [2] and the fragment below:
>
> [[[
> ## Library Output Artifacts
>
> A library output artifact of a buildsystem target may be:
>
> - The loadable module file (e.g. .dll or .so) of a module library target
> created by the add_library() command with the MODULE option.
> ...
> ]]]
>
> This means that when changing the LIBRARY property to `lib`, we actually
> put DLL files into this directory, which is incorrect and causes the
> problem.
>
> I'd like to suggest the following; First, there is already a macro for
> declaring modules, so install invocation can be called from there directly,
> using the correct destination path, instead of collecting a list of targets
> to install. In this way we can set up the install properly and resolve the
> issue. Then also a minor improvement in installation sequence can be done,
> since it is required to call it a single time, only for the libaprutil-1
> target.
>
> This worked perfectly for me, and the cmake file seemed a little cleaner
> after this work. See the patch attached to this email, which represents
> those fixes.
>
> [1]
> https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#module-libraries
> [2]
> https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#library-output-artifacts
>
> WDYT?
>
> Oops, my fault.

I fixed it in the same way as in apr-trunk in r1925670.

Thanks!

-- 
Ivan Zhakov


Re: APR-util bundled vcpkg port

2025-05-18 Thread Timofei Zhakov
[...]


> I remember I had to do something with this in subversion. Let me dig down
> a little bit...
>

In the current scenario the problem seems to be due to incorrect
installation of targets in cmake. Currently, the install is called with
custom bin and lib directories. However, apr modules are declared as MODULE
libraries [1] in cmake. The thing is the artifacts this kind of targets
work in a way so when install is called, the LIBRARY artifacts actually
include DLL files of the targets. See [2] and the fragment below:

[[[
## Library Output Artifacts

A library output artifact of a buildsystem target may be:

- The loadable module file (e.g. .dll or .so) of a module library target
created by the add_library() command with the MODULE option.
...
]]]

This means that when changing the LIBRARY property to `lib`, we actually
put DLL files into this directory, which is incorrect and causes the
problem.

I'd like to suggest the following; First, there is already a macro for
declaring modules, so install invocation can be called from there directly,
using the correct destination path, instead of collecting a list of targets
to install. In this way we can set up the install properly and resolve the
issue. Then also a minor improvement in installation sequence can be done,
since it is required to call it a single time, only for the libaprutil-1
target.

This worked perfectly for me, and the cmake file seemed a little cleaner
after this work. See the patch attached to this email, which represents
those fixes.

[1]
https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#module-libraries
[2]
https://cmake.org/cmake/help/latest/manual/cmake-buildsystem.7.html#library-output-artifacts

WDYT?

-- 
Timofei Zhakov
Index: CMakeLists.txt
===
--- CMakeLists.txt  (revision 1925665)
+++ CMakeLists.txt  (working copy)
@@ -287,7 +287,6 @@
 MACRO(ADD_APU_MODULE name dllname sources libraries)
   IF(APU_DSO_BUILD)
 ADD_LIBRARY(${name} MODULE ${sources})
-LIST(APPEND install_targets ${name})
 LIST(APPEND install_bin_pdb $)
 
 TARGET_SOURCES(${name} PRIVATE libaprutil.rc)
@@ -298,6 +297,7 @@
   PRIVATE libaprutil-1 apr::libapr-1)
 TARGET_LINK_LIBRARIES(${name}
   PRIVATE ${libraries})
+INSTALL(TARGETS ${name} DESTINATION ${APU_INSTALL_BIN_DIR})
   ELSE()
 LIST(APPEND APU_EXTRA_SOURCES ${sources})
 LIST(APPEND APU_EXTRA_LIBRARIES ${libraries})
@@ -350,7 +350,11 @@
 ENDIF()
 
 ADD_LIBRARY(libaprutil-1 ${APR_SOURCES} ${APU_EXTRA_SOURCES} 
${APR_PUBLIC_HEADERS_GENERATED})
-LIST(APPEND install_targets libaprutil-1)
+INSTALL(TARGETS libaprutil-1
+RUNTIME DESTINATION ${APU_INSTALL_BIN_DIR}
+LIBRARY DESTINATION ${APU_INSTALL_LIB_DIR}
+ARCHIVE DESTINATION ${APU_INSTALL_LIB_DIR}
+   )
 TARGET_LINK_LIBRARIES(libaprutil-1
   PRIVATE ${XMLLIB_LIBRARIES} ${XLATE_LIBRARIES} 
${APU_EXTRA_LIBRARIES})
 TARGET_INCLUDE_DIRECTORIES(libaprutil-1
@@ -433,14 +437,6 @@
   ADD_DEPENDENCIES(testall memcachedmock)
 ENDIF (APU_BUILD_TEST)
 
-# Installation
-
-INSTALL(TARGETS ${install_targets}
-RUNTIME DESTINATION ${APU_INSTALL_BIN_DIR}
-LIBRARY DESTINATION ${APU_INSTALL_LIB_DIR}
-ARCHIVE DESTINATION ${APU_INSTALL_LIB_DIR}
-   )
-
 IF(INSTALL_PDB)
   INSTALL(FILES ${install_bin_pdb}
   DESTINATION ${APU_INSTALL_BIN_DIR}


Re: APR-util bundled vcpkg port

2025-05-18 Thread Timofei Zhakov
On Sun, May 18, 2025 at 12:49 PM Timofei Zhakov  wrote:

> On Sat, 17 May 2025 at 12:07 PM, Graham Leggett  wrote:
>
>> On 16 May 2025, at 17:53, Timofei Zhakov  wrote:
>>
>> > I've made a patch for apr util where I added a bundled vcpkg port to
>> test that the latest version can be built correctly according to vcpkg
>> requirements. The similar thing already exists in the trunk (apr-2) and
>> stable apr, but doesn't in the apr util.
>> >
>> > However, there would be a problem when compiling against apr 1.7.5, due
>> to the missing header. I PR-ed to the official registry update to 1.7.6,
>> but they will publish it after approximately a few weeks, so the github
>> workflow won't work. I did not actually test the workflow, but it should
>> work, since I copied it from the trunk.
>>
>> Would it be possible to add the LDAP module to the apr-util build?
>
>
>
> Yeah, it can be done through APR_HAS_LDAP cmake option. However, there is
> the same problem existing in the trunk, so the same has to be done there as
> well.
>
> I will make a new patch soonly…
>

This worked for me. Patch is in the attachments.

However, I did notice some kind of strange behaviour of module builds; It
seems like cmake tries to install their DLLs into the `lib` directory
instead of `bin`. The same problem existed in for example
apr_dbd_odbc-1.dll.

[[[
...
-- Performing post-build validation
D:\tmp\apr-util\build\vcpkg\apr-util\portfile.cmake: warning: The following
dlls were found in ${CURRENT_PACKAGES_DIR}/lib or
${CURRENT_PACKAGES_DIR}/debug/lib. Please move them to
${CURRENT_PACKAGES_DIR}/bin or ${CURRENT_PACKAGES_DIR}/debug/bin,
respectively.
D:\vcpkg\vcpkg-apr\packages\apr-util_x64-windows: note: the DLLs are
relative to ${CURRENT_PACKAGES_DIR} here
note: debug/lib/apr_dbd_odbc-1.dll
note: lib/apr_dbd_odbc-1.dll
...
]]]

I remember I had to do something with this in subversion. Let me dig down a
little bit...

-- 
Timofei Zhakov
Index: .github/workflows/windows-vcpkg.yml
===
--- .github/workflows/windows-vcpkg.yml (revision 1925665)
+++ .github/workflows/windows-vcpkg.yml (working copy)
@@ -18,7 +18,7 @@
 port:
 - apr-util
 - apr-util[core]
-- apr-util[crypto,dbd-sqlite3,dbd-odbc,dbd-postgresql,xlate]
+- apr-util[crypto,dbd-sqlite3,dbd-odbc,dbd-postgresql,xlate,ldap]
   fail-fast: false
 
 name: "${{ matrix.port }}:${{ matrix.triplet }} on ${{ matrix.os }}"
Index: build/vcpkg/apr-util/portfile.cmake
===
--- build/vcpkg/apr-util/portfile.cmake (revision 1925665)
+++ build/vcpkg/apr-util/portfile.cmake (working copy)
@@ -18,6 +18,7 @@
 dbd-odbc FEATURE_DBD_ODBC
 dbd-sqlite3 FEATURE_DBD_SQLITE3
 dbd-postgresql FEATURE_DBD_PGQL
+ldap FEATURE_LDAP
 )
 
 vcpkg_cmake_configure(
@@ -29,6 +30,7 @@
 -DAPU_HAVE_ODBC=${FEATURE_DBD_ODBC}
 -DAPU_HAVE_SQLITE3=${FEATURE_DBD_SQLITE3}
 -DAPU_HAVE_PGSQL=${FEATURE_DBD_PGQL}
+-DAPR_HAS_LDAP=${FEATURE_LDAP}
 -DAPU_USE_EXPAT=ON
 )
 
Index: build/vcpkg/apr-util/vcpkg.json
===
--- build/vcpkg/apr-util/vcpkg.json (revision 1925665)
+++ build/vcpkg/apr-util/vcpkg.json (working copy)
@@ -51,6 +51,9 @@
   "dependencies": [
 "libiconv"
   ]
+},
+"ldap": {
+  "description": "LDAP support"
 }
   },
   "default-features": []


Re: APR-util bundled vcpkg port

2025-05-18 Thread Timofei Zhakov
On Sat, 17 May 2025 at 12:07 PM, Graham Leggett  wrote:

> On 16 May 2025, at 17:53, Timofei Zhakov  wrote:
>
> > I've made a patch for apr util where I added a bundled vcpkg port to
> test that the latest version can be built correctly according to vcpkg
> requirements. The similar thing already exists in the trunk (apr-2) and
> stable apr, but doesn't in the apr util.
> >
> > However, there would be a problem when compiling against apr 1.7.5, due
> to the missing header. I PR-ed to the official registry update to 1.7.6,
> but they will publish it after approximately a few weeks, so the github
> workflow won't work. I did not actually test the workflow, but it should
> work, since I copied it from the trunk.
>
> Would it be possible to add the LDAP module to the apr-util build?



Yeah, it can be done through APR_HAS_LDAP cmake option. However, there is
the same problem existing in the trunk, so the same has to be done there as
well.

I will make a new patch soonly…


Re: APR-util bundled vcpkg port

2025-05-17 Thread Graham Leggett via dev
On 16 May 2025, at 17:53, Timofei Zhakov  wrote:

> I've made a patch for apr util where I added a bundled vcpkg port to test 
> that the latest version can be built correctly according to vcpkg 
> requirements. The similar thing already exists in the trunk (apr-2) and 
> stable apr, but doesn't in the apr util.
> 
> However, there would be a problem when compiling against apr 1.7.5, due to 
> the missing header. I PR-ed to the official registry update to 1.7.6, but 
> they will publish it after approximately a few weeks, so the github workflow 
> won't work. I did not actually test the workflow, but it should work, since I 
> copied it from the trunk.

Would it be possible to add the LDAP module to the apr-util build?

Regards,
Graham
--



Re: APR-util bundled vcpkg port

2025-05-16 Thread Timofei Zhakov
On Fri, 16 May 2025 at 8:29 PM, Ivan Zhakov  wrote:

> On Fri, 16 May 2025 at 18:54, Timofei Zhakov  wrote:
>
>> Hi,
>>
>> I've made a patch for apr util where I added a bundled vcpkg port to test
>> that the latest version can be built correctly according to vcpkg
>> requirements. The similar thing already exists in the trunk (apr-2) and
>> stable apr, but doesn't in the apr util.
>>
>> However, there would be a problem when compiling against apr 1.7.5, due
>> to the missing header. I PR-ed to the official registry update to 1.7.6,
>> but they will publish it after approximately a few weeks, so the github
>> workflow won't work. I did not actually test the workflow, but it should
>> work, since I copied it from the trunk.
>>
>> I'm attaching the patch to the email as `apr-util-vcpkg-port-v2.txt`.
>>
>> This work is a little bit rough, and requires testing (I can't test
>> github workflow), but I'm still sharing it.
>>
>> PS: I also support official vcpkg ports for apr during recent time :3
>>
>>
> Committed patch in r1925589 and r1925590 separately.
>
> Thanks a lot!
>

Awesome, thanks!


> --
> Ivan Zhakov
>


Re: APR-util bundled vcpkg port

2025-05-16 Thread Ivan Zhakov
On Fri, 16 May 2025 at 18:54, Timofei Zhakov  wrote:

> Hi,
>
> I've made a patch for apr util where I added a bundled vcpkg port to test
> that the latest version can be built correctly according to vcpkg
> requirements. The similar thing already exists in the trunk (apr-2) and
> stable apr, but doesn't in the apr util.
>
> However, there would be a problem when compiling against apr 1.7.5, due
> to the missing header. I PR-ed to the official registry update to 1.7.6,
> but they will publish it after approximately a few weeks, so the github
> workflow won't work. I did not actually test the workflow, but it should
> work, since I copied it from the trunk.
>
> I'm attaching the patch to the email as `apr-util-vcpkg-port-v2.txt`.
>
> This work is a little bit rough, and requires testing (I can't test github
> workflow), but I'm still sharing it.
>
> PS: I also support official vcpkg ports for apr during recent time :3
>
>
Committed patch in r1925589 and r1925590 separately.

Thanks a lot!

-- 
Ivan Zhakov