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

Specify minimum cmake version for generic_board.cmake #1879

Merged
merged 1 commit into from
Sep 28, 2024

Conversation

lurch
Copy link
Contributor

@lurch lurch commented Aug 28, 2024

cmake/generic_board.cmake uses POP_FRONT, which https://cmake.org/cmake/help/latest/command/list.html#pop-front says was added in CMake version 3.15.
(this change means that people still using an ancient version of cmake will get a nicer error-message)

@kilograham
Copy link
Contributor

This should probably go in the top-level CMakeLists.txt - i am loathe to change minimum versions, but then again, this code is always used an no one has complained, so most people must be >= 3.15

@kilograham kilograham added this to the 2.0.1 milestone Aug 28, 2024
@lurch
Copy link
Contributor Author

lurch commented Aug 28, 2024

I wasn't sure what the "policy" was WRT CMake minimum versions (i.e. do we just have a single "global" minimum version, or does each CMakeLists.txt declare the minimum version that it needs itself?), so I based this change on https://github.com/raspberrypi/pico-sdk/blob/develop/tools/Findpicotool.cmake (which already declares a higher minimum-version than the top-level CMakeLists.txt).

this code is always used an no one has complained, so most people must be >= 3.15

I discovered this by trying to see (just out of curiosity) whether the SDK would still build on a Debian Buster docker image, which comes with CMake 3.13.4 (Debian Buster is EOL).

@kilograham
Copy link
Contributor

@will-v-pi thoughts?

@will-v-pi
Copy link
Contributor

FindPicotool.cmake is only used when the SDK needs to compile picotool (contrary to its name), so if you have installed picotool then that file won’t be used and a lower CMake version should work. I think it makes sense to do the same for generic_board.cmake as you could set your own simplified board cmake file if you need to use an older version of CMake.

I’d agree that for any CMakeLists.txt files we should update it in the top level project, but for these script-esque files which are not always included it’s probably better to put the minimum version in the file?

@kilograham kilograham merged commit 78533ec into develop Sep 28, 2024
2 checks passed
@kilograham kilograham deleted the lurch-patch-1 branch September 28, 2024 04: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.

3 participants