Skip to content

Conversation

@HTRamsey
Copy link
Collaborator

Fix #12734

@HTRamsey
Copy link
Collaborator Author

HTRamsey commented May 18, 2025

@DonLakeFlyer thoughts on this? I originally didn't accommodate for the fact that CPM_SOURCE_CACHE was supposed to be an Env variable instead of a CMake variable. Also FYI passing a parameter in the terminal cmake config command will override any cache variables not marked with FORCE, so that would have still worked.

@HTRamsey HTRamsey requested a review from DonLakeFlyer May 18, 2025 21:04
@DonLakeFlyer
Copy link
Collaborator

Why not just let the normal CPM stuff handle this and remove all the special stuff in CMakeLists and CustomOptions? If folks want to override they can just set the regular CPM_SOURCE_CACHE in their custom build. Is the default provided by CPM for this not good so we need to specify our own?

@HTRamsey
Copy link
Collaborator Author

Well for one I think it should be enabled by default, and also env variables are less obvious than CMake options and I want to make sure people see this option

@DonLakeFlyer
Copy link
Collaborator

Makes sense then

@HTRamsey HTRamsey force-pushed the dev-cmake-cpm branch 2 times, most recently from 1e722d8 to 13ec2ee Compare May 21, 2025 04:24
@HTRamsey HTRamsey merged commit 9a60e78 into mavlink:master May 24, 2025
22 checks passed
@HTRamsey HTRamsey deleted the dev-cmake-cpm branch May 24, 2025 01:47
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.

Figure out CPM_SOURCE_CACHE usage

2 participants