Clean Cache commands #285
Clean Cache commands #285
Conversation
weswc
left a comment
weswc
left a comment
There was a problem hiding this comment.
Also planning on including --binaries-only and --src-only` flags after this PR
|
|
||
| removed.push(format!("{name} ({version}) - {}", types.join(" and "))) | ||
| } | ||
| PurgeDepResult::NotInCache { name, version } => not_removed.push(format!( |
There was a problem hiding this comment.
Deps that are determined to be "NotInCache" are packages that would be in cache, but haven't been synced yet. I'm a bit unsure if I should list it as "removed" or "not removed". The pros/cons I see:
- Removed - For the sake of the user, the package not being in the cache before the purge is the same result as if it was removed, so we should display it as such
- Not Removed - Factual and communicates that if you're having issues with this package its not because of the cache
Ultimately, this scenario likely won't appear that often because if you're having issues with a package, its likely in the cache and you're going to target it specifically, but wanted to call it out.
| } | ||
| } | ||
|
|
||
| impl fmt::Display for PurgeResults<'_> { |
There was a problem hiding this comment.
Decided to leave paths out of the displayed results as it is not what a user would typically want, but recorded them and included as part of the json to leave them accessible if someone needs them.
| #[derive(Debug, Subcommand)] | ||
| pub enum CacheSubcommand { | ||
| /// Shows the cache directories for each repository used by rv project | ||
| Dir, |
There was a problem hiding this comment.
Unsure if we want an aliased subcommand for dir/info, that is the same as rv cache, or remove this and leave if no subcommand, just display as before. I see both ways, so will leave it to your judgement
| } | ||
|
|
||
| #[derive(Debug, Subcommand)] | ||
| pub enum CacheSubcommand { |
There was a problem hiding this comment.
I'm not attached to any of the subcommand names, so if you have different ideas I'm open. The basic things I wanted to include is:
- A way to remove "things" from the cache, whether it be the entire cache, entire repositories, or packages
- A way to update targeted repository databases instead the bulky way of using the env var to update all of them
|
i think there is another permutation there are the repository-db's and then there are the packages in a given repository. I think those are two common scenarios to want to purge. Eg "I realized the packages from that repo i want to install differently" type deal. I'd suggest we have empty string = all? same thing goes then having |
|
Are you thinking including that in addition to I'm okay either way, but I did consider the repo db aspect and chose, for a first pass, to keep it separate from purge |
|
refresh i'm not sure the specific use case over purge + eg purge + summary etc. Eg what is the use case to just refresh then and there but never run another command? from a repo-db perspective this just needs to be a concept that is explained when people are trying to determine what to purge. From my end, we have global purges - do everything associated with everything in the rproject - then more targetted purges, where targetting could be based on a repository in question, or a particular thing in the rproject - eg all repository databases. |
Keats
left a comment
Keats
left a comment
There was a problem hiding this comment.
looks like i forgot to publish my comments
| name: &str, | ||
| version: &Version, | ||
| source: &Source, | ||
| ) -> Result<(Option<PathBuf>, Option<PathBuf>), std::io::Error> { |
There was a problem hiding this comment.
maybe return a struct so we don't guess which path is the source and which one is the binary?
| /// Repositories are aliases corresponding to repos in the config | ||
| pub fn purge_cache<'a>( | ||
| context: &'a CliContext, | ||
| resolution: &'a Resolution<'a>, |
There was a problem hiding this comment.
Why do we need a resolution to purge things?
|
|
||
| let mut dep_res = Vec::new(); | ||
| for d in dependencies { | ||
| let res = if let Some(dep) = resolution.found.iter().find(|r| &r.name == d) { |
There was a problem hiding this comment.
if people pass a dep name, don't they want to purge all the versions of that dep?
This PR contains 3 commands for cache.
rv cachehas the same behavior for consistency across versionsrv cache dir- lists the cache directories same asrv cachedoes.rv cache purge- purge/remove entries from the cache. Has the following behaviors:a. No flags purges the entire cache
b.
--repositories- listed alias of repositories in config are removedc.
--packages- package versions as resolved in the project are removedrv cache refresh- "refreshes" the repository databases for the listed repository aliases.Additionally, all cache commands no longer require the resolution to be successful in order to perform action. Previously, resolution had to be completely successful to print cache directories, but this is not ideal as determining cache details may be needed/wanted in an unresolved status. Now, the resolution takes as much as is found, and then warns the user the action/info may not be complete due to the project being unresolvable.