Add pks rm and bump the version to 0.6.0

CHANGELOG.md

@@ -1,6 +1,29 @@
 # Changelog
 
-## Unreleased
+## 0.6.0
+
+### Features
+
+#### `pks rm` removes a pack
+
+`pks rm packs/foo` deletes the pack's directory, after removing every mention of
+it from other packs: their `dependencies`, `ignored_dependencies` and
+`visible_to` lists, and the violations their `package_todo.yml` files record on
+it. A todo file with nothing left in it is deleted. Like `add-dependency`, it
+rewrites each `package.yml` it changes, which drops the file's comments and puts
+its keys in pks's order.
+
+By default it won't remove a pack that other packs may still use. It first
+looks for references from other packs to the pack's constants and lists any it
+finds, because once the pack is gone `pks check` can no longer see them. It also
+won't remove a pack with Ruby files under `lib/` that are outside the autoload
+roots, since pks can't see references to the constants those files define. The
+experimental parser reads definitions from every file, so with
+`--experimental-parser` that check is skipped. `--force` removes the pack anyway
+and leaves any remaining references for you to fix.
+
+The root pack can't be removed. Neither can a pack with other packs inside it,
+until those are removed.
 
 ### Fixes
 
@@ -83,6 +106,22 @@ longer found, so `pks check` reports it as stale until you run `pks update`. For
 the same reason, `check-unused-dependencies` may now report a dependency that
 only such an association used. packwerk still reports these references.
 
+### Performance
+
+#### A warm `pks check` skips reading files that haven't changed
+
+The cache now records each file's modification time and size along with its
+digest, and serves a file whose time and size both still match without reading
+it. On a 51,000-file application this made a warm `pks check` 1.43 times as
+fast. When either has changed, pks reads the file and compares digests as
+before. On a filesystem that records times only to the second, where a
+same-size edit within a second can keep both, it never takes the shortcut.
+
+A tool that restores file times, such as `rsync -t`, `tar -p` or `cp -p`, can
+put back different contents with the same size and time, and pks then serves
+the old cached result. `touch` the file, or run with `--no-cache`, to check it
+again.
+
 ## 0.5.0
 
 ### Breaking Changes

Cargo.toml

@@ -2,7 +2,7 @@
 
 [package]
 name = "pks"
-version = "0.5.0"
+version = "0.6.0"
 edition = "2021"
 description = "Welcome! Please see https://github.com/rubyatscale/pks for more information!"
 license = "MIT"

README.md

@@ -46,6 +46,7 @@ Usage: pks [OPTIONS] <COMMAND>
 Commands:
   greet                           Just saying hi
   create                          Create a new pack
+  rm                              Delete a pack and remove references to it from other packs
   check                           Look for violations in the codebase
   check-contents                  Check file contents piped to stdin
   update                          Update package_todo.yml files with the current violations

src/packs.rs

@@ -18,6 +18,7 @@ pub(crate) mod monkey_patch_detection;
 pub(crate) mod pack;
 pub(crate) mod parsing;
 pub(crate) mod raw_configuration;
+pub(crate) mod remover;
 pub(crate) mod template;
 pub(crate) mod text;
 pub mod walk_directory;
@@ -75,6 +76,14 @@ pub fn create(
     Ok(())
 }
 
+pub fn remove(
+    configuration: &Configuration,
+    name: String,
+    force: bool,
+) -> anyhow::Result<()> {
+    remover::remove(configuration, &name, force)
+}
+
 /// Determine whether to use colors based on the color choice
 fn color_mode_for(color: ColorChoice) -> text::ColorMode {
     match color {

src/packs/checker.rs

@@ -451,7 +451,7 @@ pub(crate) fn update(configuration: &Configuration) -> anyhow::Result<()> {
         configuration,
         violations,
         recorded_violations,
-    );
+    )?;
     println!("Successfully updated package_todo.yml files!");
 
     Ok(())

src/packs/cli.rs

@@ -84,6 +84,18 @@ enum Command {
     #[clap(about = "Create a new pack")]
     Create { name: String },
 
+    #[clap(
+        about = "Delete a pack and remove references to it from other packs"
+    )]
+    Rm {
+        /// The pack to delete
+        pack: String,
+
+        /// Delete the pack even if other packs may still use its constants
+        #[arg(short, long)]
+        force: bool,
+    },
+
     #[clap(about = "Look for violations in the codebase")]
     Check {
         /// Ignore recorded violations when reporting violations
@@ -334,5 +346,8 @@ pub fn run() -> anyhow::Result<()> {
             packs::lint_package_yml_files(&configuration)
         }
         Command::Create { name } => packs::create(&configuration, name),
+        Command::Rm { pack, force } => {
+            packs::remove(&configuration, pack, force)
+        }
     }
 }

