From a2ef2010d258ffd21b9942113e3790e03c26c1ec Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 26 Jan 2024 13:54:02 -0800 Subject: [PATCH] Add arguments for pip-compile compatibility (#1139) ## Summary This ensures that we warn when redundant options are passed (like `--allow-unsafe`, which is really common for forwards compatibility since it's going to be the default in a future release), and errors when known variants are passed that we _don't_ support (like `--resolver=backtracking`). Closes https://github.com/astral-sh/puffin/issues/1127. --- crates/puffin/src/compat/mod.rs | 260 +++++++++++++++++++++++++++++ crates/puffin/src/main.rs | 6 + crates/puffin/tests/pip_compile.rs | 78 +++++++++ 3 files changed, 344 insertions(+) create mode 100644 crates/puffin/src/compat/mod.rs diff --git a/crates/puffin/src/compat/mod.rs b/crates/puffin/src/compat/mod.rs new file mode 100644 index 000000000..254d6f865 --- /dev/null +++ b/crates/puffin/src/compat/mod.rs @@ -0,0 +1,260 @@ +use anyhow::{anyhow, Result}; +use clap::{Args, ValueEnum}; + +use puffin_warnings::warn_user; + +/// Arguments for `pip-compile` compatibility. +/// +/// These represent a subset of the `pip-compile` interface that Puffin supports by default. +/// For example, users often pass `--allow-unsafe`, which is unnecessary with Puffin. But it's a +/// nice user experience to warn, rather than fail, when users pass `--allow-unsafe`. +#[derive(Args)] +#[allow(clippy::struct_excessive_bools)] +pub(crate) struct PipCompileCompatArgs { + #[clap(long, hide = true)] + allow_unsafe: bool, + + #[clap(long, hide = true)] + no_allow_unsafe: bool, + + #[clap(long, hide = true)] + reuse_hashes: bool, + + #[clap(long, hide = true)] + no_reuse_hashes: bool, + + #[clap(long, hide = true)] + build_isolation: bool, + + #[clap(long, hide = true)] + no_build_isolation: bool, + + #[clap(long, hide = true)] + resolver: Option, + + #[clap(long, hide = true)] + annotation_style: Option, + + #[clap(long, hide = true)] + max_rounds: Option, + + #[clap(long, hide = true)] + cert: Option, + + #[clap(long, hide = true)] + client_cert: Option, + + #[clap(long, hide = true)] + trusted_host: Option, + + #[clap(long, hide = true)] + emit_trusted_host: bool, + + #[clap(long, hide = true)] + no_emit_trusted_host: bool, + + #[clap(long, hide = true)] + unsafe_package: Vec, + + #[clap(long, hide = true)] + no_config: bool, + + #[clap(long, hide = true)] + emit_index_url: bool, + + #[clap(long, hide = true)] + no_emit_index_url: bool, + + #[clap(long, hide = true)] + emit_find_links: bool, + + #[clap(long, hide = true)] + no_emit_find_links: bool, + + #[clap(long, hide = true)] + emit_options: bool, + + #[clap(long, hide = true)] + no_emit_options: bool, + + #[clap(long, hide = true)] + strip_extras: bool, + + #[clap(long, hide = true)] + no_strip_extras: bool, +} + +impl PipCompileCompatArgs { + /// Validate the arguments passed for `pip-compile` compatibility. + /// + /// This method will warn when an argument is passed that has no effect but matches Puffin's + /// behavior. If an argument is passed that does _not_ match Puffin's behavior (e.g., + /// `--no-build-isolation`), this method will return an error. + pub(crate) fn validate(&self) -> Result<()> { + if self.allow_unsafe { + warn_user!( + "pip-compile's `--allow-unsafe` has no effect (Puffin can safely pin `pip` and other packages)." + ); + } + + if self.no_allow_unsafe { + warn_user!("pip-compile's `--no-allow-unsafe` has no effect (Puffin can safely pin `pip` and other packages)."); + } + + if self.reuse_hashes { + return Err(anyhow!( + "pip-compile's `--reuse-hashes` is unsupported (Puffin doesn't reuse hashes)." + )); + } + + if self.no_reuse_hashes { + warn_user!( + "pip-compile's `--no-reuse-hashes` has no effect (Puffin doesn't reuse hashes)." + ); + } + + if self.build_isolation { + warn_user!("pip-compile's `--build-isolation` has no effect (Puffin always uses build isolation)."); + } + + if self.no_build_isolation { + return Err(anyhow!( + "pip-compile's `--no-build-isolation` is unsupported (Puffin always uses build isolation)." + )); + } + + if let Some(resolver) = self.resolver { + match resolver { + Resolver::Backtracking => { + warn_user!( + "pip-compile's `--resolver=backtracking` has no effect (Puffin always backtracks)." + ); + } + Resolver::Legacy => { + return Err(anyhow!( + "pip-compile's `--resolver=legacy` is unsupported (Puffin always backtracks)." + )); + } + } + } + + if let Some(annotation_style) = self.annotation_style { + match annotation_style { + AnnotationStyle::Split => { + warn_user!( + "pip-compile's `--annotation-style=split` has no effect (Puffin always emits split annotations)." + ); + } + AnnotationStyle::Line => { + return Err(anyhow!( + "pip-compile's `--annotation-style=line` is unsupported (Puffin always emits split annotations)." + )); + } + } + } + + if self.max_rounds.is_some() { + return Err(anyhow!( + "pip-compile's `--max-rounds` is unsupported (Puffin always resolves until convergence)." + )); + } + + if self.client_cert.is_some() { + return Err(anyhow!( + "pip-compile's `--client-cert` is unsupported (Puffin doesn't support dedicated client certificates)." + )); + } + + if self.trusted_host.is_some() { + return Err(anyhow!( + "pip-compile's `--trusted-host` is unsupported (Puffin always requires HTTPS)." + )); + } + + if self.emit_trusted_host { + return Err(anyhow!( + "pip-compile's `--emit-trusted-host` is unsupported (Puffin always requires HTTPS)." + )); + } + + if self.no_emit_trusted_host { + warn_user!( + "pip-compile's `--no-emit-trusted-host` has no effect (Puffin never emits trusted hosts)." + ); + } + + if !self.unsafe_package.is_empty() { + return Err(anyhow!( + "pip-compile's `--unsafe-package` is not supported." + )); + } + + if self.no_config { + warn_user!( + "pip-compile's `--no-config` has no effect (Puffin does not use a configuration file)." + ); + } + + if self.emit_index_url { + return Err(anyhow!( + "pip-compile's `--emit-index-url` is unsupported (Puffin never emits index URLs)." + )); + } + + if self.no_emit_index_url { + warn_user!( + "pip-compile's `--no-emit-index-url` has no effect (Puffin never emits index URLs)." + ); + } + + if self.emit_find_links { + return Err(anyhow!( + "pip-compile's `--emit-find-links` is unsupported (Puffin never emits `--find-links` URLs)." + )); + } + + if self.no_emit_find_links { + warn_user!( + "pip-compile's `--no-emit-find-links` has no effect (Puffin never emits `--find-links` URLs)." + ); + } + + if self.emit_options { + return Err(anyhow!( + "pip-compile's `--emit-options` is unsupported (Puffin never emits options)." + )); + } + + if self.no_emit_options { + warn_user!( + "pip-compile's `--no-emit-options` has no effect (Puffin never emits options)." + ); + } + + if self.strip_extras { + warn_user!( + "pip-compile's `--strip-extras` has no effect (Puffin always strips extras)." + ); + } + + if self.no_strip_extras { + return Err(anyhow!( + "pip-compile's `--no-strip-extras` is unsupported (Puffin always strips extras)." + )); + } + + Ok(()) + } +} + +#[derive(Debug, Copy, Clone, ValueEnum)] +enum Resolver { + Backtracking, + Legacy, +} + +#[derive(Debug, Copy, Clone, ValueEnum)] +enum AnnotationStyle { + Line, + Split, +} diff --git a/crates/puffin/src/main.rs b/crates/puffin/src/main.rs index 93adae7b6..787e2b5ad 100644 --- a/crates/puffin/src/main.rs +++ b/crates/puffin/src/main.rs @@ -38,6 +38,7 @@ static GLOBAL: mimalloc::MiMalloc = mimalloc::MiMalloc; static GLOBAL: tikv_jemallocator::Jemalloc = tikv_jemallocator::Jemalloc; mod commands; +mod compat; mod logging; mod printer; mod requirements; @@ -282,6 +283,9 @@ struct PipCompileArgs { /// day, i.e. until midnight UTC that day. #[arg(long, value_parser = date_or_datetime)] exclude_newer: Option>, + + #[command(flatten)] + compat_args: compat::PipCompileCompatArgs, } #[derive(Args)] @@ -647,6 +651,8 @@ async fn inner() -> Result { Commands::Pip(PipArgs { command: PipCommand::Compile(args), }) => { + args.compat_args.validate()?; + let cache = cache.with_refresh(Refresh::from_args(args.refresh, args.refresh_package)); let requirements = args .src_file diff --git a/crates/puffin/tests/pip_compile.rs b/crates/puffin/tests/pip_compile.rs index 8cc2e5246..32d78da39 100644 --- a/crates/puffin/tests/pip_compile.rs +++ b/crates/puffin/tests/pip_compile.rs @@ -3789,3 +3789,81 @@ fn no_header() -> Result<()> { Ok(()) } + +/// Emit warnings when users pass redundant options from `pip-compile`. +#[test] +fn allow_unsafe() -> Result<()> { + let temp_dir = TempDir::new()?; + let cache_dir = TempDir::new()?; + let venv = create_venv_py312(&temp_dir, &cache_dir); + + let requirements_in = temp_dir.child("requirements.in"); + requirements_in.write_str("werkzeug==3.0.1")?; + + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip") + .arg("compile") + .arg("requirements.in") + .arg("--allow-unsafe") + .arg("--cache-dir") + .arg(cache_dir.path()) + .arg("--exclude-newer") + .arg(EXCLUDE_NEWER) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir), @r###" + success: true + exit_code: 0 + ----- stdout ----- + # This file was autogenerated by Puffin v[VERSION] via the following command: + # puffin pip compile requirements.in --allow-unsafe --cache-dir [CACHE_DIR] + markupsafe==2.1.3 + # via werkzeug + werkzeug==3.0.1 + + ----- stderr ----- + warning: pip-compile's `--allow-unsafe` has no effect (Puffin can safely pin `pip` and other packages). + Resolved 2 packages in [TIME] + "###); + }); + + Ok(()) +} + +/// Emit warnings when users pass redundant options from `pip-compile`. +#[test] +fn resolver_legacy() -> Result<()> { + let temp_dir = TempDir::new()?; + let cache_dir = TempDir::new()?; + let venv = create_venv_py312(&temp_dir, &cache_dir); + + let requirements_in = temp_dir.child("requirements.in"); + requirements_in.write_str("werkzeug==3.0.1")?; + + insta::with_settings!({ + filters => INSTA_FILTERS.to_vec() + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip") + .arg("compile") + .arg("requirements.in") + .arg("--resolver=legacy") + .arg("--cache-dir") + .arg(cache_dir.path()) + .arg("--exclude-newer") + .arg(EXCLUDE_NEWER) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: pip-compile's `--resolver=legacy` is unsupported (Puffin always backtracks). + "###); + }); + + Ok(()) +}