Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling of dependency conflicts resulting in empty version ranges #383

Closed
wants to merge 8 commits into from
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,17 @@ info:
- pip-compile
- requirements.in
- "--cache-dir"
- /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmp5R6H5M
- /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpmkb9tv
env:
VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpIdL1TH/.venv
VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpO0ihlM/.venv
---
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 --cache-dir [CACHE_DIR]
sqlparse==0.4.3

----- stderr -----
Resolved 0 packages in [TIME]
Resolved 1 package in [TIME]

Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ info:
- "--constraint"
- constraints.txt
- "--cache-dir"
- /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp1Ts53o
- /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp6Agh9o
env:
VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp1vzBQa/.venv
VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp2l1dM3/.venv
---
success: true
exit_code: 0
Expand All @@ -21,7 +21,9 @@ anyio==4.0.0
idna==3.4
# via anyio
sniffio==1.3.0
# via anyio
# via
# anyio
# idna

----- stderr -----
Resolved 3 packages in [TIME]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ info:
- "--constraint"
- constraints.txt
- "--cache-dir"
- /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpvjdHOb
- /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpVzXpee
env:
VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpSAaWi3/.venv
VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpYmxcVz/.venv
---
success: true
exit_code: 0
Expand All @@ -21,7 +21,9 @@ asgiref==3.7.2
# via django
django==5.0b1
sqlparse==0.4.3
# via django
# via
# asgiref
# django

----- stderr -----
Resolved 3 packages in [TIME]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@ info:
- pip-compile
- pyproject.toml
- "--cache-dir"
- /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpN531dN
- /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpRAI4mN
env:
VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmp99w9dK/.venv
VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpvNCp98/.venv
---
success: false
exit_code: 1
----- stdout -----

----- stderr -----
× No solution found when resolving dependencies:
╰─▶ my-project depends on django∅
╰─▶ Because my-project depends on django==5.0b1 and my-project depends on
django==5.0a1, version solving failed.

Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,21 @@ info:
- pip-compile
- requirements.in
- "--cache-dir"
- /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpPGmK15
- /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpur3UYY
env:
VIRTUAL_ENV: /var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/.tmpgXUIr9/.venv
VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpBlhDHf/.venv
---
success: false
exit_code: 2
exit_code: 1
----- stdout -----

----- stderr -----
error: Conflicting URLs for package `werkzeug`: https://files.pythonhosted.org/packages/bd/24/11c3ea5a7e866bf2d97f0501d0b4b1c9bbeade102bb4b588f0d2919a5212/Werkzeug-2.0.1-py3-none-any.whl and https://files.pythonhosted.org/packages/ff/1d/960bb4017c68674a1cb099534840f18d3def3ce44aed12b5ed8b78e0153e/Werkzeug-2.0.0-py3-none-any.whl
× No solution found when resolving dependencies:
╰─▶ Because root depends on werkzeug @ https://
files.pythonhosted.org/packages/
bd/24/11c3ea5a7e866bf2d97f0501d0b4b1c9bbeade102bb4b588f0d2919a5212/
Werkzeug-2.0.1-py3-none-any.whl==0 and root depends
on werkzeug @ https://files.pythonhosted.org/packages/
ff/1d/960bb4017c68674a1cb099534840f18d3def3ce44aed12b5ed8b78e0153e/
Werkzeug-2.0.0-py3-none-any.whl==1, version solving failed.

Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ info:
- pip-compile
- requirements.in
- "--cache-dir"
- /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpMkgmyj
- /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpvfszXx
env:
VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpGndWSU/.venv
VIRTUAL_ENV: /var/folders/bc/qlsk3t6x7c9fhhbvvcg68k9c0000gp/T/.tmpf7Zv4n/.venv
---
success: false
exit_code: 1
----- stdout -----

----- stderr -----
× No solution found when resolving dependencies:
╰─▶ Because there is no version of werkzeug available matching >=3.0.0 and
flask==3.0.0 depends on werkzeug>=3.0.0, flask==3.0.0 is forbidden.
╰─▶ Because flask==3.0.0 depends on werkzeug>=3.0.0 and there is no version
of werkzeug available matching >=3.0.0, flask==3.0.0 is forbidden.
And because root depends on flask==3.0.0, version solving failed.

112 changes: 12 additions & 100 deletions crates/puffin-resolver/src/pubgrub/dependencies.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,28 @@
use fxhash::FxHashMap;
use itertools::Itertools;
use pubgrub::range::Range;
use tracing::warn;

use pep508_rs::{MarkerEnvironment, Requirement, VersionOrUrl};
use puffin_cache::CanonicalUrl;

use puffin_normalize::{ExtraName, PackageName};

use crate::pubgrub::specifier::PubGrubSpecifier;
use crate::pubgrub::{PubGrubPackage, PubGrubVersion};
use crate::ResolveError;

#[derive(Debug)]
pub struct PubGrubDependencies(FxHashMap<PubGrubPackage, Range<PubGrubVersion>>);
pub struct PubGrubDependencies(Vec<(PubGrubPackage, Range<PubGrubVersion>)>);

