From 67e3e458393a43cfc1846f4d959f7dac7171397a Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Wed, 1 Nov 2023 13:39:49 -0500 Subject: [PATCH] Add support for `--all-extras` to `pip-compile` (#259) Closes #244 Notable decision to error if `--all-extra` and `--extra ` are both provided. --- crates/puffin-cli/src/commands/pip_compile.rs | 31 ++--- crates/puffin-cli/src/commands/pip_sync.rs | 4 +- .../puffin-cli/src/commands/pip_uninstall.rs | 4 +- crates/puffin-cli/src/main.rs | 18 ++- crates/puffin-cli/src/requirements.rs | 39 ++++-- crates/puffin-cli/tests/pip_compile.rs | 124 ++++++++++++++++++ ...le__compile_pyproject_toml_all_extras.snap | 40 ++++++ 7 files changed, 230 insertions(+), 30 deletions(-) create mode 100644 crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_all_extras.snap diff --git a/crates/puffin-cli/src/commands/pip_compile.rs b/crates/puffin-cli/src/commands/pip_compile.rs index 0193216b3..1f497d717 100644 --- a/crates/puffin-cli/src/commands/pip_compile.rs +++ b/crates/puffin-cli/src/commands/pip_compile.rs @@ -8,7 +8,6 @@ use colored::Colorize; use fs_err::File; use itertools::Itertools; use pubgrub::report::Reporter; -use puffin_package::extra_name::ExtraName; use tracing::debug; use pep508_rs::Requirement; @@ -23,7 +22,7 @@ use crate::commands::reporters::ResolverReporter; use crate::commands::{elapsed, ExitStatus}; use crate::index_urls::IndexUrls; use crate::printer::Printer; -use crate::requirements::{RequirementsSource, RequirementsSpecification}; +use crate::requirements::{ExtrasSpecification, RequirementsSource, RequirementsSpecification}; const VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -32,7 +31,7 @@ const VERSION: &str = env!("CARGO_PKG_VERSION"); pub(crate) async fn pip_compile( requirements: &[RequirementsSource], constraints: &[RequirementsSource], - extras: Vec, + extras: ExtrasSpecification<'_>, output_file: Option<&Path>, resolution_mode: ResolutionMode, prerelease_mode: PreReleaseMode, @@ -51,18 +50,20 @@ pub(crate) async fn pip_compile( } = RequirementsSpecification::try_from_sources(requirements, constraints, &extras)?; // Check that all provided extras are used - let mut unused_extras = extras - .iter() - .filter(|extra| !used_extras.contains(extra)) - .collect::>(); - if !unused_extras.is_empty() { - unused_extras.sort_unstable(); - unused_extras.dedup(); - let s = if unused_extras.len() == 1 { "" } else { "s" }; - return Err(anyhow!( - "Requested extra{s} not found: {}", - unused_extras.iter().join(", ") - )); + if let ExtrasSpecification::Some(extras) = extras { + let mut unused_extras = extras + .iter() + .filter(|extra| !used_extras.contains(extra)) + .collect::>(); + if !unused_extras.is_empty() { + unused_extras.sort_unstable(); + unused_extras.dedup(); + let s = if unused_extras.len() == 1 { "" } else { "s" }; + return Err(anyhow!( + "Requested extra{s} not found: {}", + unused_extras.iter().join(", ") + )); + } } let preferences: Vec = output_file diff --git a/crates/puffin-cli/src/commands/pip_sync.rs b/crates/puffin-cli/src/commands/pip_sync.rs index 4f4b45b69..b11f395c6 100644 --- a/crates/puffin-cli/src/commands/pip_sync.rs +++ b/crates/puffin-cli/src/commands/pip_sync.rs @@ -21,7 +21,7 @@ use crate::commands::reporters::{ use crate::commands::{elapsed, ExitStatus}; use crate::index_urls::IndexUrls; use crate::printer::Printer; -use crate::requirements::{RequirementsSource, RequirementsSpecification}; +use crate::requirements::{ExtrasSpecification, RequirementsSource, RequirementsSpecification}; /// Install a set of locked requirements into the current Python environment. pub(crate) async fn pip_sync( @@ -36,7 +36,7 @@ pub(crate) async fn pip_sync( requirements, constraints: _, extras: _, - } = RequirementsSpecification::try_from_sources(sources, &[], &[])?; + } = RequirementsSpecification::try_from_sources(sources, &[], &ExtrasSpecification::None)?; if requirements.is_empty() { writeln!(printer, "No requirements found")?; diff --git a/crates/puffin-cli/src/commands/pip_uninstall.rs b/crates/puffin-cli/src/commands/pip_uninstall.rs index 6abee8e28..323610b87 100644 --- a/crates/puffin-cli/src/commands/pip_uninstall.rs +++ b/crates/puffin-cli/src/commands/pip_uninstall.rs @@ -11,7 +11,7 @@ use puffin_package::package_name::PackageName; use crate::commands::{elapsed, ExitStatus}; use crate::printer::Printer; -use crate::requirements::{RequirementsSource, RequirementsSpecification}; +use crate::requirements::{ExtrasSpecification, RequirementsSource, RequirementsSpecification}; /// Uninstall packages from the current environment. pub(crate) async fn pip_uninstall( @@ -26,7 +26,7 @@ pub(crate) async fn pip_uninstall( requirements, constraints: _, extras: _, - } = RequirementsSpecification::try_from_sources(sources, &[], &[])?; + } = RequirementsSpecification::try_from_sources(sources, &[], &ExtrasSpecification::None)?; // Detect the current Python interpreter. let platform = Platform::current()?; diff --git a/crates/puffin-cli/src/main.rs b/crates/puffin-cli/src/main.rs index 08d01fa46..12f19bdda 100644 --- a/crates/puffin-cli/src/main.rs +++ b/crates/puffin-cli/src/main.rs @@ -6,6 +6,7 @@ use colored::Colorize; use directories::ProjectDirs; use puffin_package::extra_name::ExtraName; use puffin_resolver::{PreReleaseMode, ResolutionMode}; +use requirements::ExtrasSpecification; use url::Url; use crate::commands::ExitStatus; @@ -73,9 +74,13 @@ struct PipCompileArgs { constraint: Vec, /// Include optional dependencies in the given extra group name; may be provided more than once. - #[clap(long)] + #[clap(long, conflicts_with = "all_extras")] extra: Vec, + /// Include all optional dependencies. + #[clap(long, conflicts_with = "extra")] + all_extras: bool, + #[clap(long, value_enum)] resolution: Option, @@ -204,10 +209,19 @@ async fn main() -> ExitCode { .collect::>(); let index_urls = IndexUrls::from_args(args.index_url, args.extra_index_url, args.no_index); + + let extras = if args.all_extras { + ExtrasSpecification::All + } else if args.extra.is_empty() { + ExtrasSpecification::None + } else { + ExtrasSpecification::Some(&args.extra) + }; + commands::pip_compile( &requirements, &constraints, - args.extra, + extras, args.output_file.as_deref(), args.resolution.unwrap_or_default(), args.prerelease.unwrap_or_default(), diff --git a/crates/puffin-cli/src/requirements.rs b/crates/puffin-cli/src/requirements.rs index 31ba53d5d..0776f87d0 100644 --- a/crates/puffin-cli/src/requirements.rs +++ b/crates/puffin-cli/src/requirements.rs @@ -37,6 +37,25 @@ impl From for RequirementsSource { } } +#[derive(Debug, Default)] +pub(crate) enum ExtrasSpecification<'a> { + #[default] + None, + All, + Some(&'a [ExtraName]), +} + +impl ExtrasSpecification<'_> { + /// Returns true if a name is included in the extra specification. + fn contains(&self, name: &ExtraName) -> bool { + match self { + ExtrasSpecification::All => true, + ExtrasSpecification::None => false, + ExtrasSpecification::Some(extras) => extras.contains(name), + } + } +} + #[derive(Debug, Default)] pub(crate) struct RequirementsSpecification { /// The requirements for the project. @@ -51,7 +70,7 @@ impl RequirementsSpecification { /// Read the requirements and constraints from a source. pub(crate) fn try_from_source( source: &RequirementsSource, - extras: &[ExtraName], + extras: &ExtrasSpecification, ) -> Result { Ok(match source { RequirementsSource::Name(name) => { @@ -84,13 +103,15 @@ impl RequirementsSpecification { if let Some(project) = pyproject_toml.project { requirements.extend(project.dependencies.unwrap_or_default()); // Include any optional dependencies specified in `extras` - for (name, optional_requirements) in - project.optional_dependencies.unwrap_or_default() - { - let normalized_name = ExtraName::normalize(name); - if extras.contains(&normalized_name) { - used_extras.insert(normalized_name); - requirements.extend(optional_requirements); + if !matches!(extras, ExtrasSpecification::None) { + for (name, optional_requirements) in + project.optional_dependencies.unwrap_or_default() + { + let normalized_name = ExtraName::normalize(name); + if extras.contains(&normalized_name) { + used_extras.insert(normalized_name); + requirements.extend(optional_requirements); + } } } } @@ -108,7 +129,7 @@ impl RequirementsSpecification { pub(crate) fn try_from_sources( requirements: &[RequirementsSource], constraints: &[RequirementsSource], - extras: &[ExtraName], + extras: &ExtrasSpecification, ) -> Result { let mut spec = Self::default(); diff --git a/crates/puffin-cli/tests/pip_compile.rs b/crates/puffin-cli/tests/pip_compile.rs index 545130f32..5de31af8f 100644 --- a/crates/puffin-cli/tests/pip_compile.rs +++ b/crates/puffin-cli/tests/pip_compile.rs @@ -701,3 +701,127 @@ fn conflicting_transitive_url_dependency() -> Result<()> { Ok(()) } + +/// Resolve packages from all optional dependency groups in a `pyproject.toml` file. +#[test] +fn compile_pyproject_toml_all_extras() -> Result<()> { + let temp_dir = assert_fs::TempDir::new()?; + let cache_dir = assert_fs::TempDir::new()?; + let venv = temp_dir.child(".venv"); + + Command::new(get_cargo_bin(BIN_NAME)) + .arg("venv") + .arg(venv.as_os_str()) + .arg("--cache-dir") + .arg(cache_dir.path()) + .current_dir(&temp_dir) + .assert() + .success(); + venv.assert(predicates::path::is_dir()); + + let pyproject_toml = temp_dir.child("pyproject.toml"); + pyproject_toml.touch()?; + pyproject_toml.write_str( + r#"[build-system] +requires = ["setuptools", "wheel"] + +[project] +name = "project" +dependencies = ["django==5.0b1"] +optional-dependencies.foo = [ + "anyio==4.0.0", +] +optional-dependencies.bar = [ + "httpcore==0.18.0", +] +"#, + )?; + + insta::with_settings!({ + filters => vec![ + (r"\d+(ms|s)", "[TIME]"), + (r"# .* pip-compile", "# [BIN_PATH] pip-compile"), + (r"--cache-dir .*", "--cache-dir [CACHE_DIR]"), + ] + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip-compile") + .arg("pyproject.toml") + .arg("--all-extras") + .arg("--cache-dir") + .arg(cache_dir.path()) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir)); + }); + + Ok(()) +} + +/// Resolve packages from all optional dependency groups in a `pyproject.toml` file. +#[test] +fn compile_does_not_allow_both_extra_and_all_extras() -> Result<()> { + let temp_dir = assert_fs::TempDir::new()?; + let cache_dir = assert_fs::TempDir::new()?; + let venv = temp_dir.child(".venv"); + + Command::new(get_cargo_bin(BIN_NAME)) + .arg("venv") + .arg(venv.as_os_str()) + .arg("--cache-dir") + .arg(cache_dir.path()) + .current_dir(&temp_dir) + .assert() + .success(); + venv.assert(predicates::path::is_dir()); + + let pyproject_toml = temp_dir.child("pyproject.toml"); + pyproject_toml.touch()?; + pyproject_toml.write_str( + r#"[build-system] +requires = ["setuptools", "wheel"] + +[project] +name = "project" +dependencies = ["django==5.0b1"] +optional-dependencies.foo = [ + "anyio==4.0.0", +] +optional-dependencies.bar = [ + "httpcore==0.18.0", +] +"#, + )?; + + insta::with_settings!({ + filters => vec![ + (r"\d+(ms|s)", "[TIME]"), + (r"# .* pip-compile", "# [BIN_PATH] pip-compile"), + (r"--cache-dir .*", "--cache-dir [CACHE_DIR]"), + ] + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip-compile") + .arg("pyproject.toml") + .arg("--all-extras") + .arg("--extra") + .arg("foo") + .arg("--cache-dir") + .arg(cache_dir.path()) + .env("VIRTUAL_ENV", venv.as_os_str()) + .current_dir(&temp_dir), + @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: the argument '--all-extras' cannot be used with '--extra ' + + Usage: puffin pip-compile --all-extras --cache-dir [CACHE_DIR] + + For more information, try '--help'. + "###); + }); + + Ok(()) +} diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_all_extras.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_all_extras.snap new file mode 100644 index 000000000..b018f308d --- /dev/null +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_pyproject_toml_all_extras.snap @@ -0,0 +1,40 @@ +--- +source: crates/puffin-cli/tests/pip_compile.rs +info: + program: puffin + args: + - pip-compile + - pyproject.toml + - "--all-extras" + - "--cache-dir" + - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpw8DJ9R + env: + VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmppqOrk1/.venv +--- +success: true +exit_code: 0 +----- stdout ----- +# This file was autogenerated by Puffin v0.0.1 via the following command: +# [BIN_PATH] pip-compile pyproject.toml --all-extras --cache-dir [CACHE_DIR] +anyio==4.0.0 + # via httpcore +asgiref==3.7.2 + # via django +certifi==2023.7.22 + # via httpcore +django==5.0b1 +h11==0.14.0 + # via httpcore +httpcore==0.18.0 +idna==3.4 + # via anyio +sniffio==1.3.0 + # via + # anyio + # httpcore +sqlparse==0.4.4 + # via django + +----- stderr ----- +Resolved 9 packages in [TIME] +