#382 SIZEOF_OFF_T assertion error with cmake, mingw32, and mpg123 as a subproject

[Maarten]

Hello!

We recently received a bug report [1] about the off_t static assertion in lfs_wrap.c failing.
The failure happened because CMake does not provide namespaces for variables when using projects as subprojects: there is cross contamination for the SIZEOF_OFF_T CMake variable between mpg123 and another (=wavpack) project.
WavPack also determines the size of off_t, but apparently sets _FILE_OFFSET_BITS for large file support. This results in the cache variable SIZEOFF_T to be set to 8.
Because the cache variable is already set, CMake will skip the test in mpg123, and think off_t is 64-bit by default.

The patch below fixes this error by differentiating between CMake cache variables and local variables:
MPG123_SIZEOF_OFF_T will end up in the cache, and SIZEOF_OFF_T will be local and "forgotten" when leaving the mpg123 scope.

--- a/ports/cmake/src/CMakeLists.txt
+++ b/ports/cmake/src/CMakeLists.txt
@@ -74,9 +74,12 @@ check_function_exists(execvp HAVE_EXECVP)
 check_function_exists(ctermid HAVE_CTERMID)
 check_function_exists(clock_gettime HAVE_CLOCK_GETTIME)

-check_type_size(off_t SIZEOF_OFF_T)
+# MPG123_SIZEOF_OFF_T is name of the cache variable,
+# SIZEOF_OFF_T will be used in configure_file("config.cmake.h.in" "config.h")
+check_type_size(off_t MPG123_SIZEOF_OFF_T)
+set(SIZEOF_OFF_T "${MPG123_SIZEOF_OFF_T}")

-if(SIZEOF_OFF_T LESS 8)
+if(MPG123_SIZEOF_OFF_T LESS 8)
 check_function_exists(lseek64 LFS_LARGEFILE_64)

 if(LFS_LARGEFILE_64)

[1] https://github.com/libsdl-org/SDL_mixer/issues/788

[Maarten]

As an alternative, I've attached an alternative patch that changes all SIZEOF_OFF_T to MPG123_SIZEOF_OFF_T.

This patch was created by @sezero, another member of the SDL project.

[Ozkan Sezer]

Discard my big patch above.

The following revised version of the Marteen's patch should do the trick:

diff --git a/ports/cmake/src/CMakeLists.txt b/ports/cmake/src/CMakeLists.txt
index 7861e35..f6a77b9 100644
--- a/ports/cmake/src/CMakeLists.txt
+++ b/ports/cmake/src/CMakeLists.txt
@@ -74,9 +74,9 @@ check_function_exists(execvp HAVE_EXECVP)
 check_function_exists(ctermid HAVE_CTERMID)
 check_function_exists(clock_gettime HAVE_CLOCK_GETTIME)

-check_type_size(off_t SIZEOF_OFF_T)
+check_type_size(off_t MPG123_SIZEOF_OFF_T)

-if(SIZEOF_OFF_T LESS 8)
+if(MPG123_SIZEOF_OFF_T LESS 8)
 check_function_exists(lseek64 LFS_LARGEFILE_64)

 if(LFS_LARGEFILE_64)
diff --git a/ports/cmake/src/config.cmake.h.in b/ports/cmake/src/config.cmake.h.in
index e2a4432..456b96f 100644
--- a/ports/cmake/src/config.cmake.h.in
+++ b/ports/cmake/src/config.cmake.h.in
@@ -146,7 +146,7 @@
 #define PKGLIBDIR "@CMAKE_INSTALL_LIBDIR@/@PROJECT_NAME@"

 // CMake leaves it emtpy for non-existing type. Autoconf sets it to 0.
-#define SIZEOF_OFF_T (@SIZEOF_OFF_T@+0)
+#define SIZEOF_OFF_T (@MPG123_SIZEOF_OFF_T@+0)

 #cmakedefine STDERR_FILENO @STDERR_FILENO@
 #cmakedefine STDIN_FILENO @STDIN_FILENO@

[Thomas Orgis]

Isn't this a major bug in cmake and subprojects should just be avoided unless there is some isolation between them, or at least controlled sharing of variables?

Anyhow, I imported the most recent patch. Is current revision 5532 fine for you?

[Ozkan Sezer]

Anyhow, I imported the most recent patch. Is current revision 5532 fine for you?

Looks good to me