Skip to content

Commit

Permalink
uv-resolver: evaluate extras when reading from lock
Browse files Browse the repository at this point in the history
This ensures we only pull out dependencies that satisfy their conflict
markers.
  • Loading branch information
BurntSushi committed Nov 23, 2024
1 parent 9006582 commit e7093c6
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 8 deletions.
2 changes: 1 addition & 1 deletion crates/uv-resolver/src/lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3618,7 +3618,7 @@ struct DependencyWire {
extra: BTreeSet<ExtraName>,
#[serde(default)]
marker: SimplifiedMarkerTree,
#[serde(default)]
#[serde(rename = "conflict-marker", default)]
conflict_marker: MarkerTree,
}

Expand Down
7 changes: 2 additions & 5 deletions crates/uv-resolver/src/lock/target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ impl<'env> InstallTarget<'env> {
})
.flatten()
{
if !dep.complexified_marker.evaluate(marker_env, &[]) {
if !dep.complexified_marker.satisfies(marker_env, extras, dev) {
continue;
}

Expand Down Expand Up @@ -252,9 +252,6 @@ impl<'env> InstallTarget<'env> {
// a specific marker environment and set of extras/groups.
// So at this point, we know the extras/groups have been
// satisfied, so we can safely drop the conflict marker.
//
// FIXME: Make the above true. We aren't actually checking
// the conflict marker yet.
Edge::Dev(group.clone(), dep.complexified_marker.pep508().clone()),
);

Expand Down Expand Up @@ -350,7 +347,7 @@ impl<'env> InstallTarget<'env> {
Either::Right(package.dependencies.iter())
};
for dep in deps {
if !dep.complexified_marker.evaluate(marker_env, &[]) {
if !dep.complexified_marker.satisfies(marker_env, extras, dev) {
continue;
}

Expand Down
25 changes: 23 additions & 2 deletions crates/uv-resolver/src/universal_marker.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use itertools::Itertools;

use uv_configuration::{DevGroupsManifest, ExtrasSpecification};
use uv_normalize::ExtraName;
use uv_pep508::{MarkerEnvironment, MarkerTree};
use uv_pypi_types::Conflicts;
Expand Down Expand Up @@ -135,12 +136,32 @@ impl UniversalMarker {

/// Returns true if this universal marker is satisfied by the given
/// marker environment and list of activated extras.
///
/// FIXME: This also needs to accept a list of groups.
pub(crate) fn evaluate(&self, env: &MarkerEnvironment, extras: &[ExtraName]) -> bool {
self.pep508_marker.evaluate(env, extras) && self.conflict_marker.evaluate(env, extras)
}

/// Returns true if this universal marker is satisfied by the given
/// marker environment and list of activated extras and groups.
///
/// TODO: Articulate the difference between this and `evaluate`.
pub(crate) fn satisfies(
&self,
env: &MarkerEnvironment,
extras: &ExtrasSpecification,
_dev: &DevGroupsManifest,
) -> bool {
if !self.pep508_marker.evaluate(env, &[]) {
return false;
}
let extra_list = match *extras {
// TODO(ag): This should still evaluate `dev`.
ExtrasSpecification::All => return true,
ExtrasSpecification::None => &[][..],
ExtrasSpecification::Some(ref list) => list,
};
self.conflict_marker.evaluate(env, extra_list)
}

/// Returns the PEP 508 marker for this universal marker.
///
/// One should be cautious using this. Generally speaking, it should only
Expand Down

0 comments on commit e7093c6

Please sign in to comment.