Skip to content

Commit ed1ac64

Browse files
authored
Consolidate UnusableDependencies into a generic Unavailable incompatibility (#1088)
Requires astral-sh/pubgrub#20 In short, `UnusableDependencies` can be generalized into `Unavailable` which encompasses incompatibilities where a package range which is unusable for some inherent reason as well as when its dependencies are unusable. We can eventually use this to track more incompatibilities in the solver. I made the reason string required because I can't see a case where we should leave it out. Additionally, this improves the display of conflicts in the root requirements.
1 parent 091f8e0 commit ed1ac64

File tree

7 files changed

+54
-49
lines changed

7 files changed

+54
-49
lines changed

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ owo-colors = { version = "3.5.0" }
5353
petgraph = { version = "0.6.4" }
5454
platform-info = { version = "2.0.2" }
5555
plist = { version = "1.6.0" }
56-
pubgrub = { git = "https://github.com/zanieb/pubgrub", rev = "0e02ea9fc8d021fb6a6b9e77b09ade4332068f42" }
56+
pubgrub = { git = "https://github.com/zanieb/pubgrub", rev = "7a573e3902ff338abdcf3b87682e4c6d04845f2f" }
5757
pyo3 = { version = "0.20.2" }
5858
pyo3-log = { version = "0.9.0"}
5959
pyproject-toml = { version = "0.8.1" }

crates/puffin-resolver/src/error.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,10 @@ pub enum ResolveError {
4949
#[error("~= operator requires at least two release segments: {0}")]
5050
InvalidTildeEquals(pep440_rs::VersionSpecifier),
5151

52-
#[error("Conflicting URLs for package `{0}`:\n- {1}\n- {2}")]
52+
#[error("There are conflicting URLs for package `{0}`:\n- {1}\n- {2}")]
5353
ConflictingUrls(PackageName, String, String),
5454

55-
#[error("Conflicting versions for `{0}`: {1}")]
55+
#[error("There are conflicting versions for `{0}`: {1}")]
5656
ConflictingVersions(String, String),
5757

5858
#[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.")]

crates/puffin-resolver/src/pubgrub/report.rs

Lines changed: 12 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -96,32 +96,18 @@ impl ReportFormatter<PubGrubPackage, Range<Version>> for PubGrubReportFormatter<
9696
}
9797
}
9898
}
99-
External::UnavailableDependencies(package, set) => {
100-
let set = self.simplify_set(set, package);
101-
format!(
102-
"dependencies of {}are unavailable",
103-
Padded::new("", &PackageRange::compatibility(package, &set), " ")
104-
)
105-
}
106-
External::UnusableDependencies(package, set, reason) => {
107-
if let Some(reason) = reason {
108-
if matches!(package, PubGrubPackage::Root(_)) {
109-
format!("{package} dependencies are unusable: {reason}")
110-
} else {
111-
let set = self.simplify_set(set, package);
112-
format!(
113-
"dependencies of {}are unusable: {reason}",
114-
Padded::new("", &PackageRange::compatibility(package, &set), " ")
115-
)
116-
}
117-
} else {
118-
let set = self.simplify_set(set, package);
119-
format!(
120-
"dependencies of {}are unusable",
121-
Padded::new("", &PackageRange::compatibility(package, &set), " ")
122-
)
99+
External::Unavailable(package, set, reason) => match package {
100+
PubGrubPackage::Root(Some(name)) => {
101+
format!("{name} cannot be used because {reason}")
123102
}
124-
}
103+
PubGrubPackage::Root(None) => {
104+
format!("your requirements cannot be used because {reason}")
105+
}
106+
_ => format!(
107+
"{}is unusable because {reason}",
108+
Padded::new("", &PackageRange::compatibility(package, set), " ")
109+
),
110+
},
125111
External::FromDependencyOf(package, package_set, dependency, dependency_set) => {
126112
let package_set = self.simplify_set(package_set, package);
127113
let dependency_set = self.simplify_set(dependency_set, dependency);
@@ -403,8 +389,7 @@ impl PubGrubReportFormatter<'_> {
403389
}
404390
}
405391
External::NotRoot(..) => {}
406-
External::UnavailableDependencies(..) => {}
407-
External::UnusableDependencies(..) => {}
392+
External::Unavailable(..) => {}
408393
External::FromDependencyOf(..) => {}
409394
},
410395
DerivationTree::Derived(derived) => {

crates/puffin-resolver/src/resolver/mod.rs

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -335,22 +335,30 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
335335
.get_dependencies(package, &version, &mut priorities, request_sink)
336336
.await?
337337
{
338-
Dependencies::Unusable(reason) => {
339-
state.add_incompatibility(Incompatibility::unusable_dependencies(
338+
Dependencies::Unavailable(reason) => {
339+
let message = {
340+
if matches!(package, PubGrubPackage::Root(_)) {
341+
// Including front-matter for the root package is redundant
342+
reason.clone()
343+
} else {
344+
format!("its dependencies are unusable because {reason}")
345+
}
346+
};
347+
state.add_incompatibility(Incompatibility::unavailable(
340348
package.clone(),
341349
version.clone(),
342-
reason.clone(),
350+
message,
343351
));
344352
continue;
345353
}
346-
Dependencies::Known(constraints) if constraints.contains_key(package) => {
354+
Dependencies::Available(constraints) if constraints.contains_key(package) => {
347355
return Err(PubGrubError::SelfDependency {
348356
package: package.clone(),
349357
version: version.clone(),
350358
}
351359
.into());
352360
}
353-
Dependencies::Known(constraints) => constraints,
361+
Dependencies::Available(constraints) => constraints,
354362
};
355363

356364
// Add that package and version if the dependencies are not problematic.
@@ -588,7 +596,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
588596
| ResolveError::ConflictingUrls(..)),
589597
) = constraints
590598
{
591-
return Ok(Dependencies::Unusable(Some(err.to_string())));
599+
return Ok(Dependencies::Unavailable(uncapitalize(err.to_string())));
592600
}
593601
let mut constraints = constraints?;
594602

@@ -610,10 +618,12 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
610618
);
611619
}
612620

613-
Ok(Dependencies::Known(constraints.into()))
621+
Ok(Dependencies::Available(constraints.into()))
614622
}
615623

