CMake Path parsing error on windows in gdal 3.9.0 while trying to specify absulut libsqlite3.a path

[weiznich]

What is the bug?

There seems to be an path parsing issue with the gdal cmake script on windows. It does not seem to accept paths like C:\a\gdal\gdal\target\debug\build\libsqlite3-sys-73bcbd616e04c6d1\out\libsqlite3.a as config options

Steps to reproduce the issue

git clone https://github.com/GiGainfosystems/gdal/
cd gdal
git checkout 180ea2d
cargo check -p gdal-src -F "all_drivers"

This will build gdal version 3.9.0 as part of the rust build.

This outputs the following error message:

CMake Error at D:/a/gdal/gdal/target/debug/build/gdal-src-bb72df274361ef83/out/build/CMakeFiles/CMakeScratch/TryCompile-0tymbt/CMakeLists.txt:29 (target_link_libraries):
    Syntax error in cmake code at

      D:/a/gdal/gdal/target/debug/build/gdal-src-bb72df274361ef83/out/build/CMakeFiles/CMakeScratch/TryCompile-0tymbt/CMakeLists.txt:29

    when parsing string

      D:\a\gdal\gdal\target\debug\build\libsqlite3-sys-73bcbd616e04c6d1\out\libsqlite3.a

    Invalid character escape '\a'.


  CMake Error at C:/Program Files/CMake/share/cmake-3.29/Modules/CheckSymbolExists.cmake:140 (try_compile):
    Failed to configure test project build system.
  Call Stack (most recent call first):
    C:/Program Files/CMake/share/cmake-3.29/Modules/CheckSymbolExists.cmake:66 (__CHECK_SYMBOL_EXISTS_IMPL)
    cmake/modules/packages/FindSQLite3.cmake:85 (check_symbol_exists)
    cmake/helpers/CheckDependentLibraries.cmake:149 (find_package)
    cmake/helpers/CheckDependentLibraries.cmake:557 (gdal_check_package)
    gdal.cmake:266 (include)
    CMakeLists.txt:240 (include)

See here for the full build log, including config flags, etc.

Versions and provenance

GDAL version: 3.9.0
Operating system: Windows server 2022 (Github Actions windows-latest image)

Additional context

This was discovered while trying to provide bundling support (auto building) for the rust gdal crate. It previously worked with the 3.8.4 (c2d2a61)

[rouault]

This could potentially be related to the change of 66239c2 where find_path() and find_library() are no longer run if the user provides SQLite3_INCLUDE_DIR and SQLite3_LIBRARY. Perhaps those did some path rewriting from backslash to forward slash? (EDIT: answering myself after experimenting: they do)
But my recommendation would be that you switch from backslash paths to forward slash paths, as https://www.google.com/search?q=cmake+Invalid+character+escape shows that's a constant source of issues with CMake on Windows

[rouault]

@weiznich Could you try the following patch ? Although it feels a bit ugly to have to do that, and I feel like it is a CMake bug (but I cannot reproduce an error with CheckSymbolExists() on Windows with \\ style of path) or users should just avoid backslashes

--- a/cmake/modules/packages/FindSQLite3.cmake
+++ b/cmake/modules/packages/FindSQLite3.cmake
@@ -47,6 +47,10 @@ endif()
 
 if(SQLite3_INCLUDE_DIR AND SQLite3_LIBRARY)
   set(SQLite3_FIND_QUIETLY TRUE)
+  if (CMAKE_VERSION VERSION_GREATER_EQUAL 3.20)
+    cmake_path(CONVERT "${SQLite3_INCLUDE_DIR}" TO_CMAKE_PATH_LIST SQLite3_INCLUDE_DIR)
+    cmake_path(CONVERT "${SQLite3_LIBRARY}" TO_CMAKE_PATH_LIST SQLite3_LIBRARY)
+  endif()
 else()
   find_package(PkgConfig QUIET)
   if(PKG_CONFIG_FOUND)

Perhaps @dg0yt has some clues

[dg0yt]

Garbage in:

"-DSQLite3_INCLUDE_DIR=C:\\Users\\runneradmin\\.cargo\\registry\\src\\index.crates.io-6f17d22bba15001f\\libsqlite3-sys-0.28.0/sqlite3"

garbage out.
Pass CMake-style path (using /), or at least do not mix windows and posix style paths.

+  if (CMAKE_VERSION VERSION_GREATER_EQUAL 3.20)
+    cmake_path(CONVERT "${SQLite3_INCLUDE_DIR}" TO_CMAKE_PATH_LIST SQLite3_INCLUDE_DIR)
+    cmake_path(CONVERT "${SQLite3_LIBRARY}" TO_CMAKE_PATH_LIST SQLite3_LIBRARY)
+  endif()

No!

[dg0yt]

Admittedly,

"-DSQLite3_LIBRARY=D:\\a\\gdal\\gdal\\target\\debug\\build\\libsqlite3-sys-73bcbd616e04c6d1\\out\\libsqlite3.a"

doesn't mix separators.

IIRC filepath type cache variables as managed by find_path/find_library do indeed behave a little special, converting relative paths to absolute etc.
There is also no reason to explicitly skip find_path/find_library because they should already be a no-op if the cache variable is set.

[dg0yt]

Maybe passing the type along the variable name might be enough to trigger a good conversion?

-"-DSQLite3_LIBRARY=D:\\a\\gdal\\gdal\\target\\debug\\build\\libsqlite3-sys-73bcbd616e04c6d1\\out\\libsqlite3.a"
+"-DSQLite3_LIBRARY:FILEPATH=D:\\a\\gdal\\gdal\\target\\debug\\build\\libsqlite3-sys-73bcbd616e04c6d1\\out\\libsqlite3.a"

[rouault]

There is also no reason to explicitly skip find_path/find_library because they should already be a no-op if the cache variable is set.

indeed. @zjyhjqs are you sure 66239c2 was needed ? CMake documentation states that if variables that store the result of find_path / find_library are already set, "the search will not be repeated unless the variable is cleared", so I would assume that PC_SQLITE3_LIBRARY_DIRS should be ignored i you specify already SQLite3_LIBRARY

[zjyhjqs]

There is also no reason to explicitly skip find_path/find_library because they should already be a no-op if the cache variable is set.

indeed. @zjyhjqs are you sure 66239c2 was needed ? CMake documentation states that if variables that store the result of find_path / find_library are already set, "the search will not be repeated unless the variable is cleared", so I would assume that PC_SQLITE3_LIBRARY_DIRS should be ignored i you specify already SQLite3_LIBRARY

It should be safely removed given that the search will not be repeated in this case.

Pkgconfig (pkg_check_modules) would find the SQLite in system environment, which may not provide the mutex support GDAL needs. On Android platform, the SQLite bundled by NDK would be linked in higher priority than the custom SQLite provided by SQLite3_LIBRARY, due to it’s found through linked path option.

The sqlite3 in NDK would be linked due to the -L option set by pkg_check_modules, which leads to configuration failure: ${SQLite3_LIBRARIES} lacks mutex support!

–––

IMO, the error from this issue post is caused by the wrong input. The path separator \ without an escaped character is invalid in many places.

[weiznich]

Thanks for your fast response.
I changed the code to generate paths that use / as path separator and that seems to work. I'm fine with that as long as the documentation includes information about this.

[rouault]

I'm fine with that as long as the documentation includes information about this.

cf 0913d9f

[dg0yt]

Did you test the :FILEPATH suffix to your parameter?

[weiznich]

I didn't try that as there was no easy way to pass that argument from the rust build script.