On 12. 6. 25 19:10, Daniel Sahlberg wrote:
Den tors 12 juni 2025 kl 00:34 skrev Timofei Zhakov <t...@chemodax.net>:
Revert most of the changes in svn_utf.h from r1926293.
Instead, tell the
CMake build to expose APR and APR-Util include paths everywhere.
(...)
* CMakeLists.txt: Add APR and APR-Util include paths to the
top-level
directory properties.
(...)
--- subversion/trunk/CMakeLists.txt (original)
+++ subversion/trunk/CMakeLists.txt Wed Jun 11 15:40:43 2025
@@ -289,6 +289,12 @@ else()
endif()
endif()
+# APR and APR-Util include directories must be available to
all our souroces,
+# not just those that happen to link with one or the other of
these libraries.
+get_target_property(_apr_include external-apr
INTERFACE_INCLUDE_DIRECTORIES)
+get_target_property(_apu_include external-aprutil
INTERFACE_INCLUDE_DIRECTORIES)
+include_directories(${_apr_include} ${_apu_include})
+
This is wrong. You can't modify global include directories of the
entire project to force it to use apr everywhere. This is a hack,
not a fix.
As far as I understand CMake would add the include directories
automatically on targets that link APR (or APR-U... havn't wrapped my
head around what goes where yet), but the issue is that it doesn't do
that when not linking the libraries - is that correct?
CMake has something called "imported targets", i.e., libraries that the
project uses but doesn't build. CMake targets have various properties,
one of them (the above INTERFACE_INCLUDE_DIRECTORIES) says, basically,
"code that uses this library will find the needed headers here"). That's
simplified, but for C and C++, it's what the property boils down to.
These include paths are only injected into the compiler options when a
target is a dependency of the source that's being compiled. In our case,
what that means is that if one of subversion's libraries or tools
directly links apr-util, for example, the apr-util include path will be
added to CFLAGS (or rather, C_INCLUDES in cmake's world) when the
sources of that library are compiled.
If a source doesn't link with apr-util but /does/ include one of the
apr-util headers, then we have a problem. It wasn't noticed before
because all the GH builders, and probably all the current developers'
setups, have APR and APR-Util installed in the same place; and since all
of our artefacts link with libapr, the apr-util headers were always
available -- even though the the build script doesn't take that into
account.
Homebrew on the Mac installs APR and APR-Util in different directories,
so this issue showed up when I tried to use the CMake build on a mac (in
order to test the patch to FindSerf.cmake with serf-2).
The proximal reason being that some libraries that don't link with
apr_util do use the svn_utf.h header, which includes apr_xlate.h from
apr-util, which wasn't available, which caused a 💥. Those libraries and
tools only use apr-util indirectly, thorough libsvn_subr.
Three possible solutions came to mind:
* Change build.conf, where the library dependencies are defined, to
add the dependency on apr-util. This would cause those targets to
link libaprutil, even when they're not using any symbols from it.
Thad didn't seem right.
* Change svn_utf.h to avoid that dependency ... which we know we
can't, due to backward-compatibility concerns.
* Fix the include paths in CMake.
That last part has several possible solutions. I implemented the most
obvious one, i.e., adding APR and APR-Util include paths to all
Subversion's build targets. It's an obvious solution because APR and
APR-Util are essentially part of Subversion's public interface (because
of our use of pools in the API, and APR's integer types, and containers,
etc.).
[An aside: if we decided to export our CMake targets to make them
directly usable by downstream, those apr-util include paths will have to
be included in our targets' INTERFACE_INCLUDE_DIRECTORIES. This is
essentially what we do in the pkg-config files, too, which are the
equivalent of cmake's exported targets.)
A better way would be to convince CMake to take the whole transitive
dependency tree into account, but, after having spent way too much time
chasing down CMake (3.12) documentation, I admit that I don't know how
to do that.
What is the CMake-ish solution here?
Since we generate the CMake target code from build.conf - can we add a
new parameter that add these include directories only on the targets
that actually require them?
Before someone jumps head-first in to the spaghetti that is our
build.conf generator, answer this question: why should we optimize the
include paths to the absolute minimum required, when that is clearly a
problem now and can't be anything but a problem down the road? I'm
talking specifically about APR and APR-Util include paths, which merge
into one in apr-2 in any case.
-- Brane
