Improve and test diagnostics for requirements-reading CLI commands (#143)

Also removes `owo_colors` because it was really painful to get it to
avoid printing colors during tests.

Cargo.lock

@@ -448,6 +448,17 @@ version = "1.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "acbf1af155f9b9ef647e42cdc158db4b64a1b61f743629225fde6f3e0be2a7c7"
 
+[[package]]
+name = "colored"
+version = "2.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "2674ec482fbc38012cf31e6c42ba0177b431a0cb6f15fe40efa5aab1bda516f6"
+dependencies = [
+ "is-terminal",
+ "lazy_static",
+ "windows-sys 0.48.0",
+]
+
 [[package]]
 name = "configparser"
 version = "3.0.2"
@@ -1966,13 +1977,13 @@ version = "0.0.1"
 dependencies = [
  "anyhow",
  "clap",
+ "colored",
  "directories",
  "flate2",
  "fs-err",
  "gourgeist",
  "indoc 2.0.4",
  "itertools",
- "owo-colors",
  "pep508_rs",
  "platform-host",
  "platform-tags",
@@ -2006,6 +2017,7 @@ dependencies = [
  "bitflags 2.4.1",
  "cacache",
  "clap",
+ "colored",
  "directories",
  "fs-err",
  "futures",
@@ -2015,7 +2027,6 @@ dependencies = [
  "install-wheel-rs",
  "itertools",
  "miette",
- "owo-colors",
  "pep440_rs 0.3.12",
  "pep508_rs",
  "platform-host",

Cargo.toml

@@ -18,6 +18,7 @@ bitflags = { version = "2.4.0" }
 cacache = { version = "11.7.1", default-features = false, features = ["tokio-runtime"] }
 camino = { version = "1.1.6", features = ["serde1"] }
 clap = { version = "4.4.6" }
+colored = { version = "2.0.4" }
 configparser = { version = "3.0.2" }
 csv = { version = "1.3.0" }
 data-encoding = { version = "2.4.0" }
@@ -37,7 +38,6 @@ mailparse = { version = "0.14.0" }
 memchr = { version = "2.6.4" }
 miette = { version = "5.10.0" }
 once_cell = { version = "1.18.0" }
-owo-colors = { version = "3.5.0" }
 platform-info = { version = "2.0.2" }
 plist = { version = "1.5.0" }
 pyproject-toml = { version = "0.7.0" }

crates/puffin-build/Cargo.toml

@@ -24,12 +24,12 @@ puffin-workspace = { path = "../puffin-workspace" }
 
 anyhow = { workspace = true }
 clap = { workspace = true, features = ["derive"] }
+colored = { workspace = true }
 directories = { workspace = true }
 flate2 = { workspace = true }
 fs-err = { workspace = true }
 indoc = { workspace = true }
 itertools = { workspace = true }
-owo-colors = { workspace = true }
 pyproject-toml = { workspace = true }
 serde = { workspace = true }
 serde_json = { workspace = true }

crates/puffin-build/src/main.rs

@@ -2,9 +2,9 @@
 
 use anyhow::Context;
 use clap::Parser;
+use colored::Colorize;
 use directories::ProjectDirs;
 use fs_err as fs;
-use owo_colors::OwoColorize;
 use puffin_build::{Error, SourceDistributionBuilder};
 use std::path::PathBuf;
 use std::process::ExitCode;

crates/puffin-cli/Cargo.toml

@@ -26,13 +26,13 @@ anyhow = { workspace = true }
 bitflags = { workspace = true }
 cacache = { workspace = true }
 clap = { workspace = true, features = ["derive"] }
+colored = { workspace = true }
 directories = { workspace = true }
 fs-err = { workspace = true, features = ["tokio"] }
 futures = { workspace = true }
 indicatif = { workspace = true }
 itertools = { workspace = true }
 miette = { workspace = true, features = ["fancy"] }
-owo-colors = { workspace = true }
 pyproject-toml = { workspace = true }
 tempfile = { workspace = true }
 thiserror = { workspace = true }

crates/puffin-cli/src/commands/pip_compile.rs

@@ -3,9 +3,9 @@ use std::io::{stdout, BufWriter};
 use std::path::Path;
 
 use anyhow::Result;
+use colored::Colorize;
 use fs_err::File;
 use itertools::Itertools;
-use owo_colors::OwoColorize;
 use pubgrub::report::Reporter;
 use tracing::debug;
 
@@ -28,6 +28,13 @@ pub(crate) async fn pip_compile(
 ) -> Result<ExitStatus> {
     let start = std::time::Instant::now();
 
+    // Read all requirements from the provided sources.
+    let requirements = sources
+        .iter()
+        .map(RequirementsSource::requirements)
+        .flatten_ok()
+        .collect::<Result<Vec<Requirement>>>()?;
+
     // Detect the current Python interpreter.
     let platform = Platform::current()?;
     let python = PythonExecutable::from_env(platform, cache)?;
@@ -36,13 +43,6 @@ pub(crate) async fn pip_compile(
         python.executable().display()
     );
 
-    // Read all requirements from the provided sources.
-    let requirements = sources
-        .iter()
-        .map(RequirementsSource::requirements)
-        .flatten_ok()
-        .collect::<Result<Vec<Requirement>>>()?;
-
     // Determine the current environment markers.
     let markers = python.markers();
 

crates/puffin-cli/src/commands/pip_sync.rs

@@ -2,8 +2,8 @@ use std::fmt::Write;
 use std::path::Path;
 
 use anyhow::{Context, Result};
+use colored::Colorize;
 use itertools::Itertools;
-use owo_colors::OwoColorize;
 use tracing::debug;
 
 use pep508_rs::Requirement;
@@ -247,7 +247,7 @@ pub(crate) async fn sync_requirements(
                     printer,
                     " {} {}{}",
                     "+".green(),
-                    event.distribution.name().white().bold(),
+                    event.distribution.name().as_ref().white().bold(),
                     format!("@{}", event.distribution.version()).dimmed()
                 )?;
             }
@@ -256,7 +256,7 @@ pub(crate) async fn sync_requirements(
                     printer,
                     " {} {}{}",
                     "-".red(),
-                    event.distribution.name().white().bold(),
+                    event.distribution.name().as_ref().white().bold(),
                     format!("@{}", event.distribution.version()).dimmed()
                 )?;
             }

crates/puffin-cli/src/commands/pip_uninstall.rs

@@ -2,8 +2,8 @@ use std::fmt::Write;
 use std::path::Path;
 
 use anyhow::Result;
+use colored::Colorize;
 use itertools::Itertools;
-use owo_colors::OwoColorize;
 use tracing::debug;
 
 use pep508_rs::Requirement;
@@ -23,6 +23,13 @@ pub(crate) async fn pip_uninstall(
 ) -> Result<ExitStatus> {
     let start = std::time::Instant::now();
 
+    // Read all requirements from the provided sources.
+    let requirements = sources
+        .iter()
+        .map(RequirementsSource::requirements)
+        .flatten_ok()
+        .collect::<Result<Vec<Requirement>>>()?;
+
     // Detect the current Python interpreter.
     let platform = Platform::current()?;
     let python = PythonExecutable::from_env(platform, cache)?;
@@ -31,13 +38,6 @@ pub(crate) async fn pip_uninstall(
         python.executable().display()
     );
 
-    // Read all requirements from the provided sources.
-    let requirements = sources
-        .iter()
-        .map(RequirementsSource::requirements)
-        .flatten_ok()
-        .collect::<Result<Vec<Requirement>>>()?;
-
     // Index the current `site-packages` directory.
     let site_packages = puffin_installer::SitePackages::from_executable(&python).await?;
 
@@ -64,7 +64,7 @@ pub(crate) async fn pip_uninstall(
                     "{}{} Skipping {} as it is not installed.",
                     "warning".yellow().bold(),
                     ":".bold(),
-                    package.bold()
+                    package.as_ref().bold()
                 );
                 None
             }

crates/puffin-cli/src/commands/venv.rs

@@ -2,8 +2,8 @@ use std::fmt::Write;
 use std::path::Path;
 
 use anyhow::Result;
+use colored::Colorize;
 use fs_err::tokio as fs;
-use owo_colors::OwoColorize;
 
 use crate::commands::ExitStatus;
 use crate::printer::Printer;
@@ -25,7 +25,7 @@ pub(crate) async fn venv(
     writeln!(
         printer,
         "Using Python interpreter: {}",
-        base_python.display().cyan()
+        format!("{}", base_python.display()).cyan()
     )?;
 
     // If the path already exists, remove it.
@@ -35,7 +35,7 @@ pub(crate) async fn venv(
     writeln!(
         printer,
         "Creating virtual environment at: {}",
-        path.display().cyan()
+        format!("{}", path.display()).cyan()
     )?;
 
     // Create the virtual environment.

crates/puffin-cli/src/main.rs

@@ -2,8 +2,8 @@ use std::path::PathBuf;
 use std::process::ExitCode;
 
 use clap::{Args, Parser, Subcommand};
+use colored::Colorize;
 use directories::ProjectDirs;
-use owo_colors::OwoColorize;
 
 use crate::commands::ExitStatus;
 use crate::requirements::RequirementsSource;

crates/puffin-cli/src/requirements.rs

@@ -1,7 +1,9 @@
+//! A standard interface for working with heterogeneous sources of requirements.
+
 use std::path::PathBuf;
 use std::str::FromStr;
 
-use anyhow::{bail, Result};
+use anyhow::{bail, Context, Result};
 use fs_err as fs;
 
 use pep508_rs::Requirement;
@@ -37,7 +39,8 @@ impl RequirementsSource {
     /// Return an iterator over the requirements in this source.
     pub(crate) fn requirements(&self) -> Result<impl Iterator<Item = Requirement>> {
         let iter_name = if let Self::Name(name) = self {
-            let requirement = Requirement::from_str(name)?;
+            let requirement =
+                Requirement::from_str(name).with_context(|| format!("Failed to parse `{name}`"))?;
             Some(std::iter::once(requirement))
         } else {
             None
@@ -59,8 +62,9 @@ impl RequirementsSource {
         };
 
         let iter_pyproject_toml = if let Self::PyprojectToml(path) = self {
-            let pyproject_toml =
+            let contents = fs::read_to_string(path)?;
-                toml::from_str::<pyproject_toml::PyProjectToml>(&fs::read_to_string(path)?)?;
+            let pyproject_toml = toml::from_str::<pyproject_toml::PyProjectToml>(&contents)
+                .with_context(|| format!("Failed to read `{}`", path.display()))?;
             Some(
                 pyproject_toml
                     .project

crates/puffin-cli/tests/pip_uninstall.rs

@@ -0,0 +1,125 @@
+use std::process::Command;
+
+use anyhow::Result;
+use assert_fs::prelude::*;
+use insta_cmd::{assert_cmd_snapshot, get_cargo_bin};
+
+const BIN_NAME: &str = "puffin";
+
+#[test]
+fn no_arguments() -> Result<()> {
+    let tempdir = assert_fs::TempDir::new()?;
+
+    assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
+        .arg("pip-uninstall")
+        .current_dir(&tempdir));
+
+    Ok(())
+}
+
+#[test]
+fn invalid_requirement() -> Result<()> {
+    let tempdir = assert_fs::TempDir::new()?;
+
+    assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
+        .arg("pip-uninstall")
+        .arg("flask==1.0.x")
+        .current_dir(&tempdir));
+
+    Ok(())
+}
+
+#[test]
+fn missing_requirements_txt() -> Result<()> {
+    let tempdir = assert_fs::TempDir::new()?;
+
+    assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
+        .arg("pip-uninstall")
+        .arg("-r")
+        .arg("requirements.txt")
+        .current_dir(&tempdir));
+
+    Ok(())
+}
+
+#[test]
+fn invalid_requirements_txt_requirement() -> Result<()> {
+    let tempdir = assert_fs::TempDir::new()?;
+    let requirements_txt = tempdir.child("requirements.txt");
+    requirements_txt.touch()?;
+    requirements_txt.write_str("flask==1.0.x")?;
+
+    assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
+        .arg("pip-uninstall")
+        .arg("-r")
+        .arg("requirements.txt")
+        .current_dir(&tempdir));
+
+    Ok(())
+}
+
+#[test]
+fn missing_pyproject_toml() -> Result<()> {
+    let tempdir = assert_fs::TempDir::new()?;
+
+    assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
+        .arg("pip-uninstall")
+        .arg("-r")
+        .arg("pyproject.toml")
+        .current_dir(&tempdir));
+
+    Ok(())
+}
+
+#[test]
+fn invalid_pyproject_toml_syntax() -> Result<()> {
+    let tempdir = assert_fs::TempDir::new()?;
+    let pyproject_toml = tempdir.child("pyproject.toml");
+    pyproject_toml.touch()?;
+    pyproject_toml.write_str("123 - 456")?;
+
+    assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
+        .arg("pip-uninstall")
+        .arg("-r")
+        .arg("pyproject.toml")
+        .current_dir(&tempdir));
+
+    Ok(())
+}
+
+#[test]
+fn invalid_pyproject_toml_schema() -> Result<()> {
+    let tempdir = assert_fs::TempDir::new()?;
+    let pyproject_toml = tempdir.child("pyproject.toml");
+    pyproject_toml.touch()?;
+    pyproject_toml.write_str("[project]")?;
+
+    assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
+        .arg("pip-uninstall")
+        .arg("-r")
+        .arg("pyproject.toml")
+        .current_dir(&tempdir));
+
+    Ok(())
+}
+
+#[test]
+fn invalid_pyproject_toml_requirement() -> Result<()> {
+    let tempdir = assert_fs::TempDir::new()?;
+    let pyproject_toml = tempdir.child("pyproject.toml");
+    pyproject_toml.touch()?;
+    pyproject_toml.write_str(
+        r#"[project]
+name = "project"
+dependencies = ["flask==1.0.x"]
+"#,
+    )?;
+
+    assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME))
+        .arg("pip-uninstall")
+        .arg("-r")
+        .arg("pyproject.toml")
+        .current_dir(&tempdir));
+
+    Ok(())
+}

