Skip to content

Commit

Permalink
uv-resolver: add "include" rules to ResolverEnvironment
Browse files Browse the repository at this point in the history
When we generate conflict markers for each resolution after the
resolver runs, it turns out that generating them just from exclusion
rules is not sufficient.

For example, if `foo` and `bar` are declared as conflicting extras, then
we end up with the following forks:

    A: extra != 'foo'
    B: extra != 'bar'
    C: extra != 'foo' and extra != 'bar'

Now let's take an example where these forks don't share the same version
for all packages. Consider a case where `idna==3.9` is in forks A and C,
but where `idna==3.10` is in fork B. If we combine the markers in forks
A and C through disjunction, we get the following:

     idna==3.9: extra != 'foo' or (extra != 'foo' and extra != 'bar')
    idna==3.10: extra != 'bar'

Which simplifies to:

     idna==3.9: extra != 'foo'
    idna==3.10: extra != 'bar'

But these are clearly not disjoint. Both dependencies could be selected,
for example, when neither `foo` nor `bar` are active. We can remedy this
by keeping around the inclusion rules for each fork:

    A: extra != 'foo' and extra == 'bar'
    B: extra != 'bar' and extra == 'foo'
    C: extra != 'foo' and extra != 'bar'

And so for `idna`, we have:

     idna==3.9: (extra != 'foo' and extra == 'bar') or (extra != 'foo' and extra != 'bar')
    idna==3.10: extra != 'bar' and extra == 'foo'

Which simplifies to:

     idna==3.9: extra != 'foo'
    idna==3.10: extra != 'bar' and extra == 'foo'

And these *are* properly disjoint. There is no way for them both to be
active. This also correctly accounts for fork C where neither `foo` nor
`bar` are active, and yet, `idna==3.9` is still enabled but `idna==3.10`
is not. (In the [motivating example], this comes from `baz` being enabled.)
That is, this captures the idea that for `idna==3.10` to be installed,
there must actually be a specific extra that is enabled. That's what
makes it disjoint from `idna==3.9`.

We aren't quite done yet, because this does add *too many* conflict
markers to dependency edges that don't need it. In the next commit,
we'll add in our world knowledge to simplify these conflict markers.

[motivating example]: #9289
  • Loading branch information
BurntSushi committed Nov 22, 2024
1 parent bed3f1e commit 436fed0
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 27 deletions.
88 changes: 76 additions & 12 deletions crates/uv-resolver/src/resolver/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,20 @@ enum Kind {
initial_forks: Arc<[MarkerTree]>,
/// The markers associated with this resolver fork.
markers: MarkerTree,
/// Conflicting group inclusions.
///
/// Note that inclusions don't play a role in predicates
/// like `ResolverEnvironment::included_by_group`. Instead,
/// only exclusions are considered.
///
/// We record inclusions for two reasons. First is that if
/// we somehow wind up with an inclusion and exclusion rule
/// for the same conflict item, then we treat the resulting
/// fork as impossible. (You cannot require that an extra is
/// both included and excluded. Such a rule can never be
/// satisfied.) Second is that we use the inclusion rules to
/// write conflict markers after resolution is finished.
include: Arc<crate::FxHashbrownSet<ConflictItem>>,
/// Conflicting group exclusions.
exclude: Arc<crate::FxHashbrownSet<ConflictItem>>,
},
Expand Down Expand Up @@ -134,6 +148,7 @@ impl ResolverEnvironment {
let kind = Kind::Universal {
initial_forks: initial_forks.into(),
markers: MarkerTree::TRUE,
include: Arc::new(crate::FxHashbrownSet::default()),
exclude: Arc::new(crate::FxHashbrownSet::default()),
};
ResolverEnvironment { kind }
Expand Down Expand Up @@ -201,55 +216,80 @@ impl ResolverEnvironment {
Kind::Universal {
ref initial_forks,
markers: ref lhs,
ref include,
ref exclude,
} => {
let mut markers = lhs.clone();
markers.and(rhs);
let kind = Kind::Universal {
initial_forks: Arc::clone(initial_forks),
markers,
include: Arc::clone(include),
exclude: Arc::clone(exclude),
};
ResolverEnvironment { kind }
}
}
}

