[CI][sqlite3][nss] fix missing -lm#50514
Conversation
|
I got the same build error in this run. I'm going to bump the |
|
sqlite3 is who is using the math symbols so I think that's who needs -lm in their .pc |
| # Convert CMake list (semicolons) to space-separated string so the | ||
| # entire value stays as one token inside "-Dxxx_libs=..." arguments. | ||
| list(JOIN ${out_var} " " ${out_var}) |
There was a problem hiding this comment.
No... This loop is about parsing space-separated pkg-config output into a cmake list. If you can't pass it into cmake arguments, your cmake arguments lack proper quotes. (We do it everywhere in vcpkg ports. e.g. passing FEATURES in test ports.)
| if(HAVE_LIBM) | ||
| target_link_libraries(sqlite3-bin PRIVATE m) | ||
| endif() | ||
| target_link_libraries(sqlite3-bin PRIVATE m) |
There was a problem hiding this comment.
Start relying on toolchains providing a libm placeholder even if they integrated the math into the C library?
| x_vcpkg_pkgconfig_get_modules(PREFIX PC_NSPR MODULES nspr CFLAGS LIBS) | ||
| x_vcpkg_pkgconfig_get_modules(PREFIX PC_SQLITE MODULES sqlite3 CFLAGS LIBS) | ||
| x_vcpkg_pkgconfig_get_modules(PREFIX PC_ZLIB MODULES zlib CFLAGS LIBS) | ||
| # Produce absolute include dirs and library dirs filepaths. | ||
| # Manually managing MSVC syntax because gyp converts foo.lib as if it were a relative path. | ||
| foreach(key IN ITEMS NSPR_CFLAGS_RELEASE SQLITE_CFLAGS_RELEASE ZLIB_CFLAGS_RELEASE) | ||
| separate_arguments(cflags UNIX_COMMAND "${PC_${key}}") | ||
| string(REPLACE "CFLAGS_RELEASE" "INCLUDE_DIRS" out_var "${key}") | ||
| set(${out_var} "") | ||
| foreach(item IN LISTS cflags) | ||
| if(item MATCHES "^-I(.*)") | ||
| cmake_path(SET dir NORMALIZE "${CMAKE_MATCH_1}") | ||
| if(CMAKE_HOST_WIN32) | ||
| cygpath_u(dir "${dir}") | ||
| else() | ||
| endif() | ||
| list(APPEND ${out_var} "${dir}") | ||
| endif() | ||
| endforeach() | ||
| list(JOIN ${key}_INCLUDE_DIRS ":" ${key}_INCLUDE_DIRS) | ||
| endforeach() | ||
| foreach(out_var IN ITEMS NSPR_LIBS_RELEASE NSPR_LIBS_DEBUG SQLITE_LIBS_RELEASE SQLITE_LIBS_DEBUG ZLIB_LIBS_RELEASE ZLIB_LIBS_DEBUG) | ||
| separate_arguments(libs UNIX_COMMAND "${PC_${out_var}}") | ||
| set(${out_var} "") | ||
| foreach(item IN LISTS libs) | ||
| if(item MATCHES "^-L(.*)") | ||
| cmake_path(SET dir NORMALIZE "${CMAKE_MATCH_1}") | ||
| if(CMAKE_HOST_WIN32) | ||
| cygpath_u(dir "${dir}") | ||
| endif() | ||
| if(VCPKG_DETECTED_MSVC) | ||
| list(APPEND ${out_var} "-LIBPATH:${dir}") | ||
| string(APPEND ${out_var} " -LIBPATH:${dir}") | ||
| else() | ||
| list(APPEND ${out_var} "-L${dir}") | ||
| string(APPEND ${out_var} " -L${dir}") | ||
| endif() | ||
| elseif(item MATCHES "^-l(.*)") | ||
| list(APPEND ${out_var} "${item}") | ||
| string(APPEND ${out_var} " ${item}") | ||
| endif() | ||
| endforeach() | ||
| string(STRIP "${${out_var}}" ${out_var}) | ||
| endforeach() |
There was a problem hiding this comment.
This seems to implement for LIBS what ports could delegate to x_vcpkg_pkgconfig_get_modules by USE_MSVC_SYNTAX_ON_WINDOWS.
(But I see that CFLAGS is really processed differently.)
There was a problem hiding this comment.
gyp mishandles MSVC-style .lib paths (e.g. ..\sqlite\sqlite3.lib) as
relative paths from the build directory. The manual loop correctly
produces Unix-style -L/-l flags on all platforms, which gyp expects.
There was a problem hiding this comment.
Oh, this is one of the ports which use a vendored configure script as glue to nss/build.sh.
dg0yt
left a comment
There was a problem hiding this comment.
Title still says "spatialite-tools" but it is no longer touched by this PR.
sqlite3 could be a separate PR. I still disagree with (proliferating) unchecked use of libm.
sdl3-mixer is an independent change, too.
| endforeach() | ||
|
|
||
| if(SQLITE_ENABLE_FTS5 OR SQLITE_ENABLE_MATH_FUNCTIONS) | ||
| find_library(HAVE_LIBM m) |
There was a problem hiding this comment.
IIRC the problem with find_library(HAVE_LIBM m) is that CMake searchs for a file in a set of paths which do not include CMAKE_<LANG>_IMPLICIT_LINK_DIRECTORIES.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
What do you think of JavierMatosD#4 ? |
|
Please finish this PR. The sqlite3 issue is getting a blocker, in particular with sqlite3. Either always link m on `UNIX. (Many packages do that. Toolchains do the right thing.) IOW I agree with the proposed changes to sqlite3 as is. |
|
@dg0yt It's unlikely that this will be finished; Javier is no longer at Microsoft. EDIT: I suppose I could resurrect it myself... |
When
nsslinkslibsoftokn3.so againstlibsqlite3.a, the linker sees unresolved references to all the math symbols thatsqlite3pulled in, because-lmwas never passed.On x64-linux native builds this went undetected because the host runs Ubuntu 24.04 with
glibc 2.39, wherelibmhas been merged intolibc.so.6and math symbols are satisfied implicitly. The aarch64 cross-compilation sysroot (gcc-12-aarch64-linux-gnu) usesglibc 2.36, wherelibmis still a separate library, so the missing-lmis a hard linker error.Reference: https://developers.redhat.com/articles/2021/12/17/why-glibc-234-removed-libpthread - covers the glibc consolidation effort;
libmwas completed after2.34and fully merged by the time Ubuntu 24.04 shippedglibc 2.39.