-
Notifications
You must be signed in to change notification settings - Fork 2.9k
universal-lock: add marker expressions to PubGrubPackage #3359
Description
As part of #3350, we'll need to distinguish conflicting dependencies among forks. We can achieve this by adding the relevant marker expressions to PubGrubPackage:
uv/crates/uv-resolver/src/pubgrub/package.rs
Lines 22 to 63 in e33ff95
| /// A Python package. | |
| Package( | |
| PackageName, | |
| Option<ExtraName>, | |
| /// The URL of the package, if it was specified in the requirement. | |
| /// | |
| /// There are a few challenges that come with URL-based packages, and how they map to | |
| /// PubGrub. | |
| /// | |
| /// If the user declares a direct URL dependency, and then a transitive dependency | |
| /// appears for the same package, we need to ensure that the direct URL dependency can | |
| /// "satisfy" that requirement. So if the user declares a URL dependency on Werkzeug, and a | |
| /// registry dependency on Flask, we need to ensure that Flask's dependency on Werkzeug | |
| /// is resolved by the URL dependency. This means: (1) we need to avoid adding a second | |
| /// Werkzeug variant from PyPI; and (2) we need to error if the Werkzeug version requested | |
| /// by Flask doesn't match that of the URL dependency. | |
| /// | |
| /// Additionally, we need to ensure that we disallow multiple versions of the same package, | |
| /// even if requested from different URLs. | |
| /// | |
| /// To enforce this requirement, we require that all possible URL dependencies are | |
| /// defined upfront, as `requirements.txt` or `constraints.txt` or similar. Otherwise, | |
| /// solving these graphs becomes far more complicated -- and the "right" behavior isn't | |
| /// even clear. For example, imagine that you define a direct dependency on Werkzeug, and | |
| /// then one of your other direct dependencies declares a dependency on Werkzeug at some | |
| /// URL. Which is correct? By requiring direct dependencies, the semantics are at least | |
| /// clear. | |
| /// | |
| /// With the list of known URLs available upfront, we then only need to do two things: | |
| /// | |
| /// 1. When iterating over the dependencies for a single package, ensure that we respect | |
| /// URL variants over registry variants, if the package declares a dependency on both | |
| /// `Werkzeug==2.0.0` _and_ `Werkzeug @ https://...` , which is strange but possible. | |
| /// This is enforced by [`crate::pubgrub::dependencies::PubGrubDependencies`]. | |
| /// 2. Reject any URL dependencies that aren't known ahead-of-time. | |
| /// | |
| /// Eventually, we could relax this constraint, in favor of something more lenient, e.g., if | |
| /// we're going to have a dependency that's provided as a URL, we _need_ to visit the URL | |
| /// version before the registry version. So we could just error if we visit a URL variant | |
| /// _after_ a registry variant. | |
| Option<VerbatimUrl>, | |
| ), |
I did this in my prototype by adding them to PubGrubPackage::Package:
uv/crates/uv-resolver/src/pubgrub/package.rs
Lines 50 to 53 in 7f2b401
| Package { | |
| name: PackageName, | |
| extra: Option<ExtraName>, | |
| marker: Option<MarkerTree>, |
But I think it might be worth experimenting with creating a new variant. Note that like extras, a package with a non-None marker expression should introduce a dependency on the package with the same name, but without the marker expressions:
uv/crates/uv-resolver/src/resolver/mod.rs
Lines 1031 to 1043 in 7f2b401
| // If a package has an extra, insert a constraint on the base package. | |
| if extra.is_some() || marker.is_some() { | |
| constraints.push( | |
| PubGrubPackage::Package { | |
| name: package_name.clone(), | |
| extra: None, | |
| marker: None, | |
| url: url.clone(), | |
| }, | |
| Range::singleton(version.clone()), | |
| None, | |
| ); | |
| } |
This adds the necessary constraints to ensure pubgrub's resolution is itself properly constrained.