matio
matio copied to clipboard
MinGW Build Regression
The fix for https://github.com/tbeu/matio/issues/187 caused a build regression in MinGW for me (amd64, CMake Buildsystem):
- MinGW (for compatibility w/ build systems that expect glibc) allows the user to specify
-lmfor the math functions, and automatically maps them to the correct CRT. But it does not provide a-lc, because normally nobody explicitly specifies that. - The fix for the OpenBSD issue caused the build system of matio to automatically add
-lcto the compiler line due to the commit https://github.com/tbeu/matio/commit/f058e5dfbaa8bc49fbe76631b6dbf7637be04b10.
This causes MinGW builds to fail, since HAVE_LIBM is true on those systems, but -lc doesn't work there.
The least wrong simple fix I could come up with (without testing on OpenBSD, which I don't use) was to simply also check for -lc and then only link to that if it's available. The following patch works for me on Linux, macOS and Windows:
--- a/cmake/compilerOptions.cmake 2023-11-12 13:25:19.000000000 +0100
+++ b/cmake/compilerOptions.cmake 2024-02-05 16:29:48.804984158 +0100
@@ -70,6 +70,13 @@ endforeach()
include(CheckLibraryExists)
check_library_exists(m pow "" HAVE_LIBM)
+# OpenBSD also requires -lc (See
+# <https://github.com/tbeu/matio/issues/187> for
+# details), but that doesn't exist on all platforms
+# that do have libm (most notably not MinGW), so
+# explicitly check for the existence of -lc before
+# linking against in in src.cmake.
+check_library_exists(c fopen "" HAVE_LIBC)
include(CheckCSourceCompiles)
set(TEST_CODE_DECIMAL_POINT "
--- a/cmake/src.cmake 2023-11-12 13:25:19.000000000 +0100
+++ b/cmake/src.cmake 2024-02-05 16:26:10.244019256 +0100
@@ -68,7 +68,10 @@ if(STDINT_MSVC)
endif()
if(HAVE_LIBM)
- target_link_libraries(${PROJECT_NAME} PUBLIC m c)
+ target_link_libraries(${PROJECT_NAME} PUBLIC m)
+ if(HAVE_LIBC)
+ target_link_libraries(${PROJECT_NAME} PUBLIC c)
+ endif()
endif()
if(MSVC)
While this will solve the issue, the most appropriate fix would probably be to only do that if OpenBSD requires this, for example performing a --no-undefined link check, and only add -lc if that is required to avoid the linker error with that flag. However, I can also open a pull request for my workaround if that's more the route you'd want to go in.
Thanks for the report. Need to fix it properly.
@seanm @MaartenBent FYI
Looks good to me, it works fine on Ubuntu. I also don't have OpenBSD to check.
One small remark, you could move the if(HAVE_LIBC) outside the if(HAVE_LIBM). For those very rare systems that don't have m but do need c.
I've setup a MinGW build CI that confirms the regression: https://github.com/tbeu/matio/actions/runs/7822032818/job/21340240188
I've now attempted to write a proper generic test that doesn't depend on the OS but doesn't also add -lc on operating systems where that isn't necessary. See PR #238.
Since I don't have an OpenBSD system, I can't test that case, but I did take care when writing the CMake test. If something comes up on OpenBSD, this should hopefully be easily fixable.