diff --git a/crates/criticalup-cli/src/arg.rs b/crates/criticalup-cli/src/arg.rs index 250be828..ee2dd28d 100644 --- a/crates/criticalup-cli/src/arg.rs +++ b/crates/criticalup-cli/src/arg.rs @@ -24,7 +24,7 @@ pub(crate) struct Instrumentation { pub(crate) log_level: Vec, } -impl<'a> Instrumentation { +impl Instrumentation { pub(crate) fn log_level(&self) -> String { match self.verbose { 0 => "info", @@ -34,7 +34,7 @@ impl<'a> Instrumentation { .to_string() } - pub(crate) async fn setup<'b: 'a>(&'b self, binary_name: &str) -> Result<(), crate::Error> { + pub(crate) async fn setup(&self, binary_name: &str) -> Result<(), crate::Error> { let filter_layer = self.filter_layer(binary_name)?; if self.verbose != 0 { diff --git a/crates/criticalup-cli/src/commands/clean.rs b/crates/criticalup-cli/src/commands/clean.rs index 98369a5c..96990450 100644 --- a/crates/criticalup-cli/src/commands/clean.rs +++ b/crates/criticalup-cli/src/commands/clean.rs @@ -35,10 +35,23 @@ async fn delete_cache_directory(cache_dir: &Path) -> Result<(), Error> { /// Deletes installation from `State` with `InstallationId`s that have empty manifest section, and /// deletes the installation directory from the disk if present. async fn delete_unused_installations(installations_dir: &Path, state: &State) -> Result<(), Error> { + // We need to list all the available installations on the disk first, so we can check which + // installations in state file are absent from the disk. + let mut all_installations_on_disk: Vec = Vec::new(); + let mut entries = fs::read_dir(installations_dir).await?; + while let Some(item) = entries.next_entry().await? { + if item.file_type().await?.is_dir() { + let installation_dir_name = item.file_name(); + if let Some(name) = installation_dir_name.to_str() { + all_installations_on_disk.push(InstallationId(name.into())); + } + } + } + let unused_installations: Vec = state .installations() .iter() - .filter(|item| item.1.manifests().is_empty()) + .filter(|item| item.1.manifests().is_empty() || !all_installations_on_disk.contains(item.0)) .map(|item| item.0.to_owned()) .collect(); @@ -59,8 +72,8 @@ async fn delete_unused_installations(installations_dir: &Path, state: &State) -> // Remove installation directory from physical location. let installation_dir_to_delete = installations_dir.join(&installation.0); if installation_dir_to_delete.exists() { - tracing::info!( - "deleting unused installation directory {}", + tracing::debug!( + "Deleting unused installation directory {}", &installation_dir_to_delete.display() ); fs::remove_dir_all(&installation_dir_to_delete) @@ -90,7 +103,7 @@ async fn delete_untracked_installation_dirs( if !installations_in_state.contains_key(&InstallationId(name.into())) { are_untracked_installation_dirs_present = true; tracing::info!( - "deleting untracked installation directory {}", + "Deleting untracked installation directory {}", item.path().to_path_buf().display() ); @@ -106,7 +119,7 @@ async fn delete_untracked_installation_dirs( } if !are_untracked_installation_dirs_present { - tracing::info!("no untracked installation directories found",); + tracing::info!("No untracked installation directories found",); } Ok(()) diff --git a/crates/criticalup-cli/tests/cli/auth_set.rs b/crates/criticalup-cli/tests/cli/auth_set.rs index bc5a9025..35847db6 100644 --- a/crates/criticalup-cli/tests/cli/auth_set.rs +++ b/crates/criticalup-cli/tests/cli/auth_set.rs @@ -36,9 +36,12 @@ macro_rules! run_cmd { .expect("failed to execute command"); match &$expected { Some(expected) => { - // this regex replacement dance is required because this nested macro tests + // Since 1.84, stable Rust Clippy will complain about Regex inside a for loop. + // Unblocking ourselves by ignoring this warning in tests. + #[allow(clippy::regex_creation_in_loops)] + // This regex replacement dance is required because this nested macro tests // set is instantiating the test server twice which means each run gives - // a different local port. we replace with a stable port just for this test. + // a different local port. We replace with a stable port just for this test. let re = Regex::new(r"127.0.0.1:\d+").expect("regex creation failed."); let left_str = String::from_utf8(out.stderr.clone()) .expect("string creation from bytes failed."); diff --git a/crates/criticalup-cli/tests/cli/clean.rs b/crates/criticalup-cli/tests/cli/clean.rs index 8c70cbfe..6ef56734 100644 --- a/crates/criticalup-cli/tests/cli/clean.rs +++ b/crates/criticalup-cli/tests/cli/clean.rs @@ -17,13 +17,16 @@ async fn help_message() { #[tokio::test] async fn clean_deletes_only_unused_installations() { let test_env = TestEnvironment::prepare().await; - + let root = test_env.root(); + let toolchains_dir = root.join("toolchains"); + fs::create_dir_all(&toolchains_dir).unwrap(); let installation_id_1 = "installation_id_1"; let installation_id_2 = "installation_id_2"; let installation_id_3 = "installation_id_3"; let root = test_env.root().join("state.json"); let mut state_file = std::fs::File::create(&root).unwrap(); + let content = json!({ "version": 1, "authentication_token": "criticalup_token_45_hahaha", @@ -54,6 +57,14 @@ async fn clean_deletes_only_unused_installations() { .to_string(); state_file.write_all(content.as_bytes()).unwrap(); + fs::create_dir_all(toolchains_dir.join(installation_id_1)).unwrap(); + fs::create_dir_all(toolchains_dir.join(installation_id_2)).unwrap(); + fs::create_dir_all(toolchains_dir.join(installation_id_3)).unwrap(); + + assert!(toolchains_dir.join(installation_id_1).exists()); + assert!(toolchains_dir.join(installation_id_2).exists()); + assert!(toolchains_dir.join(installation_id_3).exists()); + assert_output!(test_env.cmd().args(["clean"])); let state_file_actual: Value = @@ -255,3 +266,93 @@ async fn removes_unused_installations_from_disk_that_do_not_have_state() { assert!(!toolchains_dir.join(installation_id_2).exists()); // Does not exist. assert!(!toolchains_dir.join(installation_id_3).exists()); // Does not exist. } + +/// Remove the installation from the state, if the installation directory does not exist. This is +/// true even if the installation in the state has a manifest. +#[tokio::test] +async fn clean_deletes_only_unused_installations_that_are_not_on_disk() { + let test_env = TestEnvironment::prepare().await; + let root = test_env.root(); + let toolchains_dir = root.join("toolchains"); + fs::create_dir_all(&toolchains_dir).unwrap(); + + let installation_id_1 = "installation_id_1"; + let installation_id_2 = "installation_id_2"; + let installation_id_3 = "installation_id_3"; + + let state_file_in_root = root.join("state.json"); + let mut state_file = std::fs::File::create(&state_file_in_root).unwrap(); + let content = json!({ + "version": 1, + "authentication_token": "criticalup_token_45_hahaha", + "installations": { + installation_id_1: { + "binary_proxies": { + "cargo": "/path/toolchains/bin/cargo" + }, + "manifests": [ + "/path/to/proj/1/criticalup.toml", + "/path/to/proj/2/criticalup.toml" + ] + }, + installation_id_2: { + "binary_proxies": { + "cargo": "/path/toolchains/bin/cargo" + }, + "manifests": ["/path/to/proj/3/criticalup.toml"] + }, + installation_id_3: { + "binary_proxies": { + "cargo": "/path/toolchains/bin/rustc" + }, + "manifests": [] + } + } + }) + .to_string(); + state_file.write_all(content.as_bytes()).unwrap(); + + // Create only one of the corresponding physical directories of installations. In this case we + // create only Installation ID 1. + // TODO: We have to generate these by running `install` command, once tests for those are setup. + fs::create_dir_all(toolchains_dir.join(installation_id_1)).unwrap(); + + assert!(toolchains_dir.join(installation_id_1).exists()); + assert!(!toolchains_dir.join(installation_id_2).exists()); + assert!(!toolchains_dir.join(installation_id_3).exists()); + + // Run the `clean` command. + assert_output!(test_env.cmd().args(["clean"])); + + // Test the actual values. + let state_file_actual: Value = + serde_json::from_reader(BufReader::new(File::open(&state_file_in_root).unwrap())).unwrap(); + + // "installation_id_2" is not present. + assert_eq!( + state_file_actual.pointer("/installations/installation_id_2"), + None + ); + // "installation_id_3" is not present. + assert_eq!( + state_file_actual.pointer("/installations/installation_id_3"), + None + ); + // "installation_id_1" is still present with correct values. + assert_eq!( + state_file_actual + .pointer("/installations/installation_id_1") + .unwrap(), + &json!({ + "binary_proxies": { + "cargo": "/path/toolchains/bin/cargo" + }, + "manifests": [ + "/path/to/proj/1/criticalup.toml", + "/path/to/proj/2/criticalup.toml" + ] + }) + ); + + assert!(toolchains_dir.join(installation_id_1).exists()); +} diff --git a/crates/criticalup-cli/tests/snapshots/cli__clean__clean_deletes_only_unused_installations.snap b/crates/criticalup-cli/tests/snapshots/cli__clean__clean_deletes_only_unused_installations.snap index eb3d318c..80c96f28 100644 --- a/crates/criticalup-cli/tests/snapshots/cli__clean__clean_deletes_only_unused_installations.snap +++ b/crates/criticalup-cli/tests/snapshots/cli__clean__clean_deletes_only_unused_installations.snap @@ -2,7 +2,7 @@ source: crates/criticalup-cli/tests/cli/clean.rs expression: repr --- -exit: exit status: 1 +exit: exit status: 0 empty stdout @@ -10,5 +10,5 @@ stderr ------ INFO Deleting unused installation installation_id_2 INFO Deleting unused installation installation_id_3 -error: No such file or directory (os error 2) + INFO No untracked installation directories found ------ diff --git a/crates/criticalup-cli/tests/snapshots/cli__clean__clean_deletes_only_unused_installations_also_from_disk.snap b/crates/criticalup-cli/tests/snapshots/cli__clean__clean_deletes_only_unused_installations_also_from_disk.snap index 60c69bc0..80c96f28 100644 --- a/crates/criticalup-cli/tests/snapshots/cli__clean__clean_deletes_only_unused_installations_also_from_disk.snap +++ b/crates/criticalup-cli/tests/snapshots/cli__clean__clean_deletes_only_unused_installations_also_from_disk.snap @@ -1,7 +1,6 @@ --- source: crates/criticalup-cli/tests/cli/clean.rs expression: repr -snapshot_kind: text --- exit: exit status: 0 @@ -10,8 +9,6 @@ empty stdout stderr ------ INFO Deleting unused installation installation_id_2 - INFO deleting unused installation directory /path/to/toolchain/installation/installation_id_2/ INFO Deleting unused installation installation_id_3 - INFO deleting unused installation directory /path/to/toolchain/installation/installation_id_3/ - INFO no untracked installation directories found + INFO No untracked installation directories found ------ diff --git a/crates/criticalup-cli/tests/snapshots/cli__clean__clean_deletes_only_unused_installations_that_are_not_on_disk.snap b/crates/criticalup-cli/tests/snapshots/cli__clean__clean_deletes_only_unused_installations_that_are_not_on_disk.snap new file mode 100644 index 00000000..80c96f28 --- /dev/null +++ b/crates/criticalup-cli/tests/snapshots/cli__clean__clean_deletes_only_unused_installations_that_are_not_on_disk.snap @@ -0,0 +1,14 @@ +--- +source: crates/criticalup-cli/tests/cli/clean.rs +expression: repr +--- +exit: exit status: 0 + +empty stdout + +stderr +------ + INFO Deleting unused installation installation_id_2 + INFO Deleting unused installation installation_id_3 + INFO No untracked installation directories found +------ diff --git a/crates/criticalup-cli/tests/snapshots/cli__clean__removes_unused_installations_from_disk_that_do_not_have_state.snap b/crates/criticalup-cli/tests/snapshots/cli__clean__removes_unused_installations_from_disk_that_do_not_have_state.snap index 0b172bcb..41f7da11 100644 --- a/crates/criticalup-cli/tests/snapshots/cli__clean__removes_unused_installations_from_disk_that_do_not_have_state.snap +++ b/crates/criticalup-cli/tests/snapshots/cli__clean__removes_unused_installations_from_disk_that_do_not_have_state.snap @@ -9,6 +9,5 @@ empty stdout stderr ------ INFO Deleting unused installation installation_id_2 - INFO deleting unused installation directory /path/to/toolchain/installation/installation_id_2/ - INFO deleting untracked installation directory /path/to/toolchain/installation/installation_id_3/ + INFO Deleting untracked installation directory /path/to/toolchain/installation/installation_id_3/ ------