-
Notifications
You must be signed in to change notification settings - Fork 738
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Move conflicting dependencies into PubGrub (#1796)
## Summary This revives a PR from long ago (#383 and astral-sh/pubgrub#24) that modifies how we deal with dependencies that are declared multiple times within a single package. To quote from the originating PR: > Uses an experimental pubgrub branch (#370) that allows us to handle multiple version ranges for a single dependency to the solver which results in better error messages because the derivation tree contains all of the relevant versions. Previously, the version ranges were merged (by us) in the resolver before handing them to pubgrub since only one range could be provided per package. Since we don't merge the versions anymore, we no longer give the solver an empty range for conflicting requirements; instead the solver comes to that conclusion from the provided versions. You can see the improved error message for direct dependencies in [this snapshot](https://github.com/astral-sh/puffin/pull/383/files#diff-a0437f2c20cde5e2f15199a3bf81a102b92580063268417847ec9c793a115bd0). The main issue with that PR was around its handling of URL dependencies, so this PR _also_ refactors how we handle those. Previously, we stored URL dependencies on `PubGrubPackage`, but they were omitted from the hash and equality implementations of `PubGrubPackage`. This led to some really careful codepaths wherein we had to ensure that we always visited URLs before non-URL packages, so that the URL-inclusive versions were included in any hashmaps, etc. I considered preserving this approach, but it would require us to rely on lots of internal details of PubGrub (since we'd now be relying on PubGrub to merge those packages in the "right" order). So, instead, we now _always_ set the URL on a given package, whenever that package was _given_ a URL upfront. I think this is easier to reason about: if the user provided a URL for `flask`, then we should just always add the URL for `flask`. If we see some other URL for `flask`, we error, like before. If we see some unknown URL for `flask`, we error, like before. Closes #1522. Closes #1821. Closes #1615.
- Loading branch information
1 parent
b3be53b
commit 7eaed07
Showing
20 changed files
with
703 additions
and
367 deletions.
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
use std::hash::BuildHasherDefault; | ||
|
||
use rustc_hash::FxHashMap; | ||
|
||
use pep508_rs::Requirement; | ||
use uv_normalize::PackageName; | ||
|
||
/// A set of constraints for a set of requirements. | ||
#[derive(Debug, Default, Clone)] | ||
pub(crate) struct Constraints(FxHashMap<PackageName, Vec<Requirement>>); | ||
|
||
impl Constraints { | ||
/// Create a new set of constraints from a set of requirements. | ||
pub(crate) fn from_requirements(requirements: Vec<Requirement>) -> Self { | ||
let mut constraints: FxHashMap<PackageName, Vec<Requirement>> = | ||
FxHashMap::with_capacity_and_hasher(requirements.len(), BuildHasherDefault::default()); | ||
for requirement in requirements { | ||
constraints | ||
.entry(requirement.name.clone()) | ||
.or_default() | ||
.push(requirement); | ||
} | ||
Self(constraints) | ||
} | ||
|
||
/// Get the constraints for a package. | ||
pub(crate) fn get(&self, name: &PackageName) -> Option<&Vec<Requirement>> { | ||
self.0.get(name) | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
use std::hash::BuildHasherDefault; | ||
|
||
use rustc_hash::FxHashMap; | ||
|
||
use distribution_types::LocalEditable; | ||
use pypi_types::Metadata21; | ||
use uv_normalize::PackageName; | ||
|
||
/// A set of editable packages, indexed by package name. | ||
#[derive(Debug, Default, Clone)] | ||
pub(crate) struct Editables(FxHashMap<PackageName, (LocalEditable, Metadata21)>); | ||
|
||
impl Editables { | ||
/// Create a new set of editables from a set of requirements. | ||
pub(crate) fn from_requirements(requirements: Vec<(LocalEditable, Metadata21)>) -> Self { | ||
let mut editables = | ||
FxHashMap::with_capacity_and_hasher(requirements.len(), BuildHasherDefault::default()); | ||
for (editable_requirement, metadata) in requirements { | ||
editables.insert(metadata.name.clone(), (editable_requirement, metadata)); | ||
} | ||
Self(editables) | ||
} | ||
|
||
/// Get the editable for a package. | ||
pub(crate) fn get(&self, name: &PackageName) -> Option<&(LocalEditable, Metadata21)> { | ||
self.0.get(name) | ||
} | ||
|
||
/// Iterate over all editables. | ||
pub(crate) fn iter(&self) -> impl Iterator<Item = &(LocalEditable, Metadata21)> { | ||
self.0.values() | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.