Skip to content

Commit

Permalink
Remove python_version from lowered marker representation (#9343)
Browse files Browse the repository at this point in the history
## Summary

We never construct these -- they should be impossible, since we always
translate to `python_full_version`. This PR encodes that impossibility
in the types.
  • Loading branch information
charliermarsh authored Nov 26, 2024
1 parent df844e1 commit d187535
Show file tree
Hide file tree
Showing 6 changed files with 39 additions and 105 deletions.
18 changes: 1 addition & 17 deletions crates/uv-pep508/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#![warn(missing_docs)]

use std::collections::HashSet;
use std::error::Error;
use std::fmt::{Debug, Display, Formatter};
use std::path::Path;
Expand All @@ -41,7 +40,7 @@ pub use uv_normalize::{ExtraName, InvalidNameError, PackageName};
/// Version and version specifiers used in requirements (reexport).
// https://github.com/konstin/pep508_rs/issues/19
pub use uv_pep440;
use uv_pep440::{Version, VersionSpecifier, VersionSpecifiers};
use uv_pep440::{VersionSpecifier, VersionSpecifiers};
pub use verbatim_url::{
expand_env_vars, split_scheme, strip_host, Scheme, VerbatimUrl, VerbatimUrlError,
};
Expand Down Expand Up @@ -207,21 +206,6 @@ impl<T: Pep508Url> Requirement<T> {
self.marker.evaluate(env, extras)
}

/// Returns whether the requirement would be satisfied, independent of environment markers, i.e.
/// if there is potentially an environment that could activate this requirement.
///
/// Note that unlike [`Self::evaluate_markers`] this does not perform any checks for bogus
/// expressions but will simply return true. As caller you should separately perform a check
/// with an environment and forward all warnings.
pub fn evaluate_extras_and_python_version(
&self,
extras: &HashSet<ExtraName>,
python_versions: &[Version],
) -> bool {
self.marker
.evaluate_extras_and_python_version(extras, python_versions)
}

/// Return the requirement with an additional marker added, to require the given extra.
///
/// For example, given `flask >= 2.0.2`, calling `with_extra_marker("dotenv")` would return
Expand Down
64 changes: 37 additions & 27 deletions crates/uv-pep508/src/marker/algebra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,44 +158,54 @@ impl InternerGuard<'_> {
/// Returns a decision node for a single marker expression.
pub(crate) fn expression(&mut self, expr: MarkerExpression) -> NodeId {
let (var, children) = match expr {
// Normalize `python_version` markers to `python_full_version` nodes.
MarkerExpression::Version {
key: MarkerValueVersion::PythonVersion,
specifier,
} => match python_version_to_full_version(normalize_specifier(specifier)) {
Ok(specifier) => (
Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
// A variable representing the output of a version key. Edges correspond
// to disjoint version ranges.
MarkerExpression::Version { key, specifier } => match key {
MarkerValueVersion::ImplementationVersion => (
Variable::Version(LoweredMarkerValueVersion::ImplementationVersion),
Edges::from_specifier(specifier),
),
Err(node) => return node,
},
MarkerExpression::VersionIn {
key: MarkerValueVersion::PythonVersion,
versions,
negated,
} => match Edges::from_python_versions(versions, negated) {
Ok(edges) => (
MarkerValueVersion::PythonFullVersion => (
Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
edges,
Edges::from_specifier(specifier),
),
Err(node) => return node,
// Normalize `python_version` markers to `python_full_version` nodes.
MarkerValueVersion::PythonVersion => {
match python_version_to_full_version(normalize_specifier(specifier)) {
Ok(specifier) => (
Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
Edges::from_specifier(specifier),
),
Err(node) => return node,
}
}
},
// A variable representing the output of a version key. Edges correspond
// to disjoint version ranges.
MarkerExpression::Version { key, specifier } => (
Variable::Version(key.into()),
Edges::from_specifier(specifier),
),
// A variable representing the output of a version key. Edges correspond
// to disjoint version ranges.
MarkerExpression::VersionIn {
key,
versions,
negated,
} => (
Variable::Version(key.into()),
Edges::from_versions(&versions, negated),
),
} => match key {
MarkerValueVersion::ImplementationVersion => (
Variable::Version(LoweredMarkerValueVersion::ImplementationVersion),
Edges::from_versions(&versions, negated),
),
MarkerValueVersion::PythonFullVersion => (
Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
Edges::from_versions(&versions, negated),
),
// Normalize `python_version` markers to `python_full_version` nodes.
MarkerValueVersion::PythonVersion => {
match Edges::from_python_versions(versions, negated) {
Ok(edges) => (
Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
edges,
),
Err(node) => return node,
}
}
},
// The `in` and `contains` operators are a bit different than other operators.
// In particular, they do not represent a particular value for the corresponding
// variable, and can overlap. For example, `'nux' in os_name` and `os_name == 'Linux'`
Expand Down
1 change: 0 additions & 1 deletion crates/uv-pep508/src/marker/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ impl MarkerEnvironment {
&self.implementation_version().version
}
LoweredMarkerValueVersion::PythonFullVersion => &self.python_full_version().version,
LoweredMarkerValueVersion::PythonVersion => &self.python_version().version,
}
}

Expand Down
14 changes: 0 additions & 14 deletions crates/uv-pep508/src/marker/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,26 +12,13 @@ pub enum LoweredMarkerValueVersion {
ImplementationVersion,
/// `python_full_version`
PythonFullVersion,
/// `python_version`
PythonVersion,
}

impl Display for LoweredMarkerValueVersion {
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
match self {
Self::ImplementationVersion => f.write_str("implementation_version"),
Self::PythonFullVersion => f.write_str("python_full_version"),
Self::PythonVersion => f.write_str("python_version"),
}
}
}

impl From<MarkerValueVersion> for LoweredMarkerValueVersion {
fn from(value: MarkerValueVersion) -> Self {
match value {
MarkerValueVersion::ImplementationVersion => Self::ImplementationVersion,
MarkerValueVersion::PythonFullVersion => Self::PythonFullVersion,
MarkerValueVersion::PythonVersion => Self::PythonVersion,
}
}
}
Expand All @@ -41,7 +28,6 @@ impl From<LoweredMarkerValueVersion> for MarkerValueVersion {
match value {
LoweredMarkerValueVersion::ImplementationVersion => Self::ImplementationVersion,
LoweredMarkerValueVersion::PythonFullVersion => Self::PythonFullVersion,
LoweredMarkerValueVersion::PythonVersion => Self::PythonVersion,
}
}
}
Expand Down
44 changes: 0 additions & 44 deletions crates/uv-pep508/src/marker/tree.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use std::cmp::Ordering;
use std::collections::HashSet;
use std::fmt::{self, Display, Formatter};
use std::ops::{Bound, Deref};
use std::str::FromStr;
Expand Down Expand Up @@ -922,49 +921,6 @@ impl MarkerTree {
false
}

/// Checks if the requirement should be activated with the given set of active extras and a set
/// of possible python versions (from `requires-python`) without evaluating the remaining
/// environment markers, i.e. if there is potentially an environment that could activate this
/// requirement.
///
/// Note that unlike [`Self::evaluate`] this does not perform any checks for bogus expressions but
/// will simply return true. As caller you should separately perform a check with an environment
/// and forward all warnings.
pub fn evaluate_extras_and_python_version(
&self,
extras: &HashSet<ExtraName>,
python_versions: &[Version],
) -> bool {
match self.kind() {
MarkerTreeKind::True => true,
MarkerTreeKind::False => false,
MarkerTreeKind::Version(marker) => marker.edges().any(|(range, tree)| {
if marker.key() == LoweredMarkerValueVersion::PythonVersion {
if !python_versions
.iter()
.any(|version| range.contains(version))
{
return false;
}
}

tree.evaluate_extras_and_python_version(extras, python_versions)
}),
MarkerTreeKind::String(marker) => marker
.children()
.any(|(_, tree)| tree.evaluate_extras_and_python_version(extras, python_versions)),
MarkerTreeKind::In(marker) => marker
.children()
.any(|(_, tree)| tree.evaluate_extras_and_python_version(extras, python_versions)),
MarkerTreeKind::Contains(marker) => marker
.children()
.any(|(_, tree)| tree.evaluate_extras_and_python_version(extras, python_versions)),
MarkerTreeKind::Extra(marker) => marker
.edge(extras.contains(marker.name().extra()))
.evaluate_extras_and_python_version(extras, python_versions),
}
}

/// Checks if the requirement should be activated with the given set of active extras without evaluating
/// the remaining environment markers, i.e. if there is potentially an environment that could activate this
/// requirement.
Expand Down
3 changes: 1 addition & 2 deletions crates/uv-resolver/src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ pub(crate) fn requires_python(tree: &MarkerTree) -> Option<RequiresPythonRange>
match tree.kind() {
MarkerTreeKind::True | MarkerTreeKind::False => {}
MarkerTreeKind::Version(marker) => match marker.key() {
LoweredMarkerValueVersion::PythonVersion
| LoweredMarkerValueVersion::PythonFullVersion => {
LoweredMarkerValueVersion::PythonFullVersion => {
for (range, tree) in marker.edges() {
if !tree.is_false() {
markers.push(range.clone());
Expand Down

0 comments on commit d187535

Please sign in to comment.