-
Notifications
You must be signed in to change notification settings - Fork 205
Allow users to override the default path for CPM_SOURCE_CACHE #669
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
base: master
Are you sure you want to change the base?
Conversation
@TheLartians does this PR sound okay? Just want to optionally steer this behavior without an environment variable. PS loving CPM over at @tenstorrent |
@blozano-tt you can configure with As a side note here's a piece of code that I use often: # before including CPM.cmake or get_cpm.cmake
if(NOT CPM_SOURCE_CACHE AND NOT DEFINED ENV{CPM_SOURCE_CACHE})
set(CPM_SOURCE_CACHE "${CMAKE_SOURCE_DIR}/.cpm" CACHE PATH "CPM source cache")
message(STATUS "Setting cpm cache dir to: ${CPM_SOURCE_CACHE}")
endif() This allows me to cache Given this, I would be against merging this PR |
Hi @iboB, Thanks for sharing your thoughts. Yes that is possible, and we are doing that today in my project. We also have a similar snippet to what you shared. https://github.com/tenstorrent/tt-metal/blob/c50030e1323c5a4b71627e409146e46b799673a6/cmake/CPM.cmake#L8-L13 Someone complained that our project would not respect the ENV, so if someone reads the CPM docs, they change the ENV in a pipeline it would have no effect... this increaased the complexity of our snippet, and lead me to question ... why can't the default CPM.cmake from upstream absorb this functionality. With my proposal, CPM users can control the location with a single switch, and no one would need a custom snippet like you shared. Right? |
Ah. I think I get it now. The goal of this PR is not to make CPM_SOURCE_CACHE_DEFAULT something that one would configure with, but to reduce snippets like yours and mine from several lines to just: set(CPM_SOURCE_CACHE_DEFAULT "${CMAKE_SOURCE_DIR}/.cpm") I'm not necessarily against that. However this does not affect just |
Yes, exactly |
Currently, only the ENV CPM_SOURCE_CACHE can be used for steering CPM download caching.
However, I am not a fan of environment variable engineering, (We have too much of it already in our code base).
With this change, a user of CPM can first set CPM_SOURCE_CACHE_DEFAULT, and have that be respected by CPM.cmake.