crates/puffin-cli/tests/snapshots/pip_uninstall__invalid_pyproject_toml_requirement.snap

@@ -0,0 +1,24 @@
+---
+source: crates/puffin-cli/tests/pip_uninstall.rs
+info:
+  program: puffin
+  args:
+    - pip-uninstall
+    - "-r"
+    - pyproject.toml
+---
+success: false
+exit_code: 2
+----- stdout -----
+
+----- stderr -----
+error: Failed to read `pyproject.toml`
+  Caused by: TOML parse error at line 3, column 16
+  |
+3 | dependencies = ["flask==1.0.x"]
+  |                ^^^^^^^^^^^^^^^^
+Version specifier `==1.0.x` doesn't match PEP 440 rules
+flask==1.0.x
+     ^^^^^^^
+
+

crates/puffin-cli/tests/snapshots/pip_uninstall__invalid_pyproject_toml_schema.snap

@@ -0,0 +1,22 @@
+---
+source: crates/puffin-cli/tests/pip_uninstall.rs
+info:
+  program: puffin
+  args:
+    - pip-uninstall
+    - "-r"
+    - pyproject.toml
+---
+success: false
+exit_code: 2
+----- stdout -----
+
+----- stderr -----
+error: Failed to read `pyproject.toml`
+  Caused by: TOML parse error at line 1, column 1
+  |
+1 | [project]
+  | ^^^^^^^^^
+missing field `name`
+
+

