Control: retitle -1 freespace2: FTBFS with libsdl1.2-compat-dev: 
'malloc_usable_size' was not declared in this scope

(cc'ing adetiste because he was looking at this package recently; the
SDL2-based new upstream version will close both #1038352 and this bug,
assuming the SDL2 version compiles successfully)

On Mon, 20 Feb 2023 at 09:35:59 +0100, Andreas Beckmann wrote:
> The actual problem are missing
>   Build-Depends: pkg-config, libglu1-mesa-dev
> These were pulled in transitively in bullseye.

The actual actual problem :-) is that freespace2 build-depends on
libsdl-dev, a virtual package, and not on libsdl1.2-dev, a concrete
implementation of that virtual package. This means that buildds and other
build environments will choose either libsdl1.2-dev (currently "classic"
SDL 1.2) or libsdl1.2-compat-dev, arbitrarily. That makes this bug RC,
while in other packages with "FTBFS with libsdl1.2-compat-dev" bugs,
the bug is usually not RC yet.

libsdl1.2-compat-dev didn't exist in bullseye, and "classic" SDL 1.2 always
pulled in libglu1-mesa-dev (see #1039072) and pkg-config (see #1039074),
which is why this worked in bullseye. It will not be reliable in bookworm,
which does have libsdl1.2-compat-dev available.

libsdl1.2-compat-dev (>= 1.2.64-3) works around this class of bug by
depending on pkgconf and libglu1-mesa-dev, so this part is not critical
to address in freespace2.

It is still a good idea to have an explicit build-dependency on anything
that the package explicitly uses, to avoid being broken by "action at
a distance" in a dependency.

It would also be a good idea to switch the build-dependency to
the concrete package, libsdl1.2-dev. At the moment, this is "classic"
SDL 1.2. My intention is that before the trixie freeze, sdl12-compat
will take over that binary package name, so Build-Depends: libsdl1.2-dev
will still be the correct thing for SDL 1.2 codebases to use.

The other part of this FTBFS *is* release-critical to address in
freespace2 if not worked around in libsdl1.2-compat-dev:

> Then you need to make the inclusion of malloc.h unconditional in
> code/windows_stub/stubs.cpp (or fix the build system s.t. it checks for
> malloc.h and populates HAVE_MALLOC_H accordingly).

This is also triggered by using libsdl1.2-compat-dev. The headers in
that package aim to be compatible with the ones in libsdl1.2-dev, but for
licensing reasons they are partially a rewrite, and for maintainability
reasons they differ in various implementation details.

In this case, the error seen is:

windows_stub/stubs.cpp: In function 'void* _vm_malloc(int, char*, int, int)':
windows_stub/stubs.cpp:49:32: error: 'malloc_usable_size' was not declared in 
this scope
   49 | #define MALLOC_USABLE(pointer) malloc_usable_size(pointer)
      |                                ^~~~~~~~~~~~~~~~~~
windows_stub/stubs.cpp:627:28: note: in expansion of macro 'MALLOC_USABLE'
  627 |         size_t used_size = MALLOC_USABLE(ptr);
      |                            ^~~~~~~~~~~~~
windows_stub/stubs.cpp: In function 'void* _vm_realloc(void*, int, char*, int, 
int)':
windows_stub/stubs.cpp:49:32: error: 'malloc_usable_size' was not declared in 
this scope
   49 | #define MALLOC_USABLE(pointer) malloc_usable_size(pointer)
      |                                ^~~~~~~~~~~~~~~~~~
windows_stub/stubs.cpp:649:27: note: in expansion of macro 'MALLOC_USABLE'
  649 |         size_t old_size = MALLOC_USABLE(ptr);
      |                           ^~~~~~~~~~~~~
windows_stub/stubs.cpp: In function 'void _vm_free(void*, char*, int)':
windows_stub/stubs.cpp:49:32: error: 'malloc_usable_size' was not declared in 
this scope
   49 | #define MALLOC_USABLE(pointer) malloc_usable_size(pointer)
      |                                ^~~~~~~~~~~~~~~~~~
windows_stub/stubs.cpp:731:21: note: in expansion of macro 'MALLOC_USABLE'
  731 |         TotalRam -= MALLOC_USABLE(ptr);
      |                     ^~~~~~~~~~~~~

This appears to be because freespace2 checks #ifdef HAVE_MALLOC_H,
but never actually checks for malloc.h, so it was accidentally relying
on SDL's SDL_config.h to leak information about the headers that SDL
uses internally. Here is a patch that is sufficient to resolve this part
of the bug:

----8<----
diff --git a/configure.ac b/configure.ac
index d850f85..af54a1e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -311,6 +311,7 @@ AM_CONDITIONAL(FS2_DEBUG,      test "$fs2_debug"      = 
"yes")
 AM_CONDITIONAL([am__fastdepOBJC],  test "$fs2_os_osx"  = "yes")

 AC_CHECK_HEADER(stdlib.h)
+AC_CHECK_HEADERS_ONCE([malloc.h])

 dnl From licq: Copyright (c) 2000 Dirk Mueller
 dnl Check if the type socklen_t is defined anywhere
----8<----

If a SDL2 version cannot be uploaded any time soon, please apply that
patch.

I might work around this in sdl12-compat too (I'll see what upstream
think) but this is really not SDL's job, so please add that check anyway.

Thanks,
    smcv

Reply via email to