On 5. 7. 25 19:44, Daniel Sahlberg wrote:
Den tis 1 juli 2025 kl 00:37 skrev Branko Čibej <br...@apache.org>:
On 30. 6. 25 23:53, Graham Leggett wrote:
> On 30 Jun 2025, at 09:31, br...@apache.org wrote:
>
>> Author: brane
>> Date: Mon Jun 30 08:31:43 2025
>> New Revision: 1926869
>>
>> URL: http://svn.apache.org/viewvc?rev=1926869&view=rev
<http://svn.apache.org/viewvc?rev=1926869&view=rev>
>> Log:
>> In the CMake build, make MockHTTPinC a separats static library
and change
>> flags to use C99 and silence all warnings only for that target.
>>
>> * test/CMakeLists.txt: Include the MockHTTPinC subdirectory and
don't
>> fiddle with the CMAKE_C_FLAGS.
>> (TEST_ALL_SOURCES): Remove the MockHTTPinC source files.
>> (test_all): Depend on an link the mockhttpinc library.
>>
>> * test/MockHTTPinC/CMakeLists.txt: New. Build the mockhttpinc
library.
>
> I'm getting this sudden build failure on RHEL9, looks like some
header
> files are being included in the wrong order if google is to be
> believed. Do you see anything like this?
>
> [ 88%] Building C object
> test/MockHTTPinC/CMakeFiles/mockhttpinc.dir/MockHTTP.c.o
>
> In file included from /usr/include/apr-1/apr.h:19,
>
> from /usr/include/apr-1/apr_pools.h:43,
>
> from
>
/home/minfrin/src/apache/sandbox/serf/test/MockHTTPinC/MockHTTP_private.h:19,
>
> from
>
/home/minfrin/src/apache/sandbox/serf/test/MockHTTPinC/MockHTTP.c:16:
>
> /usr/include/apr-1/apr-x86_64.h:632:2: error: #error no decision
has
> been made on APR_PATH_MAX for your platform
>
> 632 | #errorno decision has been made on APR_PATH_MAX for your
platform
>
> | ^~~~~
>
I get the same problem on Rocky Linux 9.
$ rm -rf out; svn up -r1926868 && cmake -B out && cmake --build out
[... succeeds ...]
$ rm -rf out; svn up -r1926869 && cmake -B out && cmake --build out
[... fails ...]
No, I'm not seeing that, and I tested on macOS, Debian 12 and
Windows.
Looking at the include trace, the headers are included are in the
correct order. APR_PATH_MAX should be defined in apr.h, based on
PATH_MAX from <limits.h>.
apr-x86_64.h is not an APR header, so I can't even begin to guess
what
was patched on RHEL, sorry. I can't imagine in which universe it
wouldn't have a standard <limits.h> header.
On RHEL it seems like apr.h is only a stub to include an architecture
specific header, a comment in the beginning says "This file is here to
prevent a file conflict on multiarch systems. A conflict will occur
because apr.h has arch-specific definitions." and it then goes on to
#if defined(__i386__)
#include "apr-i386.h"
[...]
#elif defined(__x86_64__)
#include "apr-x86_64.h"
[...]
#endif
apr-x86_64.h seems to be the "normal" generated apr.h from APR, with
the suitable arch-dependent defines, such as the substitution of
#define APR_SIZEOF_VOIDP @voidp_size@
I don't think RedHat's special-handling of the header is to blame here.
By the way, all I did was restrict the -std=c99 to the MockHTTP
sources;
before this change, all the tests, including MockHTTP, used C99. So
there's really no change in the context of that source file.
-- Brane
I exported the compile commands (-DCMAKE_EXPORT_COMPILE_COMMANDS=ON)
and compared the command lines and there are differences in the
command line:
[[[
--- compile_commands_r1926868.txt
+++ compile_commands_r1926869.txt
[...]
- "command": "/usr/bin/cc -DHAVE_STDBOOL_H=1 -DMOCKHTTP_OPENSSL
-DOPENSSL_NO_STDIO -DSERF_HAVE_BROTLI
-DSERF_HAVE_BROTLI_DECODER_RESULT -DSERF_HAVE_GSSAPI
-DSERF_HAVE_OPENSSL_ALPN -DSERF_HAVE_OPENSSL_MALLOC_INIT
-DSERF_HAVE_OPENSSL_SSL_LIBRARY_INIT -DSERF_HAVE_OPENSSL_VERSION_NUM
-DSERF_HAVE_OSSL_HANDSHAKE_STATE -DSERF_HAVE_SSL_LOCKING_CALLBACKS
-I/home/dsg/serf_PR-8 -isystem /usr/include/apr-1.0 -Wall
-Wdeclaration-after-statement -Wmissing-prototypes -std=c99 -O3
-DNDEBUG -DLINUX -D_REENTRANT -D_GNU_SOURCE -o
CMakeFiles/test_all.dir/MockHTTPinC/MockHTTP.c.o -c
/home/dsg/serf_PR-8/test/MockHTTPinC/MockHTTP.c",
[...]
+ "command": "/usr/bin/cc -DHAVE_STDBOOL_H=1 -DMOCKHTTP_OPENSSL
-DOPENSSL_NO_STDIO -DSERF_HAVE_BROTLI
-DSERF_HAVE_BROTLI_DECODER_RESULT -DSERF_HAVE_GSSAPI
-DSERF_HAVE_OPENSSL_ALPN -DSERF_HAVE_OPENSSL_MALLOC_INIT
-DSERF_HAVE_OPENSSL_SSL_LIBRARY_INIT -DSERF_HAVE_OPENSSL_VERSION_NUM
-DSERF_HAVE_OSSL_HANDSHAKE_STATE -DSERF_HAVE_SSL_LOCKING_CALLBACKS
-isystem /usr/include/apr-1.0 -std=c99 -w -O3 -DNDEBUG -o
CMakeFiles/mockhttpinc.dir/MockHTTP.c.o -c
/home/dsg/serf_PR-8/test/MockHTTPinC/MockHTTP.c",
[...]
]]]
In the RockyLinux instance, it seems like adding -D_REENTRANT to the
command line helps.
$ rm -rf out; svn up -r1926869 && CFLAGS=-D_REENTRANT cmake -B out &&
cmake --build out
[... succeeds ...]
I'm usually testing on Ubuntu 25.04 (under WSL) and after this
revision, I get the following error:
[[[
[ 88%] Built target serf_bwtp
[ 88%] Building C object
test/MockHTTPinC/CMakeFiles/mockhttpinc.dir/MockHTTP.c.o
/home/dsg/serf_PR-8/test/MockHTTPinC/MockHTTP.c: In function
‘methodToCode’:
/home/dsg/serf_PR-8/test/MockHTTPinC/MockHTTP.c:562:23: error:
implicit declaration of function ‘strcasecmp’; did you mean ‘strncmp’?
[-Wimplicit-function-declaration]
562 | if (ch1 == 'G' && strcasecmp(code, "GET") == 0)
| ^~~~~~~~~~
| strncmp
gmake[2]: ***
[test/MockHTTPinC/CMakeFiles/mockhttpinc.dir/build.make:79:
test/MockHTTPinC/CMakeFiles/mockhttpinc.dir/MockHTTP.c.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:475:
test/MockHTTPinC/CMakeFiles/mockhttpinc.dir/all] Error 2
gmake: *** [Makefile:146: all] Error 2
]]]
With the same checks as above, it seems like -D_GNU_SOURCE makes the
trick for me:
$ rm -rf out; svn up -r1926869 && cmake -B out && cmake --build out
[...fails...]
$ rm -rf out; svn up -r1926869 && CFLAGS=-D_GNU_SOURCE cmake -B out &&
cmake --build out
[...succeeds...]
The same issue still exists in trunk@1926976.
I don't know CMake well enough to understand why _REENTRANT and
_GNU_SOURCE are added in the main CMakeLists.txt but not in the one in
test/MockHTTPinC - I don't seen anything obvious in Brane's change.
The failure in Ubuntu 25.04 is also possible to rectify by including
<strings.h> in MockHTTP.c.
Right, I see the problem now. Having changed MockHTTP to a static
library, it didn't seem necessary to make it depend on the apr and
apr-util imported targets, only to use their include paths. And so we
missed APR's --cpppflags and --cflags.
Does r1926983 help?
-- Brane