crates/puffin-cli/tests/snapshots/pip_uninstall__invalid_pyproject_toml_syntax.snap

@@ -0,0 +1,22 @@
+---
+source: crates/puffin-cli/tests/pip_uninstall.rs
+info:
+  program: puffin
+  args:
+    - pip-uninstall
+    - "-r"
+    - pyproject.toml
+---
+success: false
+exit_code: 2
+----- stdout -----
+
+----- stderr -----
+error: Failed to read `pyproject.toml`
+  Caused by: TOML parse error at line 1, column 5
+  |
+1 | 123 - 456
+  |     ^
+expected `.`, `=`
+
+

crates/puffin-cli/tests/snapshots/pip_uninstall__invalid_requirement.snap

@@ -0,0 +1,18 @@
+---
+source: crates/puffin-cli/tests/pip_uninstall.rs
+info:
+  program: puffin
+  args:
+    - pip-uninstall
+    - flask==1.0.x
+---
+success: false
+exit_code: 2
+----- stdout -----
+
+----- stderr -----
+error: Failed to parse `flask==1.0.x`
+  Caused by: Version specifier `==1.0.x` doesn't match PEP 440 rules
+flask==1.0.x
+     ^^^^^^^
+

crates/puffin-cli/tests/snapshots/pip_uninstall__invalid_requirements_txt_requirement.snap

