Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase CMake minimum version from 3.9 to 3.16 #3997

Merged
merged 10 commits into from
Jan 14, 2024

Conversation

mwtoews
Copy link
Member

@mwtoews mwtoews commented Jan 10, 2024

This PR upgrades the CMake minimum version from 3.9 to 3.16. It also removes old workaround and hacks that were needed to support older CMake versions.

Also, fetching the "internal" googletest is modified according to this doc that needs CMake 3.14. (to be resubmitted for a separate PR)

See discussion on mail list.

Related to #3987 (ping @ErixenCruz).
Refer to #1907 for previous CMake bump four years ago.

@mwtoews mwtoews marked this pull request as draft January 10, 2024 09:17
@rouault
Copy link
Member

rouault commented Jan 10, 2024

The error with CIFuzz is unrelated and will be fixed if you rebase on top of latest master

@dg0yt
Copy link
Contributor

dg0yt commented Jan 11, 2024

Do you also want to migrate from cmake/FindSqlite3.cmake to the official SQLite3?
https://cmake.org/cmake/help/latest/module/FindSQLite3.html

Note that it uses a different set of variable names, and cache variables are used for user input.

@mwtoews
Copy link
Member Author

mwtoews commented Jan 11, 2024

Thanks for the suggestion @dg0yt . It seems that we'd need to translate (and possibly deprecate) these documented options:

  • SQLITE3_INCLUDE_DIRSQLite3_INCLUDE_DIRS
  • SQLITE3_LIBRARYSQLite3_LIBRARIES

@ErixenCruz
Copy link
Contributor

Vote for CMake 3.16 passed https://lists.osgeo.org/pipermail/proj/2024-January/011235.html. This PR can update FindSQLite3, or we can do it separately in #3987. It may make sense to do it all here in one "3.16 upgrade" PR, but it could go either way

# fileno() and other functions, when building gtest or including it
add_definitions(-D_GNU_SOURCE)
endif()
# Add Google Test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know why we fetch directly in CMake instead of relying on something like a git submodule? I think git submodule could be simpler/cleaner

@mwtoews
Copy link
Member Author

mwtoews commented Jan 12, 2024

Thanks for the update @ErixenCruz

I'm going to simplify this PR, and split off FetchContent/googletest and FindSqlite3 upgrades into separate PRs, since these are slightly more complicated and probably deserve a bit more focus/conversation. This PR should cover the ubuntu upgrades and the low-hanging CMake upgrades already done (are there any others?)

@mwtoews mwtoews marked this pull request as ready for review January 12, 2024 18:42
@rouault rouault added this to the 9.4.0 milestone Jan 14, 2024
@rouault rouault merged commit 95db9aa into OSGeo:master Jan 14, 2024
22 checks passed
@mwtoews mwtoews deleted the cmake-3.16 branch January 14, 2024 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants