Improve display of resolution errors for workspace member conflicts with optional dependencies (#6123)

We have bad error messages for optional (extra) dependencies and
development dependencies in workspaces:

1. We weren't showing the full package, so we'd drop `:dev` and
`[extra]` by accident
2. We didn't include derived packages, e.g., `member[extra]` in tree
processing collapse operation, so we'd include extra clauses like the
ones we removed in #6092

Also

- Reverts
f0de4f71f2
— it turns out it wasn't quite correct and it didn't seem worth using
the custom incompatibility anymore.
- Fixes a bug in the display of `package:dev` which was not showing
`:dev` for some variants (see 94d8020b58)
This commit is contained in:
Zanie Blue 2024-08-15 20:50:43 -05:00 committed by GitHub
parent e6ddce0246
commit 89efe2491b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 71 additions and 62 deletions

View File

@ -221,7 +221,7 @@ impl std::fmt::Display for NoSolutionError {
// Transform the error tree for reporting // Transform the error tree for reporting
let mut tree = self.error.clone(); let mut tree = self.error.clone();
collapse_unavailable_workspace_members(&mut tree); collapse_no_versions_of_workspace_members(&mut tree, &self.workspace_members);
if self.workspace_members.len() == 1 { if self.workspace_members.len() == 1 {
let project = self.workspace_members.iter().next().unwrap(); let project = self.workspace_members.iter().next().unwrap();
@ -248,10 +248,11 @@ impl std::fmt::Display for NoSolutionError {
} }
} }
/// Given a [`DerivationTree`], collapse any [`UnavailablePackage::WorkspaceMember`] incompatibilities /// Given a [`DerivationTree`], collapse any `NoVersion` incompatibilities for workspace members
/// to avoid saying things like "only <workspace-member>==0.1.0 is available". /// to avoid saying things like "only <workspace-member>==0.1.0 is available".
fn collapse_unavailable_workspace_members( fn collapse_no_versions_of_workspace_members(
tree: &mut DerivationTree<PubGrubPackage, Range<Version>, UnavailableReason>, tree: &mut DerivationTree<PubGrubPackage, Range<Version>, UnavailableReason>,
workspace_members: &BTreeSet<PackageName>,
) { ) {
match tree { match tree {
DerivationTree::External(_) => {} DerivationTree::External(_) => {}
@ -260,33 +261,36 @@ fn collapse_unavailable_workspace_members(
Arc::make_mut(&mut derived.cause1), Arc::make_mut(&mut derived.cause1),
Arc::make_mut(&mut derived.cause2), Arc::make_mut(&mut derived.cause2),
) { ) {
// If one node is an unavailable workspace member... // If we have a node for a package with no versions...
( (DerivationTree::External(External::NoVersions(package, _)), ref mut other)
DerivationTree::External(External::Custom( | (ref mut other, DerivationTree::External(External::NoVersions(package, _))) => {
_, // First, always recursively visit the other side of the tree
_, collapse_no_versions_of_workspace_members(other, workspace_members);
UnavailableReason::Package(UnavailablePackage::WorkspaceMember),
)),
ref mut other,
)
| (
ref mut other,
DerivationTree::External(External::Custom(
_,
_,
UnavailableReason::Package(UnavailablePackage::WorkspaceMember),
)),
) => {
// First, recursively collapse the other side of the tree
collapse_unavailable_workspace_members(other);
// Then, replace this node with the other tree // Then, if the package is a workspace member...
let (PubGrubPackageInner::Package { name, .. }
| PubGrubPackageInner::Extra { name, .. }
| PubGrubPackageInner::Dev { name, .. }) = &**package
else {
return;
};
if !workspace_members.contains(name) {
return;
}
// Replace this node with the other tree
*tree = other.clone(); *tree = other.clone();
} }
// If not, just recurse // If not, just recurse
_ => { _ => {
collapse_unavailable_workspace_members(Arc::make_mut(&mut derived.cause1)); collapse_no_versions_of_workspace_members(
collapse_unavailable_workspace_members(Arc::make_mut(&mut derived.cause2)); Arc::make_mut(&mut derived.cause1),
workspace_members,
);
collapse_no_versions_of_workspace_members(
Arc::make_mut(&mut derived.cause2),
workspace_members,
);
} }
} }
} }

View File

@ -196,13 +196,13 @@ impl std::fmt::Display for PubGrubPackageInner {
name, name,
extra: None, extra: None,
marker: None, marker: None,
.. dev: None,
} => write!(f, "{name}"), } => write!(f, "{name}"),
Self::Package { Self::Package {
name, name,
extra: Some(extra), extra: Some(extra),
marker: None, marker: None,
.. dev: None,
} => { } => {
write!(f, "{name}[{extra}]") write!(f, "{name}[{extra}]")
} }
@ -210,19 +210,40 @@ impl std::fmt::Display for PubGrubPackageInner {
name, name,
extra: None, extra: None,
marker: Some(marker), marker: Some(marker),
.. dev: None,
} => write!(f, "{name}{{{marker}}}"), } => write!(f, "{name}{{{marker}}}"),
Self::Package { Self::Package {
name, name,
extra: Some(extra), extra: Some(extra),
marker: Some(marker), marker: Some(marker),
.. dev: None,
} => { } => {
write!(f, "{name}[{extra}]{{{marker}}}") write!(f, "{name}[{extra}]{{{marker}}}")
} }
Self::Package {
name,
extra: None,
marker: None,
dev: Some(dev),
} => write!(f, "{name}:{dev}"),
Self::Package {
name,
extra: None,
marker: Some(marker),
dev: Some(dev),
} => {
write!(f, "{name}[{dev}]{{{marker}}}")
}
Self::Marker { name, marker, .. } => write!(f, "{name}{{{marker}}}"), Self::Marker { name, marker, .. } => write!(f, "{name}{{{marker}}}"),
Self::Extra { name, extra, .. } => write!(f, "{name}[{extra}]"), Self::Extra { name, extra, .. } => write!(f, "{name}[{extra}]"),
Self::Dev { name, dev, .. } => write!(f, "{name}:{dev}"), Self::Dev { name, dev, .. } => write!(f, "{name}:{dev}"),
// It is guaranteed that `extra` and `dev` are never set at the same time.
Self::Package {
name: _,
extra: Some(_),
marker: _,
dev: Some(_),
} => unreachable!(),
} }
} }
} }

