uv-resolver: use Requires-Python to filter dependencies during universal resolution

[BurntSushi]

In the time before universal resolving, we would always pass a
MarkerEnvironment, and this environment would capture any relevant
Requires-Python specifier (including if -p/--python was provided on
the CLI).

But in universal resolution, we very specifically do not use a
MarkerEnvironment because we want to produce a resolution that is
compatible across potentially multiple environments. This in turn meant
that we lost Requires-Python filtering.

This PR adds it back. We do this by converting our PythonRequirement
into a MarkerTree that encodes the version specifiers in a
Requires-Python specifier. We then ask whether that MarkerTree is
disjoint with a dependency specification's MarkerTree. If it is, then
we know it's impossible for that dependency specification to every be
true, and we can completely ignore it.

Packse tests were added in this PR: astral-sh/packse#187

[charliermarsh]

Should this evaluate to None if requires_python is empty? What's the thinking behind MarkerTree::And(vec![])?

[BurntSushi]

I believe it would be correct for this to be None. But that's only because we treat None and "marker expression is always true" as equivalent. It just makes the contract a little nicer I guess:

    /// This is derived from `PythonRequirement` once at initialization
    /// time. It's used in universal mode to filter our dependencies with
    /// a `python_version` marker expression that has no overlap with the
    /// `Requires-Python` specifier.
    ///
    /// This is non-None if and only if the resolver is operating in
    /// universal mode. (i.e., when `markers` is `None`.)
    requires_python: Option<MarkerTree>,

If we used None here then this would need to be rephrased.

I don't have a strong opinion here or anything, but this just made the most sense to me at the time.

[BurntSushi]

(I sometimes wonder if we could get away with not having Option<MarkerTree> anywhere.)

[charliermarsh]

I guess I was hoping to avoid ever allowing And(vec![]) or Or(vec![]). Ideally that would be impossible in the future...

[BurntSushi]

Yeah I had similar hopes for regex initially (e.g., banning "" and [^\s\S]), and it just turns out that it's useful to have types like this be able to represent "always match" and "never match." For example, you might be able to ban And(vec![]), but 1) you'll have to juggle it with Option<MarkerTree> which is annoying and 2) there an infinite number of MarkerTree values that have the same truth table as And(vec![]). Because of (2), you kinda need to deal with "always true" (and also, "never true") expressions anyway. So you might as well allow the trivial examples of them.

[charliermarsh]

I'm having trouble reasoning about whether we should be using both of these (i.e., both python_version and python_full_version).

[charliermarsh]

Trying to talk it out... Let's say you have Requires-Python = ">= 3.10.1"...

Then we'd add python_version >= "3.10.1" and python_full_version >= "3.10.1".

Then let's say we see a marker on a dependency python_version == "3.10".

That actually does intersect with Requires-Python = ">= 3.10.1", because by definition in python_version we only capture the major and minor (it's computed as: '.'.join(platform.python_version_tuple()[:2])). But here we would consider it disjoint from python_version >= "3.10.1".

So, really, we want python_version >= "3.10", not python_version >= "3.10.1"?

[charliermarsh]

Ok, to simplify things, I might suggest this: use self.bound rather than self.specifiers. And add >= self.bound, and only include the major and minor release for the MarkerValueVersion::PythonVersion segment.

[BurntSushi]

The way I see it is if you have foo ; python_full_version == '3.9' and Requires-Python: >=3.10, then foo should be ignored.

That work by transforming Requires-Python: >=3.10 to python_version >= '3.10' and python_full_version >= '3.10'. That marker expression in turn has zero overlap with python_full_version == '3.9', so it is excluded. But if we instead had bar ; python_full_version == '3.11', then there is possible overlap and thus they aren't considered disjoint.

[BurntSushi]

(Oops, when I wrote the above comment, I didn't see your two most recent comments. The GitHub UI is maybe slow to update.)

[charliermarsh]

Yeah, the basic problem is that python_version is definitionally truncated, and I worry that could lead to incorrect reasoning here.

[BurntSushi]

Yeah I'm going to use your example above to write a regression test.

[BurntSushi]

All righty, this should be fixed now! I added a regression test here: astral-sh/packse#193

[charliermarsh]

Why can this one unwrap?

[BurntSushi]

Same reason as the one above. I duplicated the comment to make this clearer.

Although I suppose the constructor of RequiresPython permits any Version which means it could technically have a local segment. Likely a logic error, but I added without_local() to strip it from the version anyway.