Custom cache directory name #543
Conversation
Enable setting a custom directory name for cached packages. Required if e.g. patch_command uses external variables or just to set a human readable name.
There was a problem hiding this comment.
Hey, thanks for the issue and PR!
The code looks good and I think I understand some of the use-case, but I'm not sure this approach is without issues, as it could create hard to track bugs if different projects use the same custom cache dir name. In general I feel we want different patch commands to resolve to different cache directories as the result itself may be different.
TBH I'm not sure what is a good solution for this, maybe you can elaborate more on what exactly the issue is with the package you are attempting to add?
cmake/CPM.cmake
Outdated
| # Application set a custom unique directory name | ||
| set(download_directory ${CPM_SOURCE_CACHE}/${lower_case_name}/${CPM_ARGS_CUSTOM_CACHE_DIR}) |
There was a problem hiding this comment.
Currently the cache directory is a hash of all used arguments to guarantee that the dependency is actually present in precisely the requested version. How would we avoid conflicts here if users could set custom directories? E.g. say I have two projects that both use foo as a custom cache dir, but they use different versions?
There was a problem hiding this comment.
Version must be handled manually (like with a combination of the download version and a manual sha of the patch_command files).
The current sha will not work when e.g. patch_command refers to external files, so it is not fool proof either.
Is the description in the readme clear enough?
My use case with some more details:
- Shared toolchain that are several GB and may be used in many application repos. The patch command contains external files adjusting the installation (licenses that may be updated more often than the toolchain).
- As paths are in the patch_command, the sha will differ and the paths will not be shared
- As there are external files in the patch_command, the sha is not reliable, requires change to the arguments too.
- The sha makes it hard to see what is in the cache - I may want to clean certain test versions only
- Using Docker is inconvenient, the toolchain has a few branches
I would have used SOURCE_DIR for this change if not compatibility had been broken...
There was a problem hiding this comment.
That's true, patch commands using external files will definitely create issues. I'm still unsure about how I feel about applying patches in the cache as it can create very hard to track issues. In the JavaScript world using npm or yarn, dependencies are copied from the cache to a project-specific location and any patches are applied there. That way the original cache remains valid no matter how the patch is applied. Maybe a mechanism like that would make sense as well here?
I'm also not completely against this approach, just the name could be confusing as it seems to imply an absolute path. How about calling it something like CUSTOM_CACHE_KEY instead?
There was a problem hiding this comment.
That's true, patch commands using external files will definitely create issues. I'm still unsure about how I feel about applying patches in the cache as it can create very hard to track issues. In the JavaScript world using
npmoryarn, dependencies are copied from the cache to a project-specific location and any patches are applied there. That way the original cache remains valid no matter how the patch is applied. Maybe a mechanism like that would make sense as well here?
Maybe, even if my several GB packages should not be copied.
That would men to intercept and handle the fetch content arguments.
A bigger feature, I believe this can coexist without maintenance issues.
I'm also not completely against this approach, just the name could be confusing as it seems to imply an absolute path. How about calling it something like
CUSTOM_CACHE_KEYinstead?
Fine, I pushed a change.
Fixes #542
Enable setting a custom directory name for cached packages. Required if e.g. patch_command uses external variables or just to set a human readable name.
Unit test added.