Remove `python_version` from lowered marker representation (#9343)

## 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.
This commit is contained in:
Charlie Marsh 2024-11-26 09:39:36 -05:00 committed by GitHub
parent df844e1ec9
commit d18753527f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 40 additions and 106 deletions

View File

@ -16,7 +16,6 @@
#![warn(missing_docs)] #![warn(missing_docs)]
use std::collections::HashSet;
use std::error::Error; use std::error::Error;
use std::fmt::{Debug, Display, Formatter}; use std::fmt::{Debug, Display, Formatter};
use std::path::Path; use std::path::Path;
@ -41,7 +40,7 @@ pub use uv_normalize::{ExtraName, InvalidNameError, PackageName};
/// Version and version specifiers used in requirements (reexport). /// Version and version specifiers used in requirements (reexport).
// https://github.com/konstin/pep508_rs/issues/19 // https://github.com/konstin/pep508_rs/issues/19
pub use uv_pep440; pub use uv_pep440;
use uv_pep440::{Version, VersionSpecifier, VersionSpecifiers}; use uv_pep440::{VersionSpecifier, VersionSpecifiers};
pub use verbatim_url::{ pub use verbatim_url::{
expand_env_vars, split_scheme, strip_host, Scheme, VerbatimUrl, VerbatimUrlError, expand_env_vars, split_scheme, strip_host, Scheme, VerbatimUrl, VerbatimUrlError,
}; };
@ -207,21 +206,6 @@ impl<T: Pep508Url> Requirement<T> {
self.marker.evaluate(env, extras) 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. /// 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 /// For example, given `flask >= 2.0.2`, calling `with_extra_marker("dotenv")` would return

View File

@ -158,44 +158,54 @@ impl InternerGuard<'_> {
/// Returns a decision node for a single marker expression. /// Returns a decision node for a single marker expression.
pub(crate) fn expression(&mut self, expr: MarkerExpression) -> NodeId { pub(crate) fn expression(&mut self, expr: MarkerExpression) -> NodeId {
let (var, children) = match expr { let (var, children) = match expr {
// 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),
),
MarkerValueVersion::PythonFullVersion => (
Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
Edges::from_specifier(specifier),
),
// Normalize `python_version` markers to `python_full_version` nodes. // Normalize `python_version` markers to `python_full_version` nodes.
MarkerExpression::Version { MarkerValueVersion::PythonVersion => {
key: MarkerValueVersion::PythonVersion, match python_version_to_full_version(normalize_specifier(specifier)) {
specifier,
} => match python_version_to_full_version(normalize_specifier(specifier)) {
Ok(specifier) => ( Ok(specifier) => (
Variable::Version(LoweredMarkerValueVersion::PythonFullVersion), Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
Edges::from_specifier(specifier), Edges::from_specifier(specifier),
), ),
Err(node) => return node, Err(node) => return node,
}
}
}, },
MarkerExpression::VersionIn {
key: MarkerValueVersion::PythonVersion,
versions,
negated,
} => match Edges::from_python_versions(versions, negated) {
Ok(edges) => (
Variable::Version(LoweredMarkerValueVersion::PythonFullVersion),
edges,
),
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 // A variable representing the output of a version key. Edges correspond
// to disjoint version ranges. // to disjoint version ranges.
MarkerExpression::VersionIn { MarkerExpression::VersionIn {
key, key,
versions, versions,
negated, negated,
} => ( } => match key {
Variable::Version(key.into()), MarkerValueVersion::ImplementationVersion => (
Variable::Version(LoweredMarkerValueVersion::ImplementationVersion),
Edges::from_versions(&versions, negated), 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. // The `in` and `contains` operators are a bit different than other operators.
// In particular, they do not represent a particular value for the corresponding // 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'` // variable, and can overlap. For example, `'nux' in os_name` and `os_name == 'Linux'`

View File

@ -39,7 +39,6 @@ impl MarkerEnvironment {
&self.implementation_version().version &self.implementation_version().version
} }
LoweredMarkerValueVersion::PythonFullVersion => &self.python_full_version().version, LoweredMarkerValueVersion::PythonFullVersion => &self.python_full_version().version,
LoweredMarkerValueVersion::PythonVersion => &self.python_version().version,
} }
} }

View File

@ -12,8 +12,6 @@ pub enum LoweredMarkerValueVersion {
ImplementationVersion, ImplementationVersion,
/// `python_full_version` /// `python_full_version`
PythonFullVersion, PythonFullVersion,
/// `python_version`
PythonVersion,
} }
impl Display for LoweredMarkerValueVersion { impl Display for LoweredMarkerValueVersion {
@ -21,17 +19,6 @@ impl Display for LoweredMarkerValueVersion {
match self { match self {
Self::ImplementationVersion => f.write_str("implementation_version"), Self::ImplementationVersion => f.write_str("implementation_version"),
Self::PythonFullVersion => f.write_str("python_full_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,
} }
} }
} }
@ -41,7 +28,6 @@ impl From<LoweredMarkerValueVersion> for MarkerValueVersion {
match value { match value {
LoweredMarkerValueVersion::ImplementationVersion => Self::ImplementationVersion, LoweredMarkerValueVersion::ImplementationVersion => Self::ImplementationVersion,
LoweredMarkerValueVersion::PythonFullVersion => Self::PythonFullVersion, LoweredMarkerValueVersion::PythonFullVersion => Self::PythonFullVersion,
LoweredMarkerValueVersion::PythonVersion => Self::PythonVersion,
} }
} }
} }

View File

@ -1,5 +1,4 @@
use std::cmp::Ordering; use std::cmp::Ordering;
use std::collections::HashSet;
use std::fmt::{self, Display, Formatter}; use std::fmt::{self, Display, Formatter};
use std::ops::{Bound, Deref}; use std::ops::{Bound, Deref};
use std::str::FromStr; use std::str::FromStr;
@ -922,49 +921,6 @@ impl MarkerTree {
false 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 /// 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 /// the remaining environment markers, i.e. if there is potentially an environment that could activate this
/// requirement. /// requirement.

View File

@ -10,8 +10,7 @@ pub(crate) fn requires_python(tree: &MarkerTree) -> Option<RequiresPythonRange>
match tree.kind() { match tree.kind() {
MarkerTreeKind::True | MarkerTreeKind::False => {} MarkerTreeKind::True | MarkerTreeKind::False => {}
MarkerTreeKind::Version(marker) => match marker.key() { MarkerTreeKind::Version(marker) => match marker.key() {
LoweredMarkerValueVersion::PythonVersion LoweredMarkerValueVersion::PythonFullVersion => {
| LoweredMarkerValueVersion::PythonFullVersion => {
for (range, tree) in marker.edges() { for (range, tree) in marker.edges() {
if !tree.is_false() { if !tree.is_false() {
markers.push(range.clone()); markers.push(range.clone());