From 305b9b080a6c942ddd3baf424f0438d0c0a683cd Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 15 Dec 2023 13:43:23 -0500 Subject: [PATCH] Show resolution error once on pip-install failure (#665) Closes https://github.com/astral-sh/puffin/issues/664. --- crates/puffin-cli/src/commands/pip_install.rs | 74 +++++++++++-------- crates/puffin-cli/tests/pip_install.rs | 32 ++++++++ 2 files changed, 75 insertions(+), 31 deletions(-) diff --git a/crates/puffin-cli/src/commands/pip_install.rs b/crates/puffin-cli/src/commands/pip_install.rs index 4b9646475..68b0aab3e 100644 --- a/crates/puffin-cli/src/commands/pip_install.rs +++ b/crates/puffin-cli/src/commands/pip_install.rs @@ -86,7 +86,7 @@ pub(crate) async fn pip_install( } // Resolve the requirements. - let resolution = resolve( + let resolution = match resolve( spec, reinstall, resolution_mode, @@ -98,8 +98,20 @@ pub(crate) async fn pip_install( &venv, printer, ) - .await? - .into(); + .await + { + Ok(resolution) => Resolution::from(resolution), + Err(Error::Resolve(puffin_resolver::ResolveError::NoSolution(err))) => { + #[allow(clippy::print_stderr)] + { + let report = miette::Report::msg(format!("{err}")) + .context("No solution found when resolving dependencies:"); + eprint!("{report:?}"); + } + return Ok(ExitStatus::Failure); + } + Err(err) => return Err(err.into()), + }; // Sync the environment. install( @@ -126,16 +138,14 @@ fn specification( constraints: &[RequirementsSource], overrides: &[RequirementsSource], extras: &ExtrasSpecification<'_>, -) -> Result { +) -> Result { // If the user requests `extras` but does not provide a pyproject toml source if !matches!(extras, ExtrasSpecification::None) && !requirements .iter() .any(|source| matches!(source, RequirementsSource::PyprojectToml(_))) { - return Err(anyhow!( - "Requesting extras requires a pyproject.toml input file." - )); + return Err(anyhow!("Requesting extras requires a pyproject.toml input file.").into()); } // Read all requirements from the provided sources. @@ -155,7 +165,8 @@ fn specification( return Err(anyhow!( "Requested extra{s} not found: {}", unused_extras.iter().join(", ") - )); + ) + .into()); } } @@ -163,8 +174,8 @@ fn specification( } /// Returns `true` if the requirements are already satisfied. -fn satisfied(spec: &RequirementsSpecification, venv: &Virtualenv) -> Result { - SitePackages::from_executable(venv)?.satisfies(&spec.requirements, &spec.constraints) +fn satisfied(spec: &RequirementsSpecification, venv: &Virtualenv) -> Result { + Ok(SitePackages::from_executable(venv)?.satisfies(&spec.requirements, &spec.constraints)?) } /// Resolve a set of requirements, similar to running `pip-compile`. @@ -180,7 +191,7 @@ async fn resolve( cache: &Cache, venv: &Virtualenv, mut printer: Printer, -) -> Result { +) -> Result { let start = std::time::Instant::now(); // Create a manifest of the requirements. @@ -207,12 +218,6 @@ async fn resolve( let manifest = Manifest::new(requirements, constraints, overrides, preferences, project); let options = ResolutionOptions::new(resolution_mode, prerelease_mode, exclude_newer); - debug!( - "Using Python {} at {}", - venv.interpreter().markers().python_version, - venv.python_executable().display() - ); - // Determine the compatible platform tags. let tags = Tags::from_interpreter(venv.interpreter())?; @@ -240,18 +245,7 @@ async fn resolve( // Resolve the dependencies. let resolver = Resolver::new(manifest, options, markers, &tags, &client, &build_dispatch) .with_reporter(ResolverReporter::from(printer)); - let resolution = match resolver.resolve().await { - Err(puffin_resolver::ResolveError::NoSolution(err)) => { - #[allow(clippy::print_stderr)] - { - let report = miette::Report::msg(format!("{err}")) - .context("No solution found when resolving dependencies:"); - eprint!("{report:?}"); - } - return Err(puffin_resolver::ResolveError::NoSolution(err).into()); - } - result => result, - }?; + let resolution = resolver.resolve().await?; let s = if resolution.len() == 1 { "" } else { "s" }; writeln!( @@ -279,7 +273,7 @@ async fn install( cache: &Cache, venv: &Virtualenv, mut printer: Printer, -) -> Result<()> { +) -> Result<(), Error> { let start = std::time::Instant::now(); // Determine the current environment markers. @@ -455,7 +449,7 @@ async fn install( } /// Validate the installed packages in the virtual environment. -fn validate(resolution: &Resolution, venv: &Virtualenv, mut printer: Printer) -> Result<()> { +fn validate(resolution: &Resolution, venv: &Virtualenv, mut printer: Printer) -> Result<(), Error> { let site_packages = SitePackages::from_executable(venv)?; let diagnostics = site_packages.diagnostics()?; for diagnostic in diagnostics { @@ -475,3 +469,21 @@ fn validate(resolution: &Resolution, venv: &Virtualenv, mut printer: Printer) -> } Ok(()) } + +#[derive(thiserror::Error, Debug)] +enum Error { + #[error(transparent)] + Resolve(#[from] puffin_resolver::ResolveError), + + #[error(transparent)] + Platform(#[from] platform_host::PlatformError), + + #[error(transparent)] + Io(#[from] std::io::Error), + + #[error(transparent)] + Fmt(#[from] std::fmt::Error), + + #[error(transparent)] + Anyhow(#[from] anyhow::Error), +} diff --git a/crates/puffin-cli/tests/pip_install.rs b/crates/puffin-cli/tests/pip_install.rs index 6b3fe3092..7d42b5d9c 100644 --- a/crates/puffin-cli/tests/pip_install.rs +++ b/crates/puffin-cli/tests/pip_install.rs @@ -52,6 +52,38 @@ fn missing_requirements_txt() -> Result<()> { Ok(()) } +#[test] +fn no_solution() -> Result<()> { + let temp_dir = assert_fs::TempDir::new()?; + let cache_dir = assert_fs::TempDir::new()?; + let venv = create_venv_py312(&temp_dir, &cache_dir); + + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .arg("pip-install") + .arg("flask>=3.0.0") + .arg("WerkZeug<1.0.0") + .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: 1 + ----- stdout ----- + + ----- stderr ----- + × No solution found when resolving dependencies: + ╰─▶ Because flask==3.0.0 depends on werkzeug>=3.0.0 and there is no + version of flask available matching >3.0.0, flask>=3.0.0 depends on + werkzeug>=3.0.0. + And because root depends on werkzeug<1.0.0 and root depends on + flask>=3.0.0, version solving failed. + "###); + + Ok(()) +} + /// Install a package from the command line into a virtual environment. #[test] fn install_package() -> Result<()> {