From 8e4f468e6ce062c68bd393f9be8c28535ddd6a41 Mon Sep 17 00:00:00 2001 From: Zanie Date: Tue, 14 Nov 2023 10:48:23 -0600 Subject: [PATCH 1/3] Add an `UnusableDependencies` incompatibility kind and use for conflicting versions --- crates/puffin-cli/tests/pip_compile.rs | 18 ++++++----- ...pile__compile_unsolvable_requirements.snap | 7 ++-- crates/puffin-resolver/src/error.rs | 3 ++ .../src/pubgrub/dependencies.rs | 18 ++++++++--- crates/puffin-resolver/src/pubgrub/report.rs | 25 +++++++++++++++ crates/puffin-resolver/src/resolver.rs | 16 +++++++++- .../pubgrub/src/internal/incompatibility.rs | 16 ++++++++++ vendor/pubgrub/src/range.rs | 4 +++ vendor/pubgrub/src/report.rs | 32 +++++++++++++++++++ 9 files changed, 123 insertions(+), 16 deletions(-) diff --git a/crates/puffin-cli/tests/pip_compile.rs b/crates/puffin-cli/tests/pip_compile.rs index 77ddf57c9c98..95915a0d2df7 100644 --- a/crates/puffin-cli/tests/pip_compile.rs +++ b/crates/puffin-cli/tests/pip_compile.rs @@ -666,14 +666,16 @@ fn compile_numpy_py38() -> Result<()> { .arg("--no-build") .env("VIRTUAL_ENV", venv.as_os_str()) .current_dir(&temp_dir), @r###" - success: false - exit_code: 2 - ----- stdout ----- - - ----- stderr ----- - error: Failed to build distribution: numpy-1.24.4.tar.gz - Caused by: Building source distributions is disabled - "###); + success: true + exit_code: 0 + ----- stdout ----- + # This file was autogenerated by Puffin v0.0.1 via the following command: + # [BIN_PATH] pip-compile requirements.in --python-version py38 --cache-dir [CACHE_DIR] + numpy==1.24.4 + + ----- stderr ----- + Resolved 1 package in [TIME] + "###); }); Ok(()) diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements.snap index 61a6e7f1e855..f4320bd8fc09 100644 --- a/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements.snap +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements.snap @@ -6,9 +6,9 @@ info: - pip-compile - pyproject.toml - "--cache-dir" - - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpN531dN + - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpvGljPp env: - VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp99w9dK/.venv + VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp2rBXR2/.venv --- success: false exit_code: 1 @@ -16,5 +16,6 @@ exit_code: 1 ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ my-project depends on django∅ + ╰─▶ my-project dependencies are unusable: `django==5.0b1` does not intersect + with `django==5.0a1` diff --git a/crates/puffin-resolver/src/error.rs b/crates/puffin-resolver/src/error.rs index 48089c96375c..8351dd165d23 100644 --- a/crates/puffin-resolver/src/error.rs +++ b/crates/puffin-resolver/src/error.rs @@ -44,6 +44,9 @@ pub enum ResolveError { #[error("Conflicting URLs for package `{0}`: {1} and {2}")] ConflictingUrls(PackageName, String, String), + #[error("`{0}{1}` does not intersect with `{0}{2}`")] + ConflictingVersions(PubGrubPackage, Range, Range), + #[error("Package `{0}` attempted to resolve via URL: {1}. URL dependencies must be expressed as direct requirements or constraints. Consider adding `{0} @ {1}` to your dependencies or constraints file.")] DisallowedUrl(PackageName, Url), diff --git a/crates/puffin-resolver/src/pubgrub/dependencies.rs b/crates/puffin-resolver/src/pubgrub/dependencies.rs index f59247384958..955e1034dac9 100644 --- a/crates/puffin-resolver/src/pubgrub/dependencies.rs +++ b/crates/puffin-resolver/src/pubgrub/dependencies.rs @@ -59,7 +59,7 @@ impl PubGrubDependencies { if let Some(entry) = dependencies.get_key_value(&package) { // Merge the versions. - let version = merge_versions(entry.1, &version); + let version = merge_versions(&package, entry.1, &version)?; // Merge the package. if let Some(package) = merge_package(entry.0, &package)? { @@ -107,7 +107,7 @@ impl PubGrubDependencies { if let Some(entry) = dependencies.get_key_value(&package) { // Merge the versions. - let version = merge_versions(entry.1, &version); + let version = merge_versions(&package, entry.1, &version)?; // Merge the package. if let Some(package) = merge_package(entry.0, &package)? { @@ -178,10 +178,20 @@ fn to_pubgrub( /// Merge two [`PubGrubVersion`] ranges. fn merge_versions( + package: &PubGrubPackage, left: &Range, right: &Range, -) -> Range { - left.intersection(right) +) -> Result, ResolveError> { + let result = left.intersection(right); + if result.is_empty() { + Err(ResolveError::ConflictingVersions( + package.clone(), + left.clone(), + right.clone(), + )) + } else { + Ok(result) + } } /// Merge two [`PubGrubPackage`] instances. diff --git a/crates/puffin-resolver/src/pubgrub/report.rs b/crates/puffin-resolver/src/pubgrub/report.rs index 3565205a23d8..15521d6dd074 100644 --- a/crates/puffin-resolver/src/pubgrub/report.rs +++ b/crates/puffin-resolver/src/pubgrub/report.rs @@ -1,5 +1,6 @@ use std::fmt; +use pubgrub::package::Package; use pubgrub::range::Range; use pubgrub::report::{DerivationTree, Derived, External, Reporter}; use pubgrub::term::Term; @@ -345,6 +346,8 @@ enum PuffinExternal { NoVersions(PubGrubPackage, Range), /// Dependencies of the package are unavailable for versions in that set. UnavailableDependencies(PubGrubPackage, Range), + /// Dependencies of the package are unusable for versions in that set. + UnusableDependencies(PubGrubPackage, Range, Option), /// Incompatibility coming from the dependencies of a given package. FromDependencyOf( PubGrubPackage, @@ -362,6 +365,9 @@ impl PuffinExternal { External::UnavailableDependencies(p, vs) => { PuffinExternal::UnavailableDependencies(p, vs) } + External::UnusableDependencies(p, vs, reason) => { + PuffinExternal::UnusableDependencies(p, vs, reason) + } External::FromDependencyOf(p, vs, p_dep, vs_dep) => { PuffinExternal::FromDependencyOf(p, vs, p_dep, vs_dep) } @@ -395,6 +401,25 @@ impl fmt::Display for PuffinExternal { ) } } + Self::UnusableDependencies(package, set, reason) => { + if let Some(reason) = reason { + if matches!(package, PubGrubPackage::Root(_)) { + write!(f, "{package} dependencies are unusable: {reason}") + } else { + if set == &Range::full() { + write!(f, "dependencies of {package} are unusable: {reason}") + } else { + write!(f, "dependencies of {package}{set} are unusable: {reason}",) + } + } + } else { + if set == &Range::full() { + write!(f, "dependencies of {package} are unusable") + } else { + write!(f, "dependencies of {package}{set} are unusable") + } + } + } Self::FromDependencyOf(package, package_set, dependency, dependency_set) => { if package_set == &Range::full() && dependency_set == &Range::full() { write!(f, "{package} depends on {dependency}") diff --git a/crates/puffin-resolver/src/resolver.rs b/crates/puffin-resolver/src/resolver.rs index 544777bc8c92..eb17d5b83373 100644 --- a/crates/puffin-resolver/src/resolver.rs +++ b/crates/puffin-resolver/src/resolver.rs @@ -239,6 +239,14 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> { )); continue; } + Dependencies::Unusable(reason) => { + state.add_incompatibility(Incompatibility::unusable_dependencies( + package.clone(), + version.clone(), + reason.clone(), + )); + continue; + } Dependencies::Known(constraints) if constraints.contains_key(package) => { return Err(PubGrubError::SelfDependency { package: package.clone(), @@ -455,7 +463,11 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> { None, None, self.markers, - )?; + ); + if let Err(err @ ResolveError::ConflictingVersions(..)) = constraints { + return Ok(Dependencies::Unusable(Some(format!("{err}")))); + } + let constraints = constraints?; for (package, version) in constraints.iter() { debug!("Adding direct dependency: {package}{version}"); @@ -910,6 +922,8 @@ impl<'a> FromIterator<&'a Url> for AllowedUrls { enum Dependencies { /// Package dependencies are unavailable. Unknown, + /// Package dependencies are not usable + Unusable(Option), /// Container for all available package versions. Known(DependencyConstraints>), } diff --git a/vendor/pubgrub/src/internal/incompatibility.rs b/vendor/pubgrub/src/internal/incompatibility.rs index 168c5218c1ab..8de990c25fc7 100644 --- a/vendor/pubgrub/src/internal/incompatibility.rs +++ b/vendor/pubgrub/src/internal/incompatibility.rs @@ -45,6 +45,8 @@ pub enum Kind { NoVersions(P, VS), /// Dependencies of the package are unavailable for versions in that range. UnavailableDependencies(P, VS), + /// Dependencies of the package are unusable for versions in that range. + UnusableDependencies(P, VS, Option), /// Incompatibility coming from the dependencies of a given package. FromDependencyOf(P, VS, P, VS), /// Derived from two causes. Stores cause ids. @@ -104,6 +106,17 @@ impl Incompatibility { } } + /// Create an incompatibility to remember + /// that a package version is not selectable + /// because its dependencies are not usable. + pub fn unusable_dependencies(package: P, version: VS::V, reason: Option) -> Self { + let set = VS::singleton(version); + Self { + package_terms: SmallMap::One([(package.clone(), Term::Positive(set.clone()))]), + kind: Kind::UnusableDependencies(package, set, reason), + } + } + /// Build an incompatibility from a given dependency. pub fn from_dependency(package: P, version: VS::V, dep: (&P, &VS)) -> Self { let set1 = VS::singleton(version); @@ -206,6 +219,9 @@ impl Incompatibility { Kind::UnavailableDependencies(package, set) => DerivationTree::External( External::UnavailableDependencies(package.clone(), set.clone()), ), + Kind::UnusableDependencies(package, set, reason) => DerivationTree::External( + External::UnusableDependencies(package.clone(), set.clone(), reason.clone()), + ), Kind::FromDependencyOf(package, set, dep_package, dep_set) => { DerivationTree::External(External::FromDependencyOf( package.clone(), diff --git a/vendor/pubgrub/src/range.rs b/vendor/pubgrub/src/range.rs index 47e76e44f52b..9800c002cb65 100644 --- a/vendor/pubgrub/src/range.rs +++ b/vendor/pubgrub/src/range.rs @@ -117,6 +117,10 @@ impl Range { segments: SmallVec::one((Included(v1.into()), Excluded(v2.into()))), } } + + pub fn is_empty(&self) -> bool { + self.segments.is_empty() + } } impl Range { diff --git a/vendor/pubgrub/src/report.rs b/vendor/pubgrub/src/report.rs index ff0b2d3f0513..57e862bde899 100644 --- a/vendor/pubgrub/src/report.rs +++ b/vendor/pubgrub/src/report.rs @@ -41,6 +41,8 @@ pub enum External { NoVersions(P, VS), /// Dependencies of the package are unavailable for versions in that set. UnavailableDependencies(P, VS), + /// Dependencies of the package are unusable for versions in that set. + UnusableDependencies(P, VS, Option), /// Incompatibility coming from the dependencies of a given package. FromDependencyOf(P, VS, P, VS), } @@ -113,6 +115,13 @@ impl DerivationTree { DerivationTree::External(External::UnavailableDependencies(_, r)) => Some( DerivationTree::External(External::UnavailableDependencies(package, set.union(&r))), ), + DerivationTree::External(External::UnusableDependencies(_, r, reason)) => { + Some(DerivationTree::External(External::UnusableDependencies( + package, + set.union(&r), + reason, + ))) + } DerivationTree::External(External::FromDependencyOf(p1, r1, p2, r2)) => { if p1 == package { Some(DerivationTree::External(External::FromDependencyOf( @@ -158,6 +167,29 @@ impl fmt::Display for External { ) } } + Self::UnusableDependencies(package, set, reason) => { + if let Some(reason) = reason { + if set == &VS::full() { + write!(f, "dependencies of {} are unusable: {reason}", package) + } else { + write!( + f, + "dependencies of {} at version {} are unusable: {reason}", + package, set + ) + } + } else { + if set == &VS::full() { + write!(f, "dependencies of {} are unusable", package) + } else { + write!( + f, + "dependencies of {} at version {} are unusable", + package, set + ) + } + } + } Self::FromDependencyOf(p, set_p, dep, set_dep) => { if set_p == &VS::full() && set_dep == &VS::full() { write!(f, "{} depends on {}", p, dep) From ada22260898f853ae0ba97d25147cc95b2ff190e Mon Sep 17 00:00:00 2001 From: Zanie Date: Tue, 14 Nov 2023 12:06:26 -0600 Subject: [PATCH 2/3] Reduce the size of the `ConflictingVersions` err variant --- .../pip_compile__compile_unsolvable_requirements.snap | 8 ++++---- crates/puffin-resolver/src/error.rs | 4 ++-- crates/puffin-resolver/src/pubgrub/dependencies.rs | 5 ++--- crates/puffin-resolver/src/pubgrub/report.rs | 1 - crates/puffin-resolver/src/resolver.rs | 2 +- 5 files changed, 9 insertions(+), 11 deletions(-) diff --git a/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements.snap b/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements.snap index f4320bd8fc09..db0d74a94fc2 100644 --- a/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements.snap +++ b/crates/puffin-cli/tests/snapshots/pip_compile__compile_unsolvable_requirements.snap @@ -6,9 +6,9 @@ info: - pip-compile - pyproject.toml - "--cache-dir" - - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpvGljPp + - /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpJ9Dkj3 env: - VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp2rBXR2/.venv + VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpBOyFGn/.venv --- success: false exit_code: 1 @@ -16,6 +16,6 @@ exit_code: 1 ----- stderr ----- × No solution found when resolving dependencies: - ╰─▶ my-project dependencies are unusable: `django==5.0b1` does not intersect - with `django==5.0a1` + ╰─▶ my-project dependencies are unusable: Conflicting versions for `django`: + `django==5.0b1` does not intersect with `django==5.0a1` diff --git a/crates/puffin-resolver/src/error.rs b/crates/puffin-resolver/src/error.rs index 8351dd165d23..59f0c6d75973 100644 --- a/crates/puffin-resolver/src/error.rs +++ b/crates/puffin-resolver/src/error.rs @@ -44,8 +44,8 @@ pub enum ResolveError { #[error("Conflicting URLs for package `{0}`: {1} and {2}")] ConflictingUrls(PackageName, String, String), - #[error("`{0}{1}` does not intersect with `{0}{2}`")] - ConflictingVersions(PubGrubPackage, Range, Range), + #[error("Conflicting versions for `{0}`: {1}")] + ConflictingVersions(String, String), #[error("Package `{0}` attempted to resolve via URL: {1}. URL dependencies must be expressed as direct requirements or constraints. Consider adding `{0} @ {1}` to your dependencies or constraints file.")] DisallowedUrl(PackageName, Url), diff --git a/crates/puffin-resolver/src/pubgrub/dependencies.rs b/crates/puffin-resolver/src/pubgrub/dependencies.rs index 955e1034dac9..e9fe2f110b6e 100644 --- a/crates/puffin-resolver/src/pubgrub/dependencies.rs +++ b/crates/puffin-resolver/src/pubgrub/dependencies.rs @@ -185,9 +185,8 @@ fn merge_versions( let result = left.intersection(right); if result.is_empty() { Err(ResolveError::ConflictingVersions( - package.clone(), - left.clone(), - right.clone(), + package.to_string(), + format!("`{package}{left}` does not intersect with `{package}{right}`"), )) } else { Ok(result) diff --git a/crates/puffin-resolver/src/pubgrub/report.rs b/crates/puffin-resolver/src/pubgrub/report.rs index 15521d6dd074..14f648916d03 100644 --- a/crates/puffin-resolver/src/pubgrub/report.rs +++ b/crates/puffin-resolver/src/pubgrub/report.rs @@ -1,6 +1,5 @@ use std::fmt; -use pubgrub::package::Package; use pubgrub::range::Range; use pubgrub::report::{DerivationTree, Derived, External, Reporter}; use pubgrub::term::Term; diff --git a/crates/puffin-resolver/src/resolver.rs b/crates/puffin-resolver/src/resolver.rs index eb17d5b83373..6b7abd393247 100644 --- a/crates/puffin-resolver/src/resolver.rs +++ b/crates/puffin-resolver/src/resolver.rs @@ -465,7 +465,7 @@ impl<'a, Context: BuildContext + Sync> Resolver<'a, Context> { self.markers, ); if let Err(err @ ResolveError::ConflictingVersions(..)) = constraints { - return Ok(Dependencies::Unusable(Some(format!("{err}")))); + return Ok(Dependencies::Unusable(Some(err.to_string()))); } let constraints = constraints?; From 5535a5417d49f87e04ab22fcc1c53af058a68010 Mon Sep 17 00:00:00 2001 From: Zanie Date: Thu, 16 Nov 2023 08:48:57 -0600 Subject: [PATCH 3/3] Regen snapshot --- crates/puffin-cli/tests/pip_compile.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/puffin-cli/tests/pip_compile.rs b/crates/puffin-cli/tests/pip_compile.rs index 89abc37f38e2..c662f8ab4315 100644 --- a/crates/puffin-cli/tests/pip_compile.rs +++ b/crates/puffin-cli/tests/pip_compile.rs @@ -542,7 +542,7 @@ fn compile_numpy_py38() -> Result<()> { exit_code: 0 ----- stdout ----- # This file was autogenerated by Puffin v0.0.1 via the following command: - # [BIN_PATH] pip-compile requirements.in --python-version py38 --cache-dir [CACHE_DIR] + # [BIN_PATH] pip-compile requirements.in --cache-dir [CACHE_DIR] numpy==1.24.4 ----- stderr -----