uv-pep508: add MarkerTree::implies

I think Ibraheem had this routine at some point in the past, but
we ended up dropping it because we didn't have a use for it. Well,
now we do!

It turns out that when we generate "conflict markers," they don't
actually take "world knowledge" into account. In particular, there
is "world knowledge" that a particular set of extras cannot be
enabled simultaneously. This in turn allows us to simplify most
conflict markers. If we didn't do this, it's likely that lock files
would become littered with conflict markers whenever any conflicts
are declared.

This is somewhat (although not completely) analogous to how we
"simplify" markers with respect to `requires-python`. That is,
`requires-python` reflects world knowledge that enables markers
to be written more simply than they otherwise would be without
world knowledge.
This commit is contained in:
Andrew Gallant 2024-11-22 11:56:16 -05:00 committed by Andrew Gallant
parent 5cdac563e5
commit 38faae3d07

View file

@ -702,6 +702,19 @@ impl MarkerTree {
self.0 = INTERNER.lock().or(self.0, tree.0);
}
/// Sets this to a marker equivalent to the implication of this one and the
/// given consequent.
///
/// If the marker set is always `true`, then it can be said that `self`
/// implies `consequent`.
#[allow(clippy::needless_pass_by_value)]
pub fn implies(&mut self, consequent: MarkerTree) {
// This could probably be optimized, but is clearly
// correct, since logical implication is `-P or Q`.
*self = self.negate();
self.or(consequent);
}
/// Returns `true` if there is no environment in which both marker trees can apply,
/// i.e. their conjunction is always `false`.
///
@ -2484,6 +2497,47 @@ mod test {
);
}
/// This tests marker implication.
///
/// Specifically, these test cases come from a [bug] where `foo` and `bar`
/// are conflicting extras, but results in an ambiguous lock file. This
/// test takes `not(extra == 'foo' and extra == 'bar')` as world knowledge.
/// That is, since they are declared as conflicting, they cannot both be
/// true. This is in turn used to determine whether particular markers are
/// implied by this world knowledge and thus can be removed.
///
/// [bug]: <https://github.com/astral-sh/uv/issues/9289>
#[test]
fn test_extra_implication() {
assert!(implies(
"extra != 'foo' or extra != 'bar'",
"extra != 'foo' or extra != 'bar'",
));
assert!(!implies(
"extra != 'foo' or extra != 'bar'",
"extra != 'foo'",
));
assert!(!implies(
"extra != 'foo' or extra != 'bar'",
"extra != 'bar' and extra == 'foo'",
));
// This simulates the case when multiple groups of conflicts
// are combined into one "world knowledge" marker.
assert!(implies(
"(extra != 'foo' or extra != 'bar') and (extra != 'baz' or extra != 'quux')",
"extra != 'foo' or extra != 'bar'",
));
assert!(!implies(
"(extra != 'foo' or extra != 'bar') and (extra != 'baz' or extra != 'quux')",
"extra != 'foo'",
));
assert!(!implies(
"(extra != 'foo' or extra != 'bar') and (extra != 'baz' or extra != 'quux')",
"extra != 'bar' and extra == 'foo'",
));
}
#[test]
fn test_marker_negation() {
assert_eq!(
@ -2951,6 +3005,12 @@ mod test {
left.is_disjoint(&right) && right.is_disjoint(&left)
}
fn implies(antecedent: &str, consequent: &str) -> bool {
let mut marker = m(antecedent);
marker.implies(m(consequent));
marker.is_true()
}
#[test]
fn complexified_markers() {
// Takes optional lower (inclusive) and upper (exclusive)