View File

@ -385,19 +385,22 @@ impl PubGrubReportFormatter<'_> {
/// Return a display name for the package if it is a workspace member. /// Return a display name for the package if it is a workspace member.
fn format_workspace_member(&self, package: &PubGrubPackage) -> Option<String> { fn format_workspace_member(&self, package: &PubGrubPackage) -> Option<String> {
match &**package { match &**package {
PubGrubPackageInner::Package { name, .. } // TODO(zanieb): Improve handling of dev and extra for single-project workspaces
| PubGrubPackageInner::Extra { name, .. } PubGrubPackageInner::Package {
| PubGrubPackageInner::Dev { name, .. } => { name, extra, dev, ..
if self.workspace_members.contains(name) { } if self.workspace_members.contains(name) => {
if self.is_single_project_workspace() { if self.is_single_project_workspace() && extra.is_none() && dev.is_none() {
Some("your project".to_string()) Some("your project".to_string())
} else {
Some(format!("{name}"))
}
} else { } else {
None Some(format!("{package}"))
} }
} }
PubGrubPackageInner::Extra { name, .. } if self.workspace_members.contains(name) => {
Some(format!("{package}"))
}
PubGrubPackageInner::Dev { name, .. } if self.workspace_members.contains(name) => {
Some(format!("{package}"))
}
_ => None, _ => None,
} }
} }
@ -615,7 +618,7 @@ impl PubGrubReportFormatter<'_> {
reason: reason.clone(), reason: reason.clone(),
}); });
} }
Some(UnavailablePackage::NotFound | UnavailablePackage::WorkspaceMember) => {} Some(UnavailablePackage::NotFound) => {}
None => {} None => {}
} }

