Clean command: removes installation from state if directory missing.

This was a bug that if the directory of the installation is
missing from the file system, and the installation is still
present in the state file, the clean command will not remove the
installation from the state file. This was certainly a problem
because if there is accidental deletion of the toolchain
directory, we need to be able to clean up the state file too.

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<InstallationId> = 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<InstallationId> = 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();
 

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());
+}

crates/criticalup-cli/tests/snapshots/cli__clean__clean_deletes_only_unused_installations.snap

@@ -2,13 +2,15 @@
 source: crates/criticalup-cli/tests/cli/clean.rs
 expression: repr
 ---
-exit: exit status: 1
+exit: exit status: 0
 
 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
-error: No such file or directory (os error 2)
+ INFO deleting unused installation directory /path/to/toolchain/installation/installation_id_3/
+ INFO no untracked installation directories found
 ------

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
+------