@@ -0,0 +1,19 @@
+---
+source: crates/puffin-cli/tests/pip_uninstall.rs
+info:
+  program: puffin
+  args:
+    - pip-uninstall
+    - "-r"
+    - requirements.txt
+---
+success: false
+exit_code: 2
+----- stdout -----
+
+----- stderr -----
+error: Couldn't parse requirement in requirements.txt position 0 to 12
+  Caused by: Version specifier `==1.0.x` doesn't match PEP 440 rules
+flask==1.0.x
+     ^^^^^^^
+

crates/puffin-cli/tests/snapshots/pip_uninstall__missing_pyproject_toml.snap

@@ -0,0 +1,17 @@
+---
+source: crates/puffin-cli/tests/pip_uninstall.rs
+info:
+  program: puffin
+  args:
+    - pip-uninstall
+    - "-r"
+    - pyproject.toml
+---
+success: false
+exit_code: 2
+----- stdout -----
+
+----- stderr -----
+error: failed to open file `pyproject.toml`
+  Caused by: No such file or directory (os error 2)
+

crates/puffin-cli/tests/snapshots/pip_uninstall__missing_requirements_txt.snap

@@ -0,0 +1,17 @@
+---
+source: crates/puffin-cli/tests/pip_uninstall.rs
+info:
+  program: puffin
+  args:
+    - pip-uninstall
+    - "-r"
+    - requirements.txt
+---
+success: false
+exit_code: 2
+----- stdout -----
+
+----- stderr -----
+error: failed to open file `requirements.txt`
+  Caused by: No such file or directory (os error 2)
+

crates/puffin-cli/tests/snapshots/pip_uninstall__no_arguments.snap

@@ -0,0 +1,19 @@
+---
+source: crates/puffin-cli/tests/pip_uninstall.rs
+info:
+  program: puffin
+  args:
+    - pip-uninstall
+---
+success: false
+exit_code: 2
+----- stdout -----
+
+----- stderr -----
+error: the following required arguments were not provided:
+  <PACKAGE|--requirement <REQUIREMENT>>
+
+Usage: puffin pip-uninstall <PACKAGE|--requirement <REQUIREMENT>>
+
+For more information, try '--help'.
+