616-
PubGrubPackage::Python(_) => Ok(Dependencies::Known(DependencyConstraints::default())),
624+
PubGrubPackage::Python(_) => {
625+
Ok(Dependencies::Available(DependencyConstraints::default()))
626+
}
617627

618628
PubGrubPackage::Package(package_name, extra, url) => {
619629
// Wait for the metadata to be available.
@@ -640,7 +650,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
640650
version.clone(),
641651
);
642652
constraints.insert(PubGrubPackage::Python(PubGrubPython::Target), version);
643-
return Ok(Dependencies::Known(constraints));
653+
return Ok(Dependencies::Available(constraints));
644654
}
645655

646656
let entry = self.index.distributions.wait(&package_id).await?;
@@ -670,7 +680,7 @@ impl<'a, Provider: ResolverProvider> Resolver<'a, Provider> {
670680
);
671681
}
672682

673-
Ok(Dependencies::Known(constraints.into()))
683+
Ok(Dependencies::Available(constraints.into()))
674684
}
675685
}
676686
}
@@ -877,8 +887,16 @@ enum Response {
877887
/// For each [Package] there is a set of versions allowed as a dependency.
878888
#[derive(Clone)]
879889
enum Dependencies {
880-
/// Package dependencies are not usable
881-
Unusable(Option<String>),
890+
/// Package dependencies are not available.
891+
Unavailable(String),
882892
/// Container for all available package versions.
883-
Known(DependencyConstraints<PubGrubPackage, Range<Version>>),
893+
Available(DependencyConstraints<PubGrubPackage, Range<Version>>),
894+
}
895+
896+
fn uncapitalize<T: AsRef<str>>(string: T) -> String {
897+
let mut chars = string.as_ref().chars();
898+
match chars.next() {
899+
None => String::new(),
900+
Some(first) => first.to_lowercase().chain(chars).collect(),
901+
}
884902
}

crates/puffin/tests/pip_compile.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1516,7 +1516,8 @@ fn conflicting_repeated_url_dependency_version_mismatch() -> Result<()> {
15161516
15171517
----- stderr -----
15181518
× No solution found when resolving dependencies:
1519-
╰─▶ root dependencies are unusable: Conflicting URLs for package `werkzeug`:
1519+
╰─▶ your requirements cannot be used because there are conflicting URLs for
1520+
package `werkzeug`:
15201521
- https://files.pythonhosted.org/packages/bd/24/11c3ea5a7e866bf2d97f0501d0b4b1c9bbeade102bb4b588f0d2919a5212/Werkzeug-2.0.1-py3-none-any.whl
15211522
- https://files.pythonhosted.org/packages/ff/1d/960bb4017c68674a1cb099534840f18d3def3ce44aed12b5ed8b78e0153e/Werkzeug-2.0.0-py3-none-any.whl
15221523
"###);
@@ -1556,7 +1557,8 @@ fn conflicting_repeated_url_dependency_version_match() -> Result<()> {
15561557
15571558
----- stderr -----
15581559
× No solution found when resolving dependencies:
1559-
╰─▶ root dependencies are unusable: Conflicting URLs for package `werkzeug`:
1560+
╰─▶ your requirements cannot be used because there are conflicting URLs for
1561+
package `werkzeug`:
15601562
- git+https://github.com/pallets/werkzeug.git@2.0.0
15611563
- https://files.pythonhosted.org/packages/ff/1d/960bb4017c68674a1cb099534840f18d3def3ce44aed12b5ed8b78e0153e/Werkzeug-2.0.0-py3-none-any.whl
15621564
"###);
@@ -1900,8 +1902,8 @@ dependencies = ["django==5.0b1", "django==5.0a1"]
19001902
19011903
----- stderr -----
19021904
× No solution found when resolving dependencies:
1903-
╰─▶ my-project dependencies are unusable: Conflicting versions for `django`:
1904-
`django==5.0b1` does not intersect with `django==5.0a1`
1905+
╰─▶ my-project cannot be used because there are conflicting versions for
1906+
`django`: `django==5.0b1` does not intersect with `django==5.0a1`
19051907
"###);
19061908
});
19071909

crates/puffin/tests/pip_install_scenarios.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1245,7 +1245,7 @@ fn direct_incompatible_versions() -> Result<()> {
12451245
12461246
----- stderr -----
12471247
× No solution found when resolving dependencies:
1248-
╰─▶ root dependencies are unusable: Conflicting versions for `albatross`: `albatross==1.0.0` does not intersect with `albatross==2.0.0`
1248+
╰─▶ your requirements cannot be used because there are conflicting versions for `albatross`: `albatross==1.0.0` does not intersect with `albatross==2.0.0`
12491249
"###);
12501250
});
12511251

0 commit comments

Comments
 (0)