/// Returns a new resolver environment with the given groups excluded from
/// it.
/// Returns a new resolver environment with the given groups included or
/// excluded from it. An `Ok` variant indicates an include rule while an
/// `Err` variant indicates en exclude rule.
///
/// When a group is excluded from a resolver environment,
/// `ResolverEnvironment::included_by_group` will return false. The idea
/// is that a dependency with a corresponding group should be excluded by
/// forks in the resolver with this environment.
/// forks in the resolver with this environment. (Include rules have no
/// effect in `included_by_group` since, for the purposes of conflicts
/// during resolution, we only care about what *isn't* allowed.)
///
/// If calling this routine results in the same conflict item being both
/// included and excluded, then this returns `None` (since it would
/// otherwise result in a fork that can never be satisfied).
///
/// # Panics
///
/// This panics if the resolver environment corresponds to one and only one
/// specific marker environment. i.e., "pip"-style resolution.
pub(crate) fn exclude_by_group(
pub(crate) fn filter_by_group(
&self,
items: impl IntoIterator<Item = ConflictItem>,
) -> ResolverEnvironment {
rules: impl IntoIterator<Item = Result<ConflictItem, ConflictItem>>,
) -> Option<ResolverEnvironment> {
match self.kind {
Kind::Specific { .. } => {
unreachable!("environment narrowing only happens in universal resolution")
}
Kind::Universal {
ref initial_forks,
ref markers,
ref include,
ref exclude,
} => {
let mut include: crate::FxHashbrownSet<_> = (**include).clone();
let mut exclude: crate::FxHashbrownSet<_> = (**exclude).clone();
for item in items {
exclude.insert(item);
for rule in rules {
match rule {
Ok(item) => {
if exclude.contains(&item) {
return None;
}
include.insert(item);
}
Err(item) => {
if include.contains(&item) {
return None;
}
exclude.insert(item);
}
}
}
let kind = Kind::Universal {
initial_forks: Arc::clone(initial_forks),
markers: markers.clone(),
include: Arc::new(include),
exclude: Arc::new(exclude),
};
ResolverEnvironment { kind }
Some(ResolverEnvironment { kind })
}
}
}
Expand All @@ -266,6 +306,7 @@ impl ResolverEnvironment {
let Kind::Universal {
ref initial_forks,
markers: ref _markers,
include: ref _include,
exclude: ref _exclude,
} = self.kind
else {
Expand Down Expand Up @@ -332,9 +373,32 @@ impl ResolverEnvironment {
pub(crate) fn try_universal_markers(&self) -> Option<UniversalMarker> {
match self.kind {
Kind::Specific { .. } => None,
Kind::Universal { ref markers, .. } => {
// FIXME: Support conflicts.
Some(UniversalMarker::new(markers.clone(), MarkerTree::TRUE))
Kind::Universal {
ref markers,
ref include,
ref exclude,
..
} => {
let mut conflict_marker = MarkerTree::TRUE;
for item in exclude.iter() {
if let Some(extra) = item.extra() {
let operator = uv_pep508::ExtraOperator::NotEqual;
let name = uv_pep508::MarkerValueExtra::Extra(extra.clone());
let expr = uv_pep508::MarkerExpression::Extra { operator, name };
let exclude_extra_marker = MarkerTree::expression(expr);
conflict_marker.and(exclude_extra_marker);
}
}
for item in include.iter() {
if let Some(extra) = item.extra() {
let operator = uv_pep508::ExtraOperator::Equal;
let name = uv_pep508::MarkerValueExtra::Extra(extra.clone());
let expr = uv_pep508::MarkerExpression::Extra { operator, name };
let exclude_extra_marker = MarkerTree::expression(expr);
conflict_marker.and(exclude_extra_marker);
}
}
Some(UniversalMarker::new(markers.clone(), conflict_marker))
}
}
}
Expand Down
39 changes: 24 additions & 15 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2941,11 +2941,9 @@ impl Forks {
}

// Create a fork that excludes ALL extras.
let mut fork_none = fork.clone();
for group in set.iter() {
fork_none = fork_none.exclude([group.clone()]);
if let Some(fork_none) = fork.clone().filter(set.iter().cloned().map(Err)) {
new.push(fork_none);
}
new.push(fork_none);

// Now create a fork for each conflicting group, where
// that fork excludes every *other* conflicting group.
Expand All @@ -2955,13 +2953,18 @@ impl Forks {
// {foo, bar}, one that excludes {foo, baz} and one
// that excludes {bar, baz}.
for (i, _) in set.iter().enumerate() {
let fork_allows_group = fork.clone().exclude(
set.iter()
.enumerate()
.filter(|&(j, _)| i != j)
.map(|(_, group)| group.clone()),
);
new.push(fork_allows_group);
let fork_allows_group =
fork.clone()
.filter(set.iter().cloned().enumerate().map(|(j, group)| {
if i == j {
Ok(group)
} else {
Err(group)
}
}));
if let Some(fork_allows_group) = fork_allows_group {
new.push(fork_allows_group);
}
}
}
forks = new;
Expand Down Expand Up @@ -3058,11 +3061,17 @@ impl Fork {
self.conflicts.contains(&item)
}

/// Exclude the given groups from this fork.
/// Include or Exclude the given groups from this fork.
///
/// This removes all dependencies matching the given conflicting groups.
fn exclude(mut self, groups: impl IntoIterator<Item = ConflictItem>) -> Fork {
self.env = self.env.exclude_by_group(groups);
///
/// If the exclusion rules would result in a fork with an unsatisfiable
/// resolver environment, then this returns `None`.
fn filter(
mut self,
rules: impl IntoIterator<Item = Result<ConflictItem, ConflictItem>>,
) -> Option<Fork> {
self.env = self.env.filter_by_group(rules)?;
self.dependencies.retain(|dep| {
let Some(conflicting_item) = dep.package.conflicting_item() else {
return true;
Expand All @@ -3075,7 +3084,7 @@ impl Fork {
}
false
});
self
Some(self)
}
}

Expand Down

0 comments on commit 436fed0

Please sign in to comment.