impl PubGrubDependencies {
/// Generate a set of `PubGrub` dependencies from a set of requirements.
/// Generate a collection of `PubGrub` dependencies from a collection of requirements.
pub(crate) fn try_from_requirements<'a>(
requirements: &[Requirement],
constraints: &[Requirement],
extra: Option<&'a ExtraName>,
source: Option<&'a PackageName>,
env: &'a MarkerEnvironment,
) -> Result<Self, ResolveError> {
let mut dependencies = FxHashMap::<PubGrubPackage, Range<PubGrubVersion>>::default();
let mut dependencies = Vec::new();

// Iterate over all declared requirements.
for requirement in requirements {
Expand Down Expand Up @@ -55,21 +54,7 @@ impl PubGrubDependencies {
.map(|extra| to_pubgrub(requirement, Some(extra))),
) {
let (package, version) = result?;

if let Some(entry) = dependencies.get_key_value(&package) {
// Merge the versions.
let version = merge_versions(entry.1, &version);

// Merge the package.
if let Some(package) = merge_package(entry.0, &package)? {
dependencies.remove(&package);
dependencies.insert(package, version);
} else {
dependencies.insert(package, version);
}
} else {
dependencies.insert(package.clone(), version.clone());
}
dependencies.push((package.clone(), version.clone()));
}
}

Expand Down Expand Up @@ -103,41 +88,26 @@ impl PubGrubDependencies {
.map(|extra| to_pubgrub(constraint, Some(extra))),
) {
let (package, version) = result?;

if let Some(entry) = dependencies.get_key_value(&package) {
// Merge the versions.
let version = merge_versions(entry.1, &version);

// Merge the package.
if let Some(package) = merge_package(entry.0, &package)? {
dependencies.insert(package, version);
} else {
dependencies.insert(package, version);
}
}
dependencies.push((package.clone(), version.clone()));
}
}

Ok(Self(dependencies))
}

/// Insert a [`PubGrubPackage`] and [`PubGrubVersion`] range into the set of dependencies.
pub(crate) fn insert(
&mut self,
package: PubGrubPackage,
version: Range<PubGrubVersion>,
) -> Option<Range<PubGrubVersion>> {
self.0.insert(package, version)
// Add a [`PubGrubPackage`] and [`PubGrubVersion`] range into the dependencies.
pub(crate) fn push(&mut self, package: PubGrubPackage, version: Range<PubGrubVersion>) {
self.0.push((package, version))
}

/// Iterate over the dependencies.
pub(crate) fn iter(&self) -> impl Iterator<Item = (&PubGrubPackage, &Range<PubGrubVersion>)> {
pub(crate) fn iter(&self) -> impl Iterator<Item = &(PubGrubPackage, Range<PubGrubVersion>)> {
self.0.iter()
}
}

/// Convert a [`PubGrubDependencies`] to a [`DependencyConstraints`].
impl From<PubGrubDependencies> for FxHashMap<PubGrubPackage, Range<PubGrubVersion>> {
/// Convert a [`PubGrubDependencies`] to a [`Vec`].
impl From<PubGrubDependencies> for Vec<(PubGrubPackage, Range<PubGrubVersion>)> {
fn from(dependencies: PubGrubDependencies) -> Self {
dependencies.0
}
Expand Down Expand Up @@ -174,61 +144,3 @@ fn to_pubgrub(
}
}
}

/// Merge two [`PubGrubVersion`] ranges.
fn merge_versions(
left: &Range<PubGrubVersion>,
right: &Range<PubGrubVersion>,
) -> Range<PubGrubVersion> {
left.intersection(right)
}

/// Merge two [`PubGrubPackage`] instances.
fn merge_package(
left: &PubGrubPackage,
right: &PubGrubPackage,
) -> Result<Option<PubGrubPackage>, ResolveError> {
match (left, right) {
// Either package is `root`.
(PubGrubPackage::Root(_), _) | (_, PubGrubPackage::Root(_)) => Ok(None),

// Left package has a URL. Propagate the URL.
(PubGrubPackage::Package(name, extra, Some(url)), PubGrubPackage::Package(.., None)) => {
Ok(Some(PubGrubPackage::Package(
name.clone(),
extra.clone(),
Some(url.clone()),
)))
}

// Right package has a URL.
(PubGrubPackage::Package(.., None), PubGrubPackage::Package(name, extra, Some(url))) => {
Ok(Some(PubGrubPackage::Package(
name.clone(),
extra.clone(),
Some(url.clone()),
)))
}

// Neither package has a URL.
(PubGrubPackage::Package(_name, _extra, None), PubGrubPackage::Package(.., None)) => {
Ok(None)
}

// Both packages have a URL.
(
PubGrubPackage::Package(name, _extra, Some(left)),
PubGrubPackage::Package(.., Some(right)),
) => {
if CanonicalUrl::new(left) == CanonicalUrl::new(right) {
Ok(None)
} else {
Err(ResolveError::ConflictingUrls(
name.clone(),
left.to_string(),
right.to_string(),
))
}
}
}
}
8 changes: 8 additions & 0 deletions crates/puffin-resolver/src/pubgrub/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ pub enum PubGrubPackage {
#[derivative(Hash = "ignore")]
Option<Url>,
),
Url(
PackageName,
#[derivative(PartialEq = "ignore")]
#[derivative(PartialOrd = "ignore")]
#[derivative(Hash = "ignore")]
Url,
),
}

impl std::fmt::Display for PubGrubPackage {
Expand All @@ -84,6 +91,7 @@ impl std::fmt::Display for PubGrubPackage {
PubGrubPackage::Package(name, Some(extra), ..) => {
write!(f, "{name}[{extra}]")
}
PubGrubPackage::Url(name, url) => write!(f, "{name} @ {url}"),
}
}
}
1 change: 1 addition & 0 deletions crates/puffin-resolver/src/pubgrub/priority.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ impl PubGrubPriorities {
.copied()
.map(|priority| priority + 1)
.map(Reverse),
PubGrubPackage::Url(..) => None,
}
}
}
Expand Down
Loading
Loading