Make warnings user-facing (#628)

## Summary

Now, `puffin_warnings::warn_once` and `puffin_warnings::warn` will go to
`stderr`, as long as the user isn't running under `--quiet`. Previously,
these went through `tracing`, and so were only visible when running
under `--verbose`.

Cargo.lock

@@ -2327,6 +2327,7 @@ dependencies = [
  "puffin-normalize",
  "puffin-resolver",
  "puffin-traits",
+ "puffin-warnings",
  "puffin-workspace",
  "pypi-types",
  "pyproject-toml",
@@ -2560,22 +2561,10 @@ dependencies = [
  "tracing",
 ]
 
-[[package]]
-name = "puffin-macros"
-version = "0.0.1"
-dependencies = [
- "colored",
- "once_cell",
- "rustc-hash",
- "tracing",
-]
-
 [[package]]
 name = "puffin-normalize"
 version = "0.0.1"
 dependencies = [
- "once_cell",
- "regex",
  "serde",
 ]
 
@@ -2610,9 +2599,9 @@ dependencies = [
  "puffin-distribution",
  "puffin-git",
  "puffin-interpreter",
- "puffin-macros",
  "puffin-normalize",
  "puffin-traits",
+ "puffin-warnings",
  "pypi-types",
  "reqwest",
  "rustc-hash",
@@ -2641,6 +2630,15 @@ dependencies = [
  "waitmap",
 ]
 
+[[package]]
+name = "puffin-warnings"
+version = "0.0.1"
+dependencies = [
+ "colored",
+ "once_cell",
+ "rustc-hash",
+]
+
 [[package]]
 name = "puffin-workspace"
 version = "0.0.1"
@@ -2738,8 +2736,8 @@ dependencies = [
  "once_cell",
  "pep440_rs 0.3.12",
  "pep508_rs",
- "puffin-macros",
  "puffin-normalize",
+ "puffin-warnings",
  "regex",
  "rfc2047-decoder",
  "serde",

crates/README.md

@@ -84,10 +84,6 @@ Functionality for installing Python packages into a virtual environment.
 
 Functionality for detecting and leveraging the current Python interpreter.
 
-## [puffin-macros](./puffin-macros)
-
-Reusable procedural macros for Puffin.
-
 ## [puffin-normalize](./puffin-normalize)
 
 Normalize package and extra names as per Python specifications.
@@ -108,6 +104,10 @@ Shared traits for Puffin, to avoid circular dependencies.
 
 General-purpose type definitions for types used in PyPI-compatible APIs.
 
+## [puffin-warnings](./puffin-warnings)
+
+User-facing warnings for Puffin.
+
 ## [requirements-txt](./requirements-txt)
 
 Functionality for parsing `requirements.txt` files.

crates/puffin-cli/Cargo.toml

@@ -17,6 +17,7 @@ name = "puffin"
 path = "src/main.rs"
 
 [dependencies]
+distribution-types = { path = "../distribution-types" }
 gourgeist = { path = "../gourgeist" }
 install-wheel-rs = { path = "../install-wheel-rs", default-features = false }
 pep440_rs = { path = "../pep440-rs" }
@@ -26,13 +27,13 @@ platform-tags = { path = "../platform-tags" }
 puffin-cache = { path = "../puffin-cache", features = ["clap"] }
 puffin-client = { path = "../puffin-client" }
 puffin-dispatch = { path = "../puffin-dispatch" }
-distribution-types = { path = "../distribution-types" }
 puffin-distribution = { path = "../puffin-distribution" }
 puffin-installer = { path = "../puffin-installer" }
 puffin-interpreter = { path = "../puffin-interpreter" }
 puffin-normalize = { path = "../puffin-normalize" }
 puffin-resolver = { path = "../puffin-resolver", features = ["clap"] }
 puffin-traits = { path = "../puffin-traits" }
+puffin-warnings = { path = "../puffin-warnings" }
 puffin-workspace = { path = "../puffin-workspace" }
 pypi-types = { path = "../pypi-types" }
 requirements-txt = { path = "../requirements-txt" }

crates/puffin-cli/src/main.rs

@@ -348,12 +348,14 @@ struct RemoveArgs {
 async fn inner() -> Result<ExitStatus> {
     let cli = Cli::parse();
 
+    // Configure the `tracing` crate, which controls internal logging.
     logging::setup_logging(if cli.verbose {
         logging::Level::Verbose
     } else {
         logging::Level::Default
     });
 
+    // Configure the `Printer`, which controls user-facing output in the CLI.
     let printer = if cli.quiet {
         printer::Printer::Quiet
     } else if cli.verbose {
@@ -362,6 +364,11 @@ async fn inner() -> Result<ExitStatus> {
         printer::Printer::Default
     };
 
+    // Configure the `warn!` macros, which control user-facing warnings in the CLI.
+    if !cli.quiet {
+        puffin_warnings::enable();
+    }
+
     let cache = Cache::try_from(cli.cache_args)?;
 
     match cli.command {

crates/puffin-cli/src/printer.rs

@@ -12,6 +12,7 @@ pub(crate) enum Printer {
 }
 
 impl Printer {
+    /// Return the [`ProgressDrawTarget`] for this printer.
     pub(crate) fn target(self) -> ProgressDrawTarget {
         match self {
             Self::Default => ProgressDrawTarget::stderr(),

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

@@ -8,9 +8,11 @@ info:
     - "--constraint"
     - constraints.txt
     - "--cache-dir"
-    - /tmp/.tmpZMtiDp
+    - /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpzGLCFa
+    - "--exclude-newer"
+    - "2023-11-18T12:00:00Z"
   env:
-    VIRTUAL_ENV: /tmp/.tmpYzlRGF/.venv
+    VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmp8uVkTx/.venv
 ---
 success: true
 exit_code: 0

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

@@ -8,9 +8,11 @@ info:
     - "--constraint"
     - constraints.txt
     - "--cache-dir"
-    - /tmp/.tmpqKe52b
+    - /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmp1JFjQT
+    - "--exclude-newer"
+    - "2023-11-18T12:00:00Z"
   env:
-    VIRTUAL_ENV: /tmp/.tmpUeuA4H/.venv
+    VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpjXkDoX/.venv
 ---
 success: true
 exit_code: 0

crates/puffin-macros/src/lib.rs

@@ -1,29 +0,0 @@
-use std::sync::Mutex;
-
-use once_cell::sync::Lazy;
-use rustc_hash::FxHashSet;
-
-// macro hygiene: The user might not have direct dependencies on those crates
-#[doc(hidden)]
-pub use colored;
-#[doc(hidden)]
-pub use tracing;
-
-pub static WARNINGS: Lazy<Mutex<FxHashSet<String>>> = Lazy::new(Mutex::default);
-
-/// Warn a user once, with uniqueness determined by the content of the message.
-#[macro_export]
-macro_rules! warn_once {
-    ($($arg:tt)*) => {
-        use $crate::colored::Colorize;
-        use $crate::tracing::warn;
-
-        if let Ok(mut states) = $crate::WARNINGS.lock() {
-            let message = format!("{}", format_args!($($arg)*));
-            let formatted = message.bold();
-            if states.insert(message) {
-                warn!("{formatted}");
-            }
-        }
-    };
-}

crates/puffin-normalize/Cargo.toml

@@ -5,6 +5,4 @@ edition = "2021"
 description = "Normalization for distribution, package and extra anmes"
 
 [dependencies]
-once_cell = { workspace = true }
-regex = { workspace = true }
 serde = { workspace = true, features = ["derive"] }

crates/puffin-resolver/Cargo.toml

@@ -25,7 +25,7 @@ distribution-types = { path = "../distribution-types" }
 puffin-distribution = { path = "../puffin-distribution" }
 puffin-git = { path = "../puffin-git" }
 puffin-interpreter = { path = "../puffin-interpreter" }
-puffin-macros = { path = "../puffin-macros" }
+puffin-warnings = { path = "../puffin-warnings" }
 puffin-normalize = { path = "../puffin-normalize" }
 puffin-traits = { path = "../puffin-traits" }
 pypi-types = { path = "../pypi-types" }

crates/puffin-resolver/src/pubgrub/dependencies.rs

@@ -5,7 +5,6 @@ use tracing::warn;
 
 use pep508_rs::{MarkerEnvironment, Requirement, VersionOrUrl};
 use puffin_cache::CanonicalUrl;
-use puffin_macros::warn_once;
 use puffin_normalize::{ExtraName, PackageName};
 
 use crate::pubgrub::specifier::PubGrubSpecifier;
@@ -31,7 +30,7 @@ impl PubGrubDependencies {
         for requirement in requirements {
             // Avoid self-dependencies.
             if source.is_some_and(|source| source == &requirement.name) {
-                warn_once!("{} has a dependency on itself", requirement.name);
+                warn!("{} has a dependency on itself", requirement.name);
                 continue;
             }
 
@@ -78,7 +77,7 @@ impl PubGrubDependencies {
         for constraint in constraints {
             // Avoid self-dependencies.
             if source.is_some_and(|source| source == &constraint.name) {
-                warn_once!("{} has a dependency on itself", constraint.name);
+                warn!("{} has a dependency on itself", constraint.name);
                 continue;
             }
 

crates/puffin-resolver/src/version_map.rs

@@ -9,8 +9,8 @@ use pep508_rs::MarkerEnvironment;
 use platform_tags::{TagPriority, Tags};
 use puffin_client::SimpleMetadata;
 use puffin_interpreter::Interpreter;
-use puffin_macros::warn_once;
 use puffin_normalize::PackageName;
+use puffin_warnings::warn_once;
 use pypi_types::Yanked;
 
 use crate::file::{DistFile, SdistFile, WheelFile};
@@ -64,7 +64,7 @@ impl VersionMap {
                         }
                         None => {
                             warn_once!(
-                                "{} is missing an upload date, but user provided {}",
+                                "{} is missing an upload date, but user provided: {}",
                                 file.filename,
                                 exclude_newer,
                             );

crates/puffin-warnings/Cargo.toml

@@ -1,5 +1,5 @@
 [package]
-name = "puffin-macros"
+name = "puffin-warnings"
 version = "0.0.1"
 edition = { workspace = true }
 rust-version = { workspace = true }
@@ -16,4 +16,3 @@ workspace = true
 colored = { workspace = true }
 once_cell = { workspace = true }
 rustc-hash = { workspace = true }
-tracing = { workspace = true }

crates/puffin-warnings/src/lib.rs

@@ -0,0 +1,51 @@
+use std::sync::atomic::AtomicBool;
+use std::sync::Mutex;
+
+// macro hygiene: The user might not have direct dependencies on those crates
+#[doc(hidden)]
+pub use colored;
+use once_cell::sync::Lazy;
+use rustc_hash::FxHashSet;
+
+/// Whether user-facing warnings are enabled.
+pub static ENABLED: AtomicBool = AtomicBool::new(false);
+
+/// Enable user-facing warnings.
+pub fn enable() {
+    ENABLED.store(true, std::sync::atomic::Ordering::SeqCst);
+}
+
+/// Warn a user, if warnings are enabled.
+#[macro_export]
+macro_rules! warn {
+    ($($arg:tt)*) => {
+        use $crate::colored::Colorize;
+
+        if $crate::ENABLED.load(std::sync::atomic::Ordering::SeqCst) {
+            let message = format!("{}", format_args!($($arg)*));
+            let formatted = message.bold();
+            eprintln!("{}{} {formatted}", "warning".yellow().bold(), ":".bold());
+        }
+    };
+}
+
+pub static WARNINGS: Lazy<Mutex<FxHashSet<String>>> = Lazy::new(Mutex::default);
+
+/// Warn a user once, if warnings are enabled, with uniqueness determined by the content of the
+/// message.
+#[macro_export]
+macro_rules! warn_once {
+    ($($arg:tt)*) => {
+        use $crate::colored::Colorize;
+
+        if $crate::ENABLED.load(std::sync::atomic::Ordering::SeqCst) {
+            if let Ok(mut states) = $crate::WARNINGS.lock() {
+                let message = format!("{}", format_args!($($arg)*));
+                let formatted = message.bold();
+                if states.insert(message) {
+                    eprintln!("{}{} {formatted}", "warning".yellow().bold(), ":".bold());
+                }
+            }
+        }
+    };
+}

crates/pypi-types/Cargo.toml

@@ -16,7 +16,7 @@ workspace = true
 pep440_rs = { path = "../pep440-rs", features = ["serde"] }
 pep508_rs = { path = "../pep508-rs", features = ["serde"] }
 puffin-normalize = { path = "../puffin-normalize" }
-puffin-macros = { path = "../puffin-macros" }
+puffin-warnings = { path = "../puffin-warnings" }
 
 chrono = { workspace = true, features = ["serde"] }
 mailparse = { workspace = true }

crates/pypi-types/src/lenient_requirement.rs

@@ -6,7 +6,7 @@ use serde::{de, Deserialize, Deserializer, Serialize};
 
 use pep440_rs::{Pep440Error, VersionSpecifiers};
 use pep508_rs::{Pep508Error, Requirement};
-use puffin_macros::warn_once;
+use puffin_warnings::warn_once;
 
 /// Ex) `>=7.2.0<8.0.0`
 static MISSING_COMMA: Lazy<Regex> = Lazy::new(|| Regex::new(r"(\d)([<>=~^!])").unwrap());
@@ -63,8 +63,8 @@ fn parse_with_fixups<Err, T: FromStr<Err = Err>>(input: &str, type_name: &str) -
 
             if let Ok(requirement) = T::from_str(&patched_input) {
                 warn_once!(
-                    "{} to fix invalid {type_name} (before: `{input}`; after: `{patched_input}`)",
+                    "Fixing invalid {type_name} by {} (before: `{input}`; after: `{patched_input}`)",
-                    messages.join(" and ")
+                    messages.join(", ")
                 );
                 return Ok(requirement);
             }