src/packs/package_todo.rs

@@ -1,3 +1,4 @@
+use anyhow::Context;
 use rayon::prelude::{IntoParallelRefIterator, ParallelIterator};
 use serde::{ser::SerializeMap, Deserialize, Serialize, Serializer};
 use std::collections::{BTreeMap, HashMap, HashSet};
@@ -135,7 +136,7 @@ pub fn write_violations_to_disk(
     configuration: &Configuration,
     violations: HashSet<Violation>,
     recorded_violations: &HashSet<ViolationIdentifier>,
-) {
+) -> anyhow::Result<()> {
     debug!("Starting writing violations to disk");
     // First we need to group the violations by the responsible pack, which today is always the referencing pack
     // Later if we change where a violation shows up, we should delegate to the checker
@@ -168,7 +169,7 @@ pub fn write_violations_to_disk(
         package_todos_for_pack_name(violations_by_responsible_pack);
 
     let all_packs = &configuration.pack_set.packs;
-    all_packs.par_iter().for_each(|p| {
+    all_packs.par_iter().try_for_each(|p| {
         let package_todo = package_todos_by_pack_name.get(&p.name);
         match package_todo {
             Some(package_todo) => write_package_todo_to_disk(
@@ -178,9 +179,10 @@ pub fn write_violations_to_disk(
             ),
             None => delete_package_todo_from_disk(p),
         }
-    });
+    })?;
 
     debug!("Finished writing violations to disk");
+    Ok(())
 }
 
 fn serialize_package_todo(
@@ -197,42 +199,51 @@ fn serialize_package_todo(
     header + &package_todo_yml
 }
 
-fn write_package_todo_to_disk(
+pub(crate) fn write_package_todo_to_disk(
     responsible_pack: &Pack,
     package_todo: &PackageTodo,
     packs_first_mode: bool,
-) {
+) -> anyhow::Result<()> {
     let package_todo_yml_absolute_filepath = responsible_pack
         .yml
         .parent()
         .unwrap()
         .join("package_todo.yml");
 
-    if !package_todo_yml_absolute_filepath.exists() {
-        std::fs::File::create(&package_todo_yml_absolute_filepath).unwrap();
-    }
-
     let package_todo_yml = serialize_package_todo(
         &responsible_pack.name,
         package_todo,
         packs_first_mode,
     );
 
-    std::fs::write(package_todo_yml_absolute_filepath, package_todo_yml)
-        .unwrap();
+    std::fs::write(&package_todo_yml_absolute_filepath, package_todo_yml)
+        .with_context(|| {
+            format!(
+                "Failed to write {}",
+                package_todo_yml_absolute_filepath.display()
+            )
+        })
 }
 
-fn delete_package_todo_from_disk(responsible_pack: &Pack) {
+pub(crate) fn delete_package_todo_from_disk(
+    responsible_pack: &Pack,
+) -> anyhow::Result<()> {
     let package_todo_yml_absolute_filepath = responsible_pack
         .yml
         .parent()
         .unwrap()
         .join("package_todo.yml");
 
     if package_todo_yml_absolute_filepath.exists() {
-        // Delete package_todo_yml_absolute_filepath
-        std::fs::remove_file(package_todo_yml_absolute_filepath).unwrap();
+        std::fs::remove_file(&package_todo_yml_absolute_filepath)
+            .with_context(|| {
+                format!(
+                    "Failed to delete {}",
+                    package_todo_yml_absolute_filepath.display()
+                )
+            })?;
     }
+    Ok(())
 }
 
 fn header(responsible_pack_name: &String, packs_first_mode: bool) -> String {