uv-resolver: add "include" rules to `ResolverEnvironment`

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]: https://github.com/astral-sh/uv/issues/9289
This commit is contained in:
Andrew Gallant 2024-11-22 11:58:47 -05:00 committed by Andrew Gallant
parent 38faae3d07
commit f7bed37a4e
2 changed files with 100 additions and 27 deletions

View File

@ -95,6 +95,20 @@ enum Kind {
initial_forks: Arc<[MarkerTree]>, initial_forks: Arc<[MarkerTree]>,
/// The markers associated with this resolver fork. /// The markers associated with this resolver fork.
markers: MarkerTree, 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. /// Conflicting group exclusions.
exclude: Arc<crate::FxHashbrownSet<ConflictItem>>, exclude: Arc<crate::FxHashbrownSet<ConflictItem>>,
}, },
@ -134,6 +148,7 @@ impl ResolverEnvironment {
let kind = Kind::Universal { let kind = Kind::Universal {
initial_forks: initial_forks.into(), initial_forks: initial_forks.into(),
markers: MarkerTree::TRUE, markers: MarkerTree::TRUE,
include: Arc::new(crate::FxHashbrownSet::default()),
exclude: Arc::new(crate::FxHashbrownSet::default()), exclude: Arc::new(crate::FxHashbrownSet::default()),
}; };
ResolverEnvironment { kind } ResolverEnvironment { kind }
@ -201,6 +216,7 @@ impl ResolverEnvironment {
Kind::Universal { Kind::Universal {
ref initial_forks, ref initial_forks,
markers: ref lhs, markers: ref lhs,
ref include,
ref exclude, ref exclude,
} => { } => {
let mut markers = lhs.clone(); let mut markers = lhs.clone();
@ -208,6 +224,7 @@ impl ResolverEnvironment {
let kind = Kind::Universal { let kind = Kind::Universal {
initial_forks: Arc::clone(initial_forks), initial_forks: Arc::clone(initial_forks),
markers, markers,
include: Arc::clone(include),
exclude: Arc::clone(exclude), exclude: Arc::clone(exclude),
}; };
ResolverEnvironment { kind } ResolverEnvironment { kind }
@ -215,22 +232,29 @@ impl ResolverEnvironment {
} }
} }
/// Returns a new resolver environment with the given groups excluded from /// Returns a new resolver environment with the given groups included or
/// it. /// 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, /// When a group is excluded from a resolver environment,
/// `ResolverEnvironment::included_by_group` will return false. The idea /// `ResolverEnvironment::included_by_group` will return false. The idea
/// is that a dependency with a corresponding group should be excluded by /// 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 /// # Panics
/// ///
/// This panics if the resolver environment corresponds to one and only one /// This panics if the resolver environment corresponds to one and only one
/// specific marker environment. i.e., "pip"-style resolution. /// specific marker environment. i.e., "pip"-style resolution.
pub(crate) fn exclude_by_group( pub(crate) fn filter_by_group(
&self, &self,
items: impl IntoIterator<Item = ConflictItem>, rules: impl IntoIterator<Item = Result<ConflictItem, ConflictItem>>,
) -> ResolverEnvironment { ) -> Option<ResolverEnvironment> {
match self.kind { match self.kind {
Kind::Specific { .. } => { Kind::Specific { .. } => {
unreachable!("environment narrowing only happens in universal resolution") unreachable!("environment narrowing only happens in universal resolution")
@ -238,18 +262,34 @@ impl ResolverEnvironment {
Kind::Universal { Kind::Universal {
ref initial_forks, ref initial_forks,
ref markers, ref markers,
ref include,
ref exclude, ref exclude,
} => { } => {
let mut include: crate::FxHashbrownSet<_> = (**include).clone();
let mut exclude: crate::FxHashbrownSet<_> = (**exclude).clone(); let mut exclude: crate::FxHashbrownSet<_> = (**exclude).clone();
for item in items { for rule in rules {
exclude.insert(item); 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 { let kind = Kind::Universal {
initial_forks: Arc::clone(initial_forks), initial_forks: Arc::clone(initial_forks),
markers: markers.clone(), markers: markers.clone(),
include: Arc::new(include),
exclude: Arc::new(exclude), exclude: Arc::new(exclude),
}; };
ResolverEnvironment { kind } Some(ResolverEnvironment { kind })
} }
} }
} }
@ -266,6 +306,7 @@ impl ResolverEnvironment {
let Kind::Universal { let Kind::Universal {
ref initial_forks, ref initial_forks,
markers: ref _markers, markers: ref _markers,
include: ref _include,
exclude: ref _exclude, exclude: ref _exclude,
} = self.kind } = self.kind
else { else {
@ -332,9 +373,32 @@ impl ResolverEnvironment {
pub(crate) fn try_universal_markers(&self) -> Option<UniversalMarker> { pub(crate) fn try_universal_markers(&self) -> Option<UniversalMarker> {
match self.kind { match self.kind {
Kind::Specific { .. } => None, Kind::Specific { .. } => None,
Kind::Universal { ref markers, .. } => { Kind::Universal {
// FIXME: Support conflicts. ref markers,
Some(UniversalMarker::new(markers.clone(), MarkerTree::TRUE)) 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))
} }
} }
} }

View File

@ -2941,11 +2941,9 @@ impl Forks {
} }
// Create a fork that excludes ALL extras. // Create a fork that excludes ALL extras.
let mut fork_none = fork.clone(); if let Some(fork_none) = fork.clone().filter(set.iter().cloned().map(Err)) {
for group in set.iter() { new.push(fork_none);
fork_none = fork_none.exclude([group.clone()]);
} }
new.push(fork_none);
// Now create a fork for each conflicting group, where // Now create a fork for each conflicting group, where
// that fork excludes every *other* conflicting group. // that fork excludes every *other* conflicting group.
@ -2955,13 +2953,18 @@ impl Forks {
// {foo, bar}, one that excludes {foo, baz} and one // {foo, bar}, one that excludes {foo, baz} and one
// that excludes {bar, baz}. // that excludes {bar, baz}.
for (i, _) in set.iter().enumerate() { for (i, _) in set.iter().enumerate() {
let fork_allows_group = fork.clone().exclude( let fork_allows_group =
set.iter() fork.clone()
.enumerate() .filter(set.iter().cloned().enumerate().map(|(j, group)| {
.filter(|&(j, _)| i != j) if i == j {
.map(|(_, group)| group.clone()), Ok(group)
); } else {
new.push(fork_allows_group); Err(group)
}
}));
if let Some(fork_allows_group) = fork_allows_group {
new.push(fork_allows_group);
}
} }
} }
forks = new; forks = new;
@ -3058,11 +3061,17 @@ impl Fork {
self.conflicts.contains(&item) 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. /// 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| { self.dependencies.retain(|dep| {
let Some(conflicting_item) = dep.package.conflicting_item() else { let Some(conflicting_item) = dep.package.conflicting_item() else {
return true; return true;
@ -3075,7 +3084,7 @@ impl Fork {
} }
false false
}); });
self Some(self)
} }
} }