View File

@ -74,8 +74,6 @@ pub(crate) enum UnavailablePackage {
InvalidMetadata(String), InvalidMetadata(String),
/// The package has an invalid structure. /// The package has an invalid structure.
InvalidStructure(String), InvalidStructure(String),
/// No other versions of the package can be used because it is a workspace member
WorkspaceMember,
} }
impl UnavailablePackage { impl UnavailablePackage {
@ -87,7 +85,6 @@ impl UnavailablePackage {
UnavailablePackage::MissingMetadata => "does not include a `METADATA` file", UnavailablePackage::MissingMetadata => "does not include a `METADATA` file",
UnavailablePackage::InvalidMetadata(_) => "has invalid metadata", UnavailablePackage::InvalidMetadata(_) => "has invalid metadata",
UnavailablePackage::InvalidStructure(_) => "has an invalid package format", UnavailablePackage::InvalidStructure(_) => "has an invalid package format",
UnavailablePackage::WorkspaceMember => "is a workspace member",
} }
} }
} }

View File

@ -442,21 +442,6 @@ impl<InstalledPackages: InstalledPackagesProvider> ResolverState<InstalledPackag
.expect("a package was chosen but we don't have a term"); .expect("a package was chosen but we don't have a term");
if let PubGrubPackageInner::Package { ref name, .. } = &*state.next { if let PubGrubPackageInner::Package { ref name, .. } = &*state.next {
// Check if the decision was due to the package being a
// workspace member
if self.workspace_members.contains(name) {
state
.pubgrub
.add_incompatibility(Incompatibility::custom_term(
state.next.clone(),
term_intersection.clone(),
UnavailableReason::Package(
UnavailablePackage::WorkspaceMember,
),
));
continue;
}
// Check if the decision was due to the package being unavailable // Check if the decision was due to the package being unavailable
if let Some(entry) = self.unavailable_packages.get(name) { if let Some(entry) = self.unavailable_packages.get(name) {
state state

View File

@ -1381,9 +1381,8 @@ fn workspace_unsatisfiable_member_dependencies_conflicting_extra() -> Result<()>
----- stderr ----- ----- stderr -----
Using Python 3.12.[X] interpreter at: [PYTHON-3.12] Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
× No solution found when resolving dependencies: × No solution found when resolving dependencies:
Because only bar is available and bar depends on anyio==4.2.0, we can conclude that bar depends on anyio==4.2.0. Because bar[some-extra] depends on anyio==4.2.0 and foo depends on anyio==4.1.0, we can conclude that foo and bar[some-extra] are incompatible.
And because foo depends on anyio==4.1.0, we can conclude that foo and bar are incompatible. And because your workspace requires bar[some-extra] and foo, we can conclude that your workspace's requirements are unsatisfiable.
And because your workspace requires bar and foo, we can conclude that your workspace's requirements are unsatisfiable.
"### "###
); );
@ -1440,7 +1439,7 @@ fn workspace_unsatisfiable_member_dependencies_conflicting_dev() -> Result<()> {
----- stderr ----- ----- stderr -----
Using Python 3.12.[X] interpreter at: [PYTHON-3.12] Using Python 3.12.[X] interpreter at: [PYTHON-3.12]
× No solution found when resolving dependencies: × No solution found when resolving dependencies:
Because bar depends on bar and bar depends on anyio==4.2.0, we can conclude that bar depends on anyio==4.2.0. Because bar depends on bar:dev and bar:dev depends on anyio==4.2.0, we can conclude that bar depends on anyio==4.2.0.
And because foo depends on anyio==4.1.0, we can conclude that bar and foo are incompatible. And because foo depends on anyio==4.1.0, we can conclude that bar and foo are incompatible.
And because your workspace requires bar and foo, we can conclude that your workspace's requirements are unsatisfiable. And because your workspace requires bar and foo, we can conclude that your workspace's requirements are unsatisfiable.
"### "###