diff --git a/crates/uv-distribution-types/src/requirement.rs b/crates/uv-distribution-types/src/requirement.rs index d97b86864..1680e6dcf 100644 --- a/crates/uv-distribution-types/src/requirement.rs +++ b/crates/uv-distribution-types/src/requirement.rs @@ -11,7 +11,8 @@ use uv_git_types::{GitOid, GitReference, GitUrl, GitUrlParseError, OidParseError use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_pep440::VersionSpecifiers; use uv_pep508::{ - MarkerEnvironment, MarkerTree, RequirementOrigin, VerbatimUrl, VersionOrUrl, marker, + MarkerEnvironment, MarkerTree, MarkerVariantsEnvironment, RequirementOrigin, VerbatimUrl, + VersionOrUrl, marker, }; use uv_redacted::DisplaySafeUrl; @@ -72,7 +73,7 @@ impl Requirement { pub fn evaluate_markers( &self, env: Option<&MarkerEnvironment>, - variants: Option<&[(String, String, String)]>, + variants: impl MarkerVariantsEnvironment, extras: &[ExtraName], ) -> bool { self.marker diff --git a/crates/uv-distribution-types/src/specified_requirement.rs b/crates/uv-distribution-types/src/specified_requirement.rs index 3fe52f913..80fc834c5 100644 --- a/crates/uv-distribution-types/src/specified_requirement.rs +++ b/crates/uv-distribution-types/src/specified_requirement.rs @@ -2,7 +2,7 @@ use std::borrow::Cow; use std::fmt::{Display, Formatter}; use uv_normalize::ExtraName; -use uv_pep508::{MarkerEnvironment, UnnamedRequirement}; +use uv_pep508::{MarkerEnvironment, MarkerVariantsEnvironment, UnnamedRequirement}; use uv_pypi_types::Hashes; use crate::{Requirement, RequirementSource, VerbatimParsedUrl}; @@ -62,7 +62,7 @@ impl UnresolvedRequirement { pub fn evaluate_markers( &self, env: Option<&MarkerEnvironment>, - variants: Option<&[(String, String, String)]>, + variants: impl MarkerVariantsEnvironment, extras: &[ExtraName], ) -> bool { match self { diff --git a/crates/uv-distribution/src/distribution_database.rs b/crates/uv-distribution/src/distribution_database.rs index f2fc40047..a13fc13e7 100644 --- a/crates/uv-distribution/src/distribution_database.rs +++ b/crates/uv-distribution/src/distribution_database.rs @@ -641,7 +641,7 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> { // TODO(konsti): We shouldn't need to do this conversion. let mut known_properties = BTreeSet::default(); for variant in variants_json.variants.values() { - for (namespace, features) in variant { + for (namespace, features) in &**variant { for (feature, value) in features { for value in value { known_properties.insert(VariantPropertyType { diff --git a/crates/uv-installer/src/site_packages.rs b/crates/uv-installer/src/site_packages.rs index a47b91bf6..fc681fcbf 100644 --- a/crates/uv-installer/src/site_packages.rs +++ b/crates/uv-installer/src/site_packages.rs @@ -14,7 +14,7 @@ use uv_distribution_types::{ use uv_fs::Simplified; use uv_normalize::PackageName; use uv_pep440::{Version, VersionSpecifiers}; -use uv_pep508::VersionOrUrl; +use uv_pep508::{MarkerVariantsUniversal, VersionOrUrl}; use uv_pypi_types::{ResolverMarkerEnvironment, VerbatimParsedUrl}; use uv_python::{Interpreter, PythonEnvironment}; use uv_redacted::DisplaySafeUrl; @@ -243,7 +243,7 @@ impl SitePackages { // Verify that the dependencies are installed. for dependency in &metadata.requires_dist { - if !dependency.evaluate_markers(markers, None, &[]) { + if !dependency.evaluate_markers(markers, MarkerVariantsUniversal, &[]) { continue; } @@ -416,7 +416,7 @@ impl SitePackages { for requirement in requirements { if let Some(r#overrides) = overrides.get(&requirement.name) { for dependency in r#overrides { - if dependency.evaluate_markers(Some(markers), None, &[]) { + if dependency.evaluate_markers(Some(markers), MarkerVariantsUniversal, &[]) { if seen.insert((*dependency).clone()) { stack.push(Cow::Borrowed(*dependency)); } @@ -424,7 +424,7 @@ impl SitePackages { } } else { // TODO(konsti): Evaluate variants - if requirement.evaluate_markers(Some(markers), None, &[]) { + if requirement.evaluate_markers(Some(markers), MarkerVariantsUniversal, &[]) { if seen.insert(requirement.clone()) { stack.push(Cow::Borrowed(requirement)); } @@ -443,7 +443,7 @@ impl SitePackages { } [distribution] => { // Validate that the requirement is satisfied. - if requirement.evaluate_markers(Some(markers), None, &[]) { + if requirement.evaluate_markers(Some(markers), MarkerVariantsUniversal, &[]) { match RequirementSatisfaction::check(distribution, &requirement.source) { RequirementSatisfaction::Mismatch | RequirementSatisfaction::OutOfDate @@ -456,7 +456,8 @@ impl SitePackages { // Validate that the installed version satisfies the constraints. for constraint in constraints.get(name).into_iter().flatten() { - if constraint.evaluate_markers(Some(markers), None, &[]) { + if constraint.evaluate_markers(Some(markers), MarkerVariantsUniversal, &[]) + { match RequirementSatisfaction::check(distribution, &constraint.source) { RequirementSatisfaction::Mismatch | RequirementSatisfaction::OutOfDate @@ -482,7 +483,7 @@ impl SitePackages { for dependency in r#overrides { if dependency.evaluate_markers( Some(markers), - None, + MarkerVariantsUniversal, &requirement.extras, ) { if seen.insert((*dependency).clone()) { @@ -491,8 +492,11 @@ impl SitePackages { } } } else { - if dependency.evaluate_markers(Some(markers), None, &requirement.extras) - { + if dependency.evaluate_markers( + Some(markers), + MarkerVariantsUniversal, + &requirement.extras, + ) { if seen.insert(dependency.clone()) { stack.push(Cow::Owned(dependency)); } diff --git a/crates/uv-pep508/src/lib.rs b/crates/uv-pep508/src/lib.rs index 7f0bc5b5b..bd9834a07 100644 --- a/crates/uv-pep508/src/lib.rs +++ b/crates/uv-pep508/src/lib.rs @@ -33,7 +33,8 @@ pub use marker::{ ContainsMarkerTree, ExtraMarkerTree, ExtraOperator, InMarkerTree, MarkerEnvironment, MarkerEnvironmentBuilder, MarkerExpression, MarkerOperator, MarkerTree, MarkerTreeContents, MarkerTreeKind, MarkerValue, MarkerValueExtra, MarkerValueList, MarkerValueString, - MarkerValueVersion, MarkerWarningKind, StringMarkerTree, StringVersion, VersionMarkerTree, + MarkerValueVersion, MarkerVariantsEnvironment, MarkerVariantsUniversal, MarkerWarningKind, + StringMarkerTree, StringVersion, VersionMarkerTree, }; pub use origin::RequirementOrigin; #[cfg(feature = "non-pep508-extensions")] @@ -267,7 +268,7 @@ impl Requirement { pub fn evaluate_markers( &self, env: &MarkerEnvironment, - variants: Option<&[(String, String, String)]>, + variants: impl MarkerVariantsEnvironment, extras: &[ExtraName], ) -> bool { self.marker.evaluate(env, variants, extras) diff --git a/crates/uv-pep508/src/marker/mod.rs b/crates/uv-pep508/src/marker/mod.rs index 55d21c69f..817dadd5a 100644 --- a/crates/uv-pep508/src/marker/mod.rs +++ b/crates/uv-pep508/src/marker/mod.rs @@ -24,7 +24,8 @@ pub use tree::{ ContainsMarkerTree, ExtraMarkerTree, ExtraOperator, InMarkerTree, MarkerExpression, MarkerOperator, MarkerTree, MarkerTreeContents, MarkerTreeDebugGraph, MarkerTreeKind, MarkerValue, MarkerValueExtra, MarkerValueList, MarkerValueString, MarkerValueVersion, - MarkerWarningKind, StringMarkerTree, StringVersion, VersionMarkerTree, + MarkerVariantsEnvironment, MarkerVariantsUniversal, MarkerWarningKind, StringMarkerTree, + StringVersion, VersionMarkerTree, }; /// `serde` helpers for [`MarkerTree`]. diff --git a/crates/uv-pep508/src/marker/tree.rs b/crates/uv-pep508/src/marker/tree.rs index 05cc90267..3e27869f8 100644 --- a/crates/uv-pep508/src/marker/tree.rs +++ b/crates/uv-pep508/src/marker/tree.rs @@ -773,6 +773,73 @@ impl<'a> ExtrasEnvironment<'a> { } } +/// The environment to use for evaluating `variant_namespaces`, `variant_features` and +/// `variant_properties` +pub trait MarkerVariantsEnvironment { + /// Whether there is a property with the given namespace, for evaluating `variant_namespaces`. + fn has_namespace(&self, namespace: &str) -> bool; + + /// Whether there is a property with the given feature, for evaluating `variant_features`. + fn has_feature(&self, namespace: &str, feature: &str) -> bool; + + /// Whether there is the given property, for evaluating `variant_properties`. + fn has_property(&self, namespace: &str, feature: &str, property: &str) -> bool; +} + +// Useful for tests +impl> MarkerVariantsEnvironment for &[(S, S, S)] { + fn has_namespace(&self, namespace: &str) -> bool { + self.iter() + .any(|(namespace_, _feature, _property)| namespace == namespace_.as_ref()) + } + + fn has_feature(&self, namespace: &str, feature: &str) -> bool { + self.iter().any(|(namespace_, feature_, _property)| { + namespace == namespace_.as_ref() && feature == feature_.as_ref() + }) + } + + fn has_property(&self, namespace: &str, feature: &str, property: &str) -> bool { + self.iter().any(|(namespace_, feature_, property_)| { + namespace == namespace_.as_ref() + && feature == feature_.as_ref() + && property == property_.as_ref() + }) + } +} + +// TODO(konsti): Use `AsRef` instead? +impl MarkerVariantsEnvironment for &T { + fn has_namespace(&self, namespace: &str) -> bool { + T::has_namespace(self, namespace) + } + + fn has_feature(&self, namespace: &str, feature: &str) -> bool { + T::has_feature(self, namespace, feature) + } + + fn has_property(&self, namespace: &str, feature: &str, property: &str) -> bool { + T::has_property(self, namespace, feature, property) + } +} + +/// A marker variants environment that always evaluates to `true`. +pub struct MarkerVariantsUniversal; + +impl MarkerVariantsEnvironment for MarkerVariantsUniversal { + fn has_namespace(&self, _namespace: &str) -> bool { + true + } + + fn has_feature(&self, _namespace: &str, _feature: &str) -> bool { + true + } + + fn has_property(&self, _namespace: &str, _feature: &str, _property: &str) -> bool { + true + } +} + /// Represents one or more nested marker expressions with and/or/parentheses. /// /// Marker trees are canonical, meaning any two functionally equivalent markers @@ -1012,7 +1079,7 @@ impl MarkerTree { pub fn evaluate( self, env: &MarkerEnvironment, - variants: Option<&[(String, String, String)]>, + variants: impl MarkerVariantsEnvironment, extras: &[ExtraName], ) -> bool { self.evaluate_reporter_impl( @@ -1029,7 +1096,7 @@ impl MarkerTree { pub fn evaluate_pep751( self, env: &MarkerEnvironment, - variants: Option<&[(String, String, String)]>, + variants: impl MarkerVariantsEnvironment, extras: &[ExtraName], groups: &[GroupName], ) -> bool { @@ -1051,7 +1118,7 @@ impl MarkerTree { pub fn evaluate_optional_environment( self, env: Option<&MarkerEnvironment>, - variants: Option<&[(String, String, String)]>, + variants: impl MarkerVariantsEnvironment, extras: &[ExtraName], ) -> bool { match env { @@ -1071,7 +1138,7 @@ impl MarkerTree { self, env: &MarkerEnvironment, extras: &[ExtraName], - variants: Option<&[(String, String, String)]>, + variants: impl MarkerVariantsEnvironment, reporter: &mut impl Reporter, ) -> bool { self.evaluate_reporter_impl( @@ -1086,7 +1153,7 @@ impl MarkerTree { self, env: &MarkerEnvironment, extras: ExtrasEnvironment, - variants: Option<&[(String, String, String)]>, + variants: impl MarkerVariantsEnvironment, reporter: &mut impl Reporter, ) -> bool { match self.kind() { @@ -1138,39 +1205,14 @@ impl MarkerTree { } MarkerTreeKind::List(marker) => { let edge = match marker.pair() { - CanonicalMarkerListPair::VariantNamespaces(marker_namespace) => { - let Some(variants) = variants else { - // If we're not limiting to specific variants, we're solving universally. - return true; - }; - - variants - .iter() - .any(|(namespace, _feature, _property)| namespace == marker_namespace) + CanonicalMarkerListPair::VariantNamespaces(namespace) => { + variants.has_namespace(namespace) } - CanonicalMarkerListPair::VariantFeatures(marker_namespace, marker_feature) => { - let Some(variants) = variants else { - return true; - }; - - variants.iter().any(|(namespace, feature, _property)| { - namespace == marker_namespace && feature == marker_feature - }) + CanonicalMarkerListPair::VariantFeatures(namespace, feature) => { + variants.has_feature(namespace, feature) } - CanonicalMarkerListPair::VariantProperties( - marker_namespace, - marker_feature, - marker_property, - ) => { - let Some(variants) = variants else { - return true; - }; - - variants.iter().any(|(namespace, feature, property)| { - namespace == marker_namespace - && feature == marker_feature - && property == marker_property - }) + CanonicalMarkerListPair::VariantProperties(namespace, feature, property) => { + variants.has_property(namespace, feature, property) } CanonicalMarkerListPair::Extras(extra) => extras.extras().contains(extra), CanonicalMarkerListPair::DependencyGroup(dependency_group) => { @@ -2057,7 +2099,9 @@ mod test { use uv_pep440::Version; use crate::marker::{MarkerEnvironment, MarkerEnvironmentBuilder}; - use crate::{MarkerExpression, MarkerOperator, MarkerTree, MarkerValueString}; + use crate::{ + MarkerExpression, MarkerOperator, MarkerTree, MarkerValueString, MarkerVariantsUniversal, + }; fn parse_err(input: &str) -> String { MarkerTree::from_str(input).unwrap_err().to_string() @@ -2214,12 +2258,12 @@ mod test { let marker3 = MarkerTree::from_str( "python_version == \"2.7\" and (sys_platform == \"win32\" or sys_platform == \"linux\")", ).unwrap(); - assert!(marker1.evaluate(&env27, None, &[])); - assert!(!marker1.evaluate(&env37, None, &[])); - assert!(marker2.evaluate(&env27, None, &[])); - assert!(marker2.evaluate(&env37, None, &[])); - assert!(marker3.evaluate(&env27, None, &[])); - assert!(!marker3.evaluate(&env37, None, &[])); + assert!(marker1.evaluate(&env27, MarkerVariantsUniversal, &[])); + assert!(!marker1.evaluate(&env37, MarkerVariantsUniversal, &[])); + assert!(marker2.evaluate(&env27, MarkerVariantsUniversal, &[])); + assert!(marker2.evaluate(&env37, MarkerVariantsUniversal, &[])); + assert!(marker3.evaluate(&env27, MarkerVariantsUniversal, &[])); + assert!(!marker3.evaluate(&env37, MarkerVariantsUniversal, &[])); } #[test] @@ -2241,48 +2285,48 @@ mod test { let env37 = env37(); let marker = MarkerTree::from_str("python_version in \"2.7 3.2 3.3\"").unwrap(); - assert!(marker.evaluate(&env27, None, &[])); - assert!(!marker.evaluate(&env37, None, &[])); + assert!(marker.evaluate(&env27, MarkerVariantsUniversal, &[])); + assert!(!marker.evaluate(&env37, MarkerVariantsUniversal, &[])); let marker = MarkerTree::from_str("python_version in \"2.7 3.7\"").unwrap(); - assert!(marker.evaluate(&env27, None, &[])); - assert!(marker.evaluate(&env37, None, &[])); + assert!(marker.evaluate(&env27, MarkerVariantsUniversal, &[])); + assert!(marker.evaluate(&env37, MarkerVariantsUniversal, &[])); let marker = MarkerTree::from_str("python_version in \"2.4 3.8 4.0\"").unwrap(); - assert!(!marker.evaluate(&env27, None, &[])); - assert!(!marker.evaluate(&env37, None, &[])); + assert!(!marker.evaluate(&env27, MarkerVariantsUniversal, &[])); + assert!(!marker.evaluate(&env37, MarkerVariantsUniversal, &[])); let marker = MarkerTree::from_str("python_version not in \"2.7 3.2 3.3\"").unwrap(); - assert!(!marker.evaluate(&env27, None, &[])); - assert!(marker.evaluate(&env37, None, &[])); + assert!(!marker.evaluate(&env27, MarkerVariantsUniversal, &[])); + assert!(marker.evaluate(&env37, MarkerVariantsUniversal, &[])); let marker = MarkerTree::from_str("python_version not in \"2.7 3.7\"").unwrap(); - assert!(!marker.evaluate(&env27, None, &[])); - assert!(!marker.evaluate(&env37, None, &[])); + assert!(!marker.evaluate(&env27, MarkerVariantsUniversal, &[])); + assert!(!marker.evaluate(&env37, MarkerVariantsUniversal, &[])); let marker = MarkerTree::from_str("python_version not in \"2.4 3.8 4.0\"").unwrap(); - assert!(marker.evaluate(&env27, None, &[])); - assert!(marker.evaluate(&env37, None, &[])); + assert!(marker.evaluate(&env27, MarkerVariantsUniversal, &[])); + assert!(marker.evaluate(&env37, MarkerVariantsUniversal, &[])); let marker = MarkerTree::from_str("python_full_version in \"2.7\"").unwrap(); - assert!(marker.evaluate(&env27, None, &[])); - assert!(!marker.evaluate(&env37, None, &[])); + assert!(marker.evaluate(&env27, MarkerVariantsUniversal, &[])); + assert!(!marker.evaluate(&env37, MarkerVariantsUniversal, &[])); let marker = MarkerTree::from_str("implementation_version in \"2.7 3.2 3.3\"").unwrap(); - assert!(marker.evaluate(&env27, None, &[])); - assert!(!marker.evaluate(&env37, None, &[])); + assert!(marker.evaluate(&env27, MarkerVariantsUniversal, &[])); + assert!(!marker.evaluate(&env37, MarkerVariantsUniversal, &[])); let marker = MarkerTree::from_str("implementation_version in \"2.7 3.7\"").unwrap(); - assert!(marker.evaluate(&env27, None, &[])); - assert!(marker.evaluate(&env37, None, &[])); + assert!(marker.evaluate(&env27, MarkerVariantsUniversal, &[])); + assert!(marker.evaluate(&env37, MarkerVariantsUniversal, &[])); let marker = MarkerTree::from_str("implementation_version not in \"2.7 3.7\"").unwrap(); - assert!(!marker.evaluate(&env27, None, &[])); - assert!(!marker.evaluate(&env37, None, &[])); + assert!(!marker.evaluate(&env27, MarkerVariantsUniversal, &[])); + assert!(!marker.evaluate(&env37, MarkerVariantsUniversal, &[])); let marker = MarkerTree::from_str("implementation_version not in \"2.4 3.8 4.0\"").unwrap(); - assert!(marker.evaluate(&env27, None, &[])); - assert!(marker.evaluate(&env37, None, &[])); + assert!(marker.evaluate(&env27, MarkerVariantsUniversal, &[])); + assert!(marker.evaluate(&env37, MarkerVariantsUniversal, &[])); } #[test] @@ -2291,7 +2335,7 @@ mod test { fn warnings1() { let env37 = env37(); let compare_keys = MarkerTree::from_str("platform_version == sys_platform").unwrap(); - compare_keys.evaluate(&env37, None, &[]); + compare_keys.evaluate(&env37, MarkerVariantsUniversal, &[]); logs_contain( "Comparing two markers with each other doesn't make any sense, will evaluate to false", ); @@ -2303,7 +2347,7 @@ mod test { fn warnings2() { let env37 = env37(); let non_pep440 = MarkerTree::from_str("python_version >= '3.9.'").unwrap(); - non_pep440.evaluate(&env37, None, &[]); + non_pep440.evaluate(&env37, MarkerVariantsUniversal, &[]); logs_contain( "Expected PEP 440 version to compare with python_version, found `3.9.`, \ will evaluate to false: after parsing `3.9`, found `.`, which is \ @@ -2317,7 +2361,7 @@ mod test { fn warnings3() { let env37 = env37(); let string_string = MarkerTree::from_str("'b' >= 'a'").unwrap(); - string_string.evaluate(&env37, None, &[]); + string_string.evaluate(&env37, MarkerVariantsUniversal, &[]); logs_contain( "Comparing two quoted strings with each other doesn't make sense: 'b' >= 'a', will evaluate to false", ); @@ -2329,7 +2373,7 @@ mod test { fn warnings4() { let env37 = env37(); let string_string = MarkerTree::from_str(r"os.name == 'posix' and platform.machine == 'x86_64' and platform.python_implementation == 'CPython' and 'Ubuntu' in platform.version and sys.platform == 'linux'").unwrap(); - string_string.evaluate(&env37, None, &[]); + string_string.evaluate(&env37, MarkerVariantsUniversal, &[]); logs_assert(|lines: &[&str]| { let lines: Vec<_> = lines .iter() @@ -2361,18 +2405,18 @@ mod test { let env37 = env37(); let result = MarkerTree::from_str("python_version > '3.6'") .unwrap() - .evaluate(&env37, None, &[]); + .evaluate(&env37, MarkerVariantsUniversal, &[]); assert!(result); let result = MarkerTree::from_str("'3.6' > python_version") .unwrap() - .evaluate(&env37, None, &[]); + .evaluate(&env37, MarkerVariantsUniversal, &[]); assert!(!result); // Meaningless expressions are ignored, so this is always true. let result = MarkerTree::from_str("'3.*' == python_version") .unwrap() - .evaluate(&env37, None, &[]); + .evaluate(&env37, MarkerVariantsUniversal, &[]); assert!(result); } @@ -2381,12 +2425,12 @@ mod test { let env37 = env37(); let result = MarkerTree::from_str("'nux' in sys_platform") .unwrap() - .evaluate(&env37, None, &[]); + .evaluate(&env37, MarkerVariantsUniversal, &[]); assert!(result); let result = MarkerTree::from_str("sys_platform in 'nux'") .unwrap() - .evaluate(&env37, None, &[]); + .evaluate(&env37, MarkerVariantsUniversal, &[]); assert!(!result); } @@ -2395,7 +2439,7 @@ mod test { let env37 = env37(); let result = MarkerTree::from_str("python_version == '3.7.*'") .unwrap() - .evaluate(&env37, None, &[]); + .evaluate(&env37, MarkerVariantsUniversal, &[]); assert!(result); } @@ -2404,7 +2448,7 @@ mod test { let env37 = env37(); let result = MarkerTree::from_str("python_version ~= '3.7'") .unwrap() - .evaluate(&env37, None, &[]); + .evaluate(&env37, MarkerVariantsUniversal, &[]); assert!(result); } @@ -3754,68 +3798,69 @@ mod test { #[test] fn marker_evaluation_variants() { let env37 = env37(); - let gpu_namespaces = [("gpu".to_string(), "cuda".to_string(), "12.4".to_string())]; - let cpu_namespaces = [("cpu".to_string(), String::new(), String::new())]; + let gpu_namespaces = [("gpu", "cuda", "12.4")].as_slice(); + let cpu_namespaces = [("cpu", "", "")].as_slice(); // namespace variant markers let marker1 = m("'gpu' in variant_namespaces"); let marker2 = m("'gpu' not in variant_namespaces"); // If no variants are provided, we solve universally. - assert!(marker1.evaluate(&env37, None, &[])); - assert!(marker2.evaluate(&env37, None, &[])); + assert!(marker1.evaluate(&env37, MarkerVariantsUniversal, &[])); + assert!(marker2.evaluate(&env37, MarkerVariantsUniversal, &[])); - assert!(marker1.evaluate(&env37, Some(&gpu_namespaces), &[])); - assert!(!marker1.evaluate(&env37, Some(&cpu_namespaces), &[])); - assert!(!marker2.evaluate(&env37, Some(&gpu_namespaces), &[])); - assert!(marker2.evaluate(&env37, Some(&cpu_namespaces), &[])); + assert!(marker1.evaluate(&env37, gpu_namespaces, &[])); + assert!(!marker1.evaluate(&env37, cpu_namespaces, &[])); + assert!(!marker2.evaluate(&env37, gpu_namespaces, &[])); + assert!(marker2.evaluate(&env37, cpu_namespaces, &[])); // property variant markers let marker3 = m("'gpu :: cuda' in variant_features"); let marker4 = m("'gpu :: rocm' in variant_features"); - assert!(marker3.evaluate(&env37, None, &[])); - assert!(marker4.evaluate(&env37, None, &[])); + assert!(marker3.evaluate(&env37, MarkerVariantsUniversal, &[])); + assert!(marker4.evaluate(&env37, MarkerVariantsUniversal, &[])); - assert!(marker3.evaluate(&env37, Some(&gpu_namespaces), &[])); - assert!(!marker3.evaluate(&env37, Some(&cpu_namespaces), &[])); - assert!(!marker4.evaluate(&env37, Some(&gpu_namespaces), &[])); - assert!(!marker4.evaluate(&env37, Some(&cpu_namespaces), &[])); + assert!(marker3.evaluate(&env37, gpu_namespaces, &[])); + assert!(!marker3.evaluate(&env37, cpu_namespaces, &[])); + assert!(!marker4.evaluate(&env37, gpu_namespaces, &[])); + assert!(!marker4.evaluate(&env37, cpu_namespaces, &[])); // feature variant markers let marker5 = m("'gpu :: cuda :: 12.4' in variant_properties"); let marker6 = m("'gpu :: cuda :: 12.8' in variant_properties"); - assert!(marker5.evaluate(&env37, None, &[])); - assert!(marker6.evaluate(&env37, None, &[])); + assert!(marker5.evaluate(&env37, MarkerVariantsUniversal, &[])); + assert!(marker6.evaluate(&env37, MarkerVariantsUniversal, &[])); - assert!(marker5.evaluate(&env37, Some(&gpu_namespaces), &[])); - assert!(!marker5.evaluate(&env37, Some(&cpu_namespaces), &[])); - assert!(!marker6.evaluate(&env37, Some(&gpu_namespaces), &[])); - assert!(!marker6.evaluate(&env37, Some(&cpu_namespaces), &[])); + assert!(marker5.evaluate(&env37, gpu_namespaces, &[])); + assert!(!marker5.evaluate(&env37, cpu_namespaces, &[])); + assert!(!marker6.evaluate(&env37, gpu_namespaces, &[])); + assert!(!marker6.evaluate(&env37, cpu_namespaces, &[])); } #[test] fn marker_evaluation_variants_combined() { let env37 = env37(); let namespaces = [ - ("gpu".to_string(), "cuda".to_string(), "12.4".to_string()), - ("gpu".to_string(), "cuda".to_string(), "12.6".to_string()), - ("cpu".to_string(), "x86_64".to_string(), "v1".to_string()), - ("cpu".to_string(), "x86_64".to_string(), "v2".to_string()), - ("cpu".to_string(), "x86_64".to_string(), "v3".to_string()), - ]; + ("gpu", "cuda", "12.4"), + ("gpu", "cuda", "12.6"), + ("cpu", "x86_64", "v1"), + ("cpu", "x86_64", "v2"), + ("cpu", "x86_64", "v3"), + ] + .as_slice(); let marker1 = m("'gpu' in variant_namespaces \ and 'cpu:: x86_64 :: v3' in variant_properties \ and python_version >= '3.7' \ and 'gpu :: rocm' not in variant_features"); - assert!(marker1.evaluate(&env37, None, &[])); - assert!(marker1.evaluate(&env37, Some(&namespaces), &[])); + assert!(marker1.evaluate(&env37, MarkerVariantsUniversal, &[])); + assert!(marker1.evaluate(&env37, namespaces, &[])); let marker2 = m("python_version >= '3.7' and 'gpu' not in variant_namespaces"); - assert!(marker2.evaluate(&env37, None, &[])); - assert!(!marker2.evaluate(&env37, Some(&namespaces), &[])); + assert!(marker2.evaluate(&env37, MarkerVariantsUniversal, &[])); + assert!(!marker2.evaluate(&env37, namespaces, &[])); } #[test] @@ -3863,33 +3908,30 @@ mod test { #[test] fn variant_invalid() { let env37 = env37(); - let namespaces = [("gpu".to_string(), "cuda".to_string(), "cuda126".to_string())]; + let namespaces = [("gpu", "cuda", "cuda126")].as_slice(); let marker = m(r"'gpu :: cuda :: cuda126 :: gtx1080' in variant_properties"); - assert!(!marker.evaluate(&env37, Some(&namespaces), &[])); + assert!(!marker.evaluate(&env37, namespaces, &[])); } #[test] fn torch_variant_marker() { let env37 = env37(); - let cu126 = [("nvidia".to_string(), "ctk".to_string(), "12.6".to_string())]; + let cu126 = [("nvidia", "ctk", "12.6")].as_slice(); let cu126_2 = [ - ("nvidia".to_string(), "ctk".to_string(), "12.6".to_string()), - ( - "nvidia".to_string(), - "cuda_version".to_string(), - ">=12.6,<13".to_string(), - ), - ]; - let cu128 = [("nvidia".to_string(), "ctk".to_string(), "12.8".to_string())]; + ("nvidia", "ctk", "12.6"), + ("nvidia", "cuda_version", ">=12.6,<13"), + ] + .as_slice(); + let cu128 = [("nvidia", "ctk", "12.8")].as_slice(); let marker = m( " platform_machine == 'x86_64' and sys_platform == 'linux' and 'nvidia :: ctk :: 12.6' in variant_properties", ); - assert!(marker.evaluate(&env37, None, &[])); - assert!(marker.evaluate(&env37, Some(&cu126), &[])); - assert!(marker.evaluate(&env37, Some(&cu126_2), &[])); - assert!(!marker.evaluate(&env37, Some(&cu128), &[])); + assert!(marker.evaluate(&env37, MarkerVariantsUniversal, &[])); + assert!(marker.evaluate(&env37, cu126, &[])); + assert!(marker.evaluate(&env37, cu126_2, &[])); + assert!(!marker.evaluate(&env37, cu128, &[])); } } diff --git a/crates/uv-pep508/src/unnamed.rs b/crates/uv-pep508/src/unnamed.rs index 2bf74058a..3221e3702 100644 --- a/crates/uv-pep508/src/unnamed.rs +++ b/crates/uv-pep508/src/unnamed.rs @@ -8,9 +8,10 @@ use uv_normalize::ExtraName; use crate::marker::parse; use crate::{ - Cursor, MarkerEnvironment, MarkerTree, Pep508Error, Pep508ErrorSource, Pep508Url, Reporter, - RequirementOrigin, Scheme, TracingReporter, VerbatimUrl, VerbatimUrlError, expand_env_vars, - parse_extras_cursor, split_extras, split_scheme, strip_host, + Cursor, MarkerEnvironment, MarkerTree, MarkerVariantsEnvironment, Pep508Error, + Pep508ErrorSource, Pep508Url, Reporter, RequirementOrigin, Scheme, TracingReporter, + VerbatimUrl, VerbatimUrlError, expand_env_vars, parse_extras_cursor, split_extras, + split_scheme, strip_host, }; /// An extension over [`Pep508Url`] that also supports parsing unnamed requirements, namely paths. @@ -85,7 +86,7 @@ impl UnnamedRequirement { pub fn evaluate_markers( &self, env: &MarkerEnvironment, - variants: Option<&[(String, String, String)]>, + variants: impl MarkerVariantsEnvironment, extras: &[ExtraName], ) -> bool { self.evaluate_optional_environment(Some(env), variants, extras) @@ -95,7 +96,7 @@ impl UnnamedRequirement { pub fn evaluate_optional_environment( &self, env: Option<&MarkerEnvironment>, - variants: Option<&[(String, String, String)]>, + variants: impl MarkerVariantsEnvironment, extras: &[ExtraName], ) -> bool { self.marker diff --git a/crates/uv-requirements/src/lookahead.rs b/crates/uv-requirements/src/lookahead.rs index a1156e0b4..7c386eaeb 100644 --- a/crates/uv-requirements/src/lookahead.rs +++ b/crates/uv-requirements/src/lookahead.rs @@ -8,6 +8,7 @@ use tracing::trace; use uv_configuration::{Constraints, Overrides}; use uv_distribution::{DistributionDatabase, Reporter}; use uv_distribution_types::{Dist, DistributionMetadata, Requirement, RequirementSource}; +use uv_pep508::MarkerVariantsUniversal; use uv_resolver::{InMemoryIndex, MetadataResponse, ResolverEnvironment}; use uv_types::{BuildContext, HashStrategy, RequestedRequirements}; @@ -91,7 +92,9 @@ impl<'a, Context: BuildContext> LookaheadResolver<'a, Context> { let mut queue: VecDeque<_> = self .constraints .apply(self.overrides.apply(self.requirements)) - .filter(|requirement| requirement.evaluate_markers(env.marker_environment(), None, &[])) + .filter(|requirement| { + requirement.evaluate_markers(env.marker_environment(), MarkerVariantsUniversal, &[]) + }) .map(|requirement| (*requirement).clone()) .collect(); @@ -112,7 +115,7 @@ impl<'a, Context: BuildContext> LookaheadResolver<'a, Context> { { if requirement.evaluate_markers( env.marker_environment(), - None, + MarkerVariantsUniversal, lookahead.extras(), ) { queue.push_back((*requirement).clone()); diff --git a/crates/uv-resolver/src/error.rs b/crates/uv-resolver/src/error.rs index c99bc54f4..b09972019 100644 --- a/crates/uv-resolver/src/error.rs +++ b/crates/uv-resolver/src/error.rs @@ -16,7 +16,9 @@ use uv_distribution_types::{ }; use uv_normalize::{ExtraName, InvalidNameError, PackageName}; use uv_pep440::{LocalVersionSlice, LowerBound, Version, VersionSpecifier}; -use uv_pep508::{MarkerEnvironment, MarkerExpression, MarkerTree, MarkerValueVersion}; +use uv_pep508::{ + MarkerEnvironment, MarkerExpression, MarkerTree, MarkerValueVersion, MarkerVariantsUniversal, +}; use uv_platform_tags::Tags; use uv_pypi_types::ParsedUrl; use uv_redacted::DisplaySafeUrl; @@ -412,7 +414,7 @@ impl NoSolutionError { ":".bold(), current_python_version, )?; - } else if !markers.evaluate(&self.current_environment, None, &[]) { + } else if !markers.evaluate(&self.current_environment, MarkerVariantsUniversal, &[]) { write!( f, "\n\n{}{} The resolution failed for an environment that is not the current one, \ diff --git a/crates/uv-resolver/src/graph_ops.rs b/crates/uv-resolver/src/graph_ops.rs index b25d760a1..7071d1860 100644 --- a/crates/uv-resolver/src/graph_ops.rs +++ b/crates/uv-resolver/src/graph_ops.rs @@ -5,7 +5,7 @@ use petgraph::visit::EdgeRef; use petgraph::{Direction, Graph}; use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; -use uv_pep508::MarkerTree; +use uv_pep508::{MarkerTree, MarkerVariantsUniversal}; use uv_pypi_types::{ConflictItem, Conflicts}; use crate::resolution::ResolutionGraphNode; @@ -272,7 +272,7 @@ pub(crate) fn simplify_conflict_markers( .collect::>(); graph[edge_index] .conflict() - .evaluate(None, &extras, &groups) + .evaluate(&extras, &groups, MarkerVariantsUniversal) }); if !all_paths_satisfied { continue; diff --git a/crates/uv-resolver/src/lock/export/pylock_toml.rs b/crates/uv-resolver/src/lock/export/pylock_toml.rs index f28a5b7b9..cde3129cf 100644 --- a/crates/uv-resolver/src/lock/export/pylock_toml.rs +++ b/crates/uv-resolver/src/lock/export/pylock_toml.rs @@ -31,7 +31,7 @@ use uv_git::{RepositoryReference, ResolvedRepositoryReference}; use uv_git_types::{GitOid, GitReference, GitUrl, GitUrlParseError}; use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_pep440::Version; -use uv_pep508::{MarkerEnvironment, MarkerTree, VerbatimUrl}; +use uv_pep508::{MarkerEnvironment, MarkerTree, MarkerVariantsUniversal, VerbatimUrl}; use uv_platform_tags::{TagCompatibility, TagPriority, Tags}; use uv_pypi_types::{HashDigests, Hashes, ParsedGitUrl, VcsKind}; use uv_redacted::DisplaySafeUrl; @@ -982,7 +982,7 @@ impl<'lock> PylockToml { // Omit packages that aren't relevant to the current environment. if !package .marker - .evaluate_pep751(markers, None, extras, groups) + .evaluate_pep751(markers, MarkerVariantsUniversal, extras, groups) { continue; } diff --git a/crates/uv-resolver/src/lock/installable.rs b/crates/uv-resolver/src/lock/installable.rs index fe8fca61b..05bbb5035 100644 --- a/crates/uv-resolver/src/lock/installable.rs +++ b/crates/uv-resolver/src/lock/installable.rs @@ -14,11 +14,12 @@ use uv_configuration::{BuildOptions, DependencyGroupsWithDefaults, InstallOption use uv_distribution::{DistributionDatabase, VariantProviderCache}; use uv_distribution_types::{Edge, Identifier, Node, Resolution, ResolvedDist}; use uv_normalize::{ExtraName, GroupName, PackageName}; +use uv_pep508::MarkerVariantsUniversal; use uv_platform_tags::Tags; use uv_pypi_types::ResolverMarkerEnvironment; use uv_types::BuildContext; use uv_variants::score_variant; -use uv_variants::variants_json::VariantPropertyType; +use uv_variants::variants_json::Variant; use crate::lock::{LockErrorKind, Package, TagPolicy}; use crate::{Lock, LockError}; @@ -151,7 +152,7 @@ pub trait Installable<'lock> { // TODO(konsti): Evaluate variant declarations on workspace/path dependencies. if !dep.complexified_marker.evaluate( marker_env, - None, + MarkerVariantsUniversal, activated_extras.iter().copied(), activated_groups.iter().copied(), ) { @@ -215,14 +216,17 @@ pub trait Installable<'lock> { // Add any requirements that are exclusive to the workspace root (e.g., dependencies in // PEP 723 scripts). for dependency in self.lock().requirements() { - if !dependency.marker.evaluate(marker_env, None, &[]) { + if !dependency + .marker + .evaluate(marker_env, MarkerVariantsUniversal, &[]) + { continue; } let root_name = &dependency.name; let dist = self .lock() - .find_by_markers(root_name, marker_env, None) + .find_by_markers(root_name, marker_env) .map_err(|_| LockErrorKind::MultipleRootPackages { name: root_name.clone(), })? @@ -267,7 +271,10 @@ pub trait Installable<'lock> { }) .flatten() { - if !dependency.marker.evaluate(marker_env, None, &[]) { + if !dependency + .marker + .evaluate(marker_env, MarkerVariantsUniversal, &[]) + { continue; } @@ -275,7 +282,7 @@ pub trait Installable<'lock> { // TODO(konsti): Evaluate variant declarations on workspace/path dependencies. let dist = self .lock() - .find_by_markers(root_name, marker_env, None) + .find_by_markers(root_name, marker_env) .map_err(|_| LockErrorKind::MultipleRootPackages { name: root_name.clone(), })? @@ -378,7 +385,7 @@ pub trait Installable<'lock> { // TODO(konsti): Evaluate variants if !dep.complexified_marker.evaluate( marker_env, - None, + MarkerVariantsUniversal, activated_extras .iter() .chain(additional_activated_extras.iter()) @@ -475,7 +482,7 @@ pub trait Installable<'lock> { for dep in deps { if !dep.complexified_marker.evaluate( marker_env, - variant_properties.as_deref(), + &variant_properties, activated_extras.iter().copied(), activated_groups.iter().copied(), ) { @@ -600,9 +607,10 @@ async fn determine_properties( workspace_root: &Path, distribution_database: &DistributionDatabase<'_, Context>, variants_cache: &VariantProviderCache, -) -> Result>, LockError> { +) -> Result { let Some(variants_json) = package.to_registry_variants_json(workspace_root)? else { - return Ok(None); + // When selecting a non-variant wheel, all variant markers evaluate to false. + return Ok(Variant::default()); }; let resolved_variants = if let Some(resolved_variants) = variants_cache.get_resolved_variants(&variants_json.resource_id()) @@ -654,39 +662,15 @@ async fn determine_properties( } if let Some((best_variant, _)) = highest_priority_variant_wheel { - // TODO(konsti): Use the proper type everywhere, we shouldn't need to do - // this conversion at all. - let mut known_properties = BTreeSet::new(); - for (namespace, features) in resolved_variants + // TODO(konsti): We shouldn't need to clone + let known_properties = resolved_variants .variants_json .variants .get(best_variant) - .expect("TODO(konsti): error handling") - { - for (feature, values) in features { - for value in values { - known_properties.insert(VariantPropertyType { - namespace: namespace.clone(), - feature: feature.clone(), - value: value.clone(), - }); - } - } - } - let known_properties: Vec<(String, String, String)> = known_properties - .into_iter() - .map(|known_property| { - ( - known_property.namespace, - known_property.feature, - known_property.value, - ) - }) - .collect(); - Ok(Some(known_properties)) + .expect("TODO(konsti): error handling"); + Ok(known_properties.clone()) } else { - // When selecting the non-variant wheel, all variant markers evaluate to - // false. - Ok(Some(Vec::new())) + // When selecting the non-variant wheel, all variant markers evaluate to false. + Ok(Variant::default()) } } diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index 98f0451e9..e7fd80a8d 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -1111,7 +1111,6 @@ impl Lock { &self, name: &PackageName, marker_env: &MarkerEnvironment, - variants: Option<&[(String, String, String)]>, ) -> Result, String> { let mut found_dist = None; for dist in &self.packages { @@ -1120,7 +1119,7 @@ impl Lock { || dist .fork_markers .iter() - .any(|marker| marker.evaluate_no_extras(marker_env, variants)) + .any(|marker| marker.evaluate_no_extras(marker_env)) { if found_dist.is_some() { return Err(format!("found multiple packages matching `{name}`")); diff --git a/crates/uv-resolver/src/lock/tree.rs b/crates/uv-resolver/src/lock/tree.rs index 274a3d7ec..b3e1ddfb4 100644 --- a/crates/uv-resolver/src/lock/tree.rs +++ b/crates/uv-resolver/src/lock/tree.rs @@ -12,7 +12,7 @@ use rustc_hash::{FxBuildHasher, FxHashMap, FxHashSet}; use uv_configuration::DependencyGroupsWithDefaults; use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_pep440::Version; -use uv_pep508::MarkerTree; +use uv_pep508::{MarkerTree, MarkerVariantsUniversal}; use uv_pypi_types::ResolverMarkerEnvironment; use crate::lock::PackageId; @@ -126,9 +126,9 @@ impl<'env> TreeDisplay<'env> { continue; } - if markers.is_some_and(|markers| { - !dep.complexified_marker.evaluate_no_extras(markers, None) - }) { + if markers + .is_some_and(|markers| !dep.complexified_marker.evaluate_no_extras(markers)) + { continue; } @@ -190,7 +190,9 @@ impl<'env> TreeDisplay<'env> { if marker.is_false() { continue; } - if markers.is_some_and(|markers| !marker.evaluate(markers, None, &[])) { + if markers.is_some_and(|markers| { + !marker.evaluate(markers, MarkerVariantsUniversal, &[]) + }) { continue; } // Add the package to the graph. @@ -227,7 +229,9 @@ impl<'env> TreeDisplay<'env> { if marker.is_false() { continue; } - if markers.is_some_and(|markers| !marker.evaluate(markers, None, &[])) { + if markers.is_some_and(|markers| { + !marker.evaluate(markers, MarkerVariantsUniversal, &[]) + }) { continue; } // Add the package to the graph. @@ -269,9 +273,9 @@ impl<'env> TreeDisplay<'env> { continue; } - if markers.is_some_and(|markers| { - !dep.complexified_marker.evaluate_no_extras(markers, None) - }) { + if markers + .is_some_and(|markers| !dep.complexified_marker.evaluate_no_extras(markers)) + { continue; } diff --git a/crates/uv-resolver/src/manifest.rs b/crates/uv-resolver/src/manifest.rs index 8e92c35af..0081463af 100644 --- a/crates/uv-resolver/src/manifest.rs +++ b/crates/uv-resolver/src/manifest.rs @@ -6,6 +6,7 @@ use either::Either; use uv_configuration::{Constraints, Overrides}; use uv_distribution_types::Requirement; use uv_normalize::PackageName; +use uv_pep508::MarkerVariantsUniversal; use uv_types::RequestedRequirements; use crate::preferences::Preferences; @@ -116,43 +117,55 @@ impl Manifest { ) -> impl Iterator> + 'a { match mode { // Include all direct and transitive requirements, with constraints and overrides applied. - DependencyMode::Transitive => Either::Left( - self.lookaheads - .iter() - .flat_map(move |lookahead| { - self.overrides - .apply(lookahead.requirements()) - .filter(move |requirement| { + DependencyMode::Transitive => { + Either::Left( + self.lookaheads + .iter() + .flat_map(move |lookahead| { + self.overrides.apply(lookahead.requirements()).filter( + move |requirement| { + requirement.evaluate_markers( + env.marker_environment(), + MarkerVariantsUniversal, + lookahead.extras(), + ) + }, + ) + }) + .chain(self.overrides.apply(&self.requirements).filter( + move |requirement| { requirement.evaluate_markers( env.marker_environment(), - None, - lookahead.extras(), + MarkerVariantsUniversal, + &[], ) - }) - }) - .chain( - self.overrides - .apply(&self.requirements) - .filter(move |requirement| { - requirement.evaluate_markers(env.marker_environment(), None, &[]) - }), - ) - .chain( - self.constraints - .requirements() - .filter(move |requirement| { - requirement.evaluate_markers(env.marker_environment(), None, &[]) - }) - .map(Cow::Borrowed), - ), - ), + }, + )) + .chain( + self.constraints + .requirements() + .filter(move |requirement| { + requirement.evaluate_markers( + env.marker_environment(), + MarkerVariantsUniversal, + &[], + ) + }) + .map(Cow::Borrowed), + ), + ) + } // Include direct requirements, with constraints and overrides applied. DependencyMode::Direct => Either::Right( self.overrides .apply(&self.requirements) .chain(self.constraints.requirements().map(Cow::Borrowed)) .filter(move |requirement| { - requirement.evaluate_markers(env.marker_environment(), None, &[]) + requirement.evaluate_markers( + env.marker_environment(), + MarkerVariantsUniversal, + &[], + ) }), ), } @@ -170,7 +183,11 @@ impl Manifest { self.overrides .requirements() .filter(move |requirement| { - requirement.evaluate_markers(env.marker_environment(), None, &[]) + requirement.evaluate_markers( + env.marker_environment(), + MarkerVariantsUniversal, + &[], + ) }) .map(Cow::Borrowed), ), @@ -179,7 +196,11 @@ impl Manifest { self.overrides .requirements() .filter(move |requirement| { - requirement.evaluate_markers(env.marker_environment(), None, &[]) + requirement.evaluate_markers( + env.marker_environment(), + MarkerVariantsUniversal, + &[], + ) }) .map(Cow::Borrowed), ), @@ -214,7 +235,7 @@ impl Manifest { move |requirement| { requirement.evaluate_markers( env.marker_environment(), - None, + MarkerVariantsUniversal, lookahead.extras(), ) }, @@ -222,7 +243,11 @@ impl Manifest { }) .chain(self.overrides.apply(&self.requirements).filter( move |requirement| { - requirement.evaluate_markers(env.marker_environment(), None, &[]) + requirement.evaluate_markers( + env.marker_environment(), + MarkerVariantsUniversal, + &[], + ) }, )), ) @@ -232,7 +257,11 @@ impl Manifest { DependencyMode::Direct => { Either::Right(self.overrides.apply(self.requirements.iter()).filter( move |requirement| { - requirement.evaluate_markers(env.marker_environment(), None, &[]) + requirement.evaluate_markers( + env.marker_environment(), + MarkerVariantsUniversal, + &[], + ) }, )) } @@ -251,7 +280,7 @@ impl Manifest { self.overrides .apply(self.requirements.iter()) .filter(move |requirement| { - requirement.evaluate_markers(env.marker_environment(), None, &[]) + requirement.evaluate_markers(env.marker_environment(), MarkerVariantsUniversal, &[]) }) } diff --git a/crates/uv-resolver/src/preferences.rs b/crates/uv-resolver/src/preferences.rs index cc0a1bce0..fd1a5905b 100644 --- a/crates/uv-resolver/src/preferences.rs +++ b/crates/uv-resolver/src/preferences.rs @@ -7,7 +7,7 @@ use tracing::trace; use uv_distribution_types::{IndexUrl, InstalledDist}; use uv_normalize::PackageName; use uv_pep440::{Operator, Version}; -use uv_pep508::{MarkerTree, VerbatimUrl, VersionOrUrl}; +use uv_pep508::{MarkerTree, MarkerVariantsUniversal, VerbatimUrl, VersionOrUrl}; use uv_pypi_types::{HashDigest, HashDigests, HashError}; use uv_requirements_txt::{RequirementEntry, RequirementsTxtRequirement}; @@ -241,7 +241,10 @@ impl Preferences { for preference in preferences { // Filter non-matching preferences when resolving for an environment. if let Some(markers) = env.marker_environment() { - if !preference.marker.evaluate(markers, None, &[]) { + if !preference + .marker + .evaluate(markers, MarkerVariantsUniversal, &[]) + { trace!("Excluding {preference} from preferences due to unmatched markers"); continue; } @@ -250,7 +253,7 @@ impl Preferences { if !preference .fork_markers .iter() - .any(|marker| marker.evaluate_no_extras(markers, None)) + .any(|marker| marker.evaluate_no_extras(markers)) { trace!( "Excluding {preference} from preferences due to unmatched fork markers" diff --git a/crates/uv-resolver/src/resolution/display.rs b/crates/uv-resolver/src/resolution/display.rs index 76bd48741..84a0dd1b8 100644 --- a/crates/uv-resolver/src/resolution/display.rs +++ b/crates/uv-resolver/src/resolution/display.rs @@ -7,7 +7,7 @@ use rustc_hash::{FxBuildHasher, FxHashMap}; use uv_distribution_types::{DistributionMetadata, Name, SourceAnnotation, SourceAnnotations}; use uv_normalize::PackageName; -use uv_pep508::MarkerTree; +use uv_pep508::{MarkerTree, MarkerVariantsUniversal}; use crate::resolution::{RequirementsTxtDist, ResolutionGraphNode}; use crate::{ResolverEnvironment, ResolverOutput}; @@ -91,7 +91,11 @@ impl std::fmt::Display for DisplayResolutionGraph<'_> { let mut sources = SourceAnnotations::default(); for requirement in self.resolution.requirements.iter().filter(|requirement| { - requirement.evaluate_markers(self.env.marker_environment(), None, &[]) + requirement.evaluate_markers( + self.env.marker_environment(), + MarkerVariantsUniversal, + &[], + ) }) { if let Some(origin) = &requirement.origin { sources.add( @@ -106,7 +110,11 @@ impl std::fmt::Display for DisplayResolutionGraph<'_> { .constraints .requirements() .filter(|requirement| { - requirement.evaluate_markers(self.env.marker_environment(), None, &[]) + requirement.evaluate_markers( + self.env.marker_environment(), + MarkerVariantsUniversal, + &[], + ) }) { if let Some(origin) = &requirement.origin { @@ -122,7 +130,11 @@ impl std::fmt::Display for DisplayResolutionGraph<'_> { .overrides .requirements() .filter(|requirement| { - requirement.evaluate_markers(self.env.marker_environment(), None, &[]) + requirement.evaluate_markers( + self.env.marker_environment(), + MarkerVariantsUniversal, + &[], + ) }) { if let Some(origin) = &requirement.origin { diff --git a/crates/uv-resolver/src/resolver/mod.rs b/crates/uv-resolver/src/resolver/mod.rs index 442671bb3..0dfa9b0c6 100644 --- a/crates/uv-resolver/src/resolver/mod.rs +++ b/crates/uv-resolver/src/resolver/mod.rs @@ -34,6 +34,7 @@ use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_pep440::{MIN_VERSION, Version, VersionSpecifiers, release_specifiers_to_ranges}; use uv_pep508::{ MarkerEnvironment, MarkerExpression, MarkerOperator, MarkerTree, MarkerValueString, + MarkerVariantsEnvironment, MarkerVariantsUniversal, }; use uv_platform_tags::Tags; use uv_pypi_types::{ConflictItem, ConflictItemRef, Conflicts, VerbatimParsedUrl}; @@ -84,7 +85,7 @@ use crate::{ pub(crate) use provider::MetadataUnavailable; use uv_torch::TorchStrategy; use uv_variants::resolved_variants::ResolvedVariants; -use uv_variants::variants_json::VariantPropertyType; +use uv_variants::variants_json::Variant; mod availability; mod batch_prefetch; @@ -1819,7 +1820,7 @@ impl ResolverState ResolverState ResolverState ResolverState Option> { + ) -> Variant { // TODO(konsti): Perf/Caching with version selection: This is in the hot path! - env.marker_environment()?; - let resolved_variants = in_memory_index + if env.marker_environment().is_none() { + return Variant::default(); + } + let Some(resolved_variants) = in_memory_index .variant_priorities() - .get(&(version_id.clone(), index_url.cloned()))?; + .get(&(version_id.clone(), index_url.cloned())) + else { + return Variant::default(); + }; let filename = pins.get(name, version).unwrap().wheel_filename(); let Some(filename) = filename else { // TODO(konsti): Handle installed dists too - return None; + return Variant::default(); }; let Some(variant_label) = filename.variant() else { warn!("Wheel variant has no associated variants: {filename}"); - return None; + return Variant::default(); }; // Collect the host properties for marker filtering. - // TODO(konsti): Use the proper type everywhere, we shouldn't need to do - // this conversion at all. - let mut known_properties = BTreeSet::new(); - let namespaces = resolved_variants + // TODO(konsti): We shouldn't need to clone + let variant = resolved_variants .variants_json .variants .get(variant_label) .expect("Missing previously select variant label"); - for (namespace, features) in namespaces { - for (feature, values) in features { - for value in values { - known_properties.insert(VariantPropertyType { - namespace: namespace.clone(), - feature: feature.clone(), - value: value.clone(), - }); - } - } - } - let known_properties: Vec<(String, String, String)> = known_properties - .into_iter() - .map(|known_property| { - ( - known_property.namespace, - known_property.feature, - known_property.value, - ) - }) - .collect(); - Some(known_properties) + variant.clone() } /// The regular and dev dependencies filtered by Python version and the markers of this fork, @@ -2109,7 +2092,7 @@ impl ResolverState, name: Option<&PackageName>, env: &'a ResolverEnvironment, - variants: Option<&'a [(String, String, String)]>, + variants: &'a impl MarkerVariantsEnvironment, python_requirement: &'a PythonRequirement, ) -> impl Iterator> { let python_marker = python_requirement.to_marker_tree(); @@ -2166,7 +2149,7 @@ impl ResolverState ResolverState + 'parameters, extra: Option<&'parameters ExtraName>, env: &'parameters ResolverEnvironment, - variants: Option<&'parameters [(String, String, String)]>, + variants: &'parameters impl MarkerVariantsEnvironment, python_marker: MarkerTree, python_requirement: &'parameters PythonRequirement, ) -> impl Iterator> + 'parameters @@ -2273,7 +2256,7 @@ impl ResolverState, env: &ResolverEnvironment, - variants: Option<&[(String, String, String)]>, + variants: impl MarkerVariantsEnvironment, python_marker: MarkerTree, python_requirement: &PythonRequirement, ) -> bool { @@ -2281,12 +2264,12 @@ impl ResolverState { // Only include requirements that are relevant for the current extra. - if requirement.evaluate_markers(env.marker_environment(), variants, &[]) { + if requirement.evaluate_markers(env.marker_environment(), &variants, &[]) { return false; } if !requirement.evaluate_markers( env.marker_environment(), - variants, + &variants, slice::from_ref(source_extra), ) { return false; @@ -2330,7 +2313,7 @@ impl ResolverState, extra: Option<&'parameters ExtraName>, env: &'parameters ResolverEnvironment, - variants: Option<&'parameters [(String, String, String)]>, + variants: impl MarkerVariantsEnvironment + 'parameters, python_marker: MarkerTree, python_requirement: &'parameters PythonRequirement, ) -> impl Iterator> + 'parameters @@ -2422,7 +2405,7 @@ impl ResolverState { if !constraint - .evaluate_markers(env.marker_environment(), variants, slice::from_ref(source_extra)) + .evaluate_markers(env.marker_environment(), &variants, slice::from_ref(source_extra)) { return None; } @@ -2432,7 +2415,7 @@ impl ResolverState { - if !constraint.evaluate_markers(env.marker_environment(), variants, &[]) { + if !constraint.evaluate_markers(env.marker_environment(), &variants, &[]) { return None; } } diff --git a/crates/uv-resolver/src/universal_marker.rs b/crates/uv-resolver/src/universal_marker.rs index 7611eeb96..3887eeee7 100644 --- a/crates/uv-resolver/src/universal_marker.rs +++ b/crates/uv-resolver/src/universal_marker.rs @@ -7,7 +7,7 @@ use rustc_hash::FxHashMap; use uv_normalize::{ExtraName, GroupName, PackageName}; use uv_pep508::{ ExtraOperator, MarkerEnvironment, MarkerEnvironmentBuilder, MarkerExpression, MarkerOperator, - MarkerTree, + MarkerTree, MarkerVariantsEnvironment, MarkerVariantsUniversal, }; use uv_pypi_types::{ConflictItem, ConflictPackage, Conflicts}; @@ -239,12 +239,8 @@ impl UniversalMarker { /// /// This should only be used when evaluating a marker that is known not to /// have any extras. For example, the PEP 508 markers on a fork. - pub(crate) fn evaluate_no_extras( - self, - env: &MarkerEnvironment, - variants: Option<&[(String, String, String)]>, - ) -> bool { - self.marker.evaluate(env, variants, &[]) + pub(crate) fn evaluate_no_extras(self, env: &MarkerEnvironment) -> bool { + self.marker.evaluate(env, MarkerVariantsUniversal, &[]) } /// Returns true if this universal marker is satisfied by the given marker @@ -256,7 +252,7 @@ impl UniversalMarker { pub(crate) fn evaluate( self, env: &MarkerEnvironment, - variants: Option<&[(String, String, String)]>, + variants: impl MarkerVariantsEnvironment, extras: impl Iterator, groups: impl Iterator, ) -> bool @@ -436,9 +432,9 @@ impl ConflictMarker { /// list of activated extras and groups. pub(crate) fn evaluate( self, - variants: Option<&[(String, String, String)]>, extras: &[(P, E)], groups: &[(P, G)], + variants: impl MarkerVariantsEnvironment, ) -> bool where P: Borrow, @@ -745,7 +741,7 @@ pub(crate) fn resolve_conflicts( mod tests { use super::*; use std::str::FromStr; - + use uv_pep508::MarkerVariantsUniversal; use uv_pypi_types::ConflictSet; /// Creates a collection of declared conflicts from the sets @@ -875,7 +871,7 @@ mod tests { .collect::>(); let groups = Vec::<(PackageName, GroupName)>::new(); assert!( - !cm.evaluate(None, &extras, &groups), + !cm.evaluate(&extras, &groups, MarkerVariantsUniversal), "expected `{extra_names:?}` to evaluate to `false` in `{cm:?}`" ); } @@ -898,7 +894,7 @@ mod tests { .collect::>(); let groups = Vec::<(PackageName, GroupName)>::new(); assert!( - cm.evaluate(None, &extras, &groups), + cm.evaluate(&extras, &groups, MarkerVariantsUniversal), "expected `{extra_names:?}` to evaluate to `true` in `{cm:?}`" ); } diff --git a/crates/uv-types/src/hash.rs b/crates/uv-types/src/hash.rs index 8e7641c2e..f77e9f5df 100644 --- a/crates/uv-types/src/hash.rs +++ b/crates/uv-types/src/hash.rs @@ -10,6 +10,7 @@ use uv_distribution_types::{ }; use uv_normalize::PackageName; use uv_pep440::Version; +use uv_pep508::MarkerVariantsUniversal; use uv_pypi_types::{HashDigest, HashDigests, HashError, ResolverMarkerEnvironment}; use uv_redacted::DisplaySafeUrl; @@ -136,7 +137,7 @@ impl HashStrategy { for (requirement, digests) in constraints { if !requirement.evaluate_markers( marker_env.map(ResolverMarkerEnvironment::markers), - None, + MarkerVariantsUniversal, &[], ) { continue; @@ -182,7 +183,7 @@ impl HashStrategy { for (requirement, digests) in requirements { if !requirement.evaluate_markers( marker_env.map(ResolverMarkerEnvironment::markers), - None, + MarkerVariantsUniversal, &[], ) { continue; diff --git a/crates/uv-variants/src/lib.rs b/crates/uv-variants/src/lib.rs index fec201fee..d334eb460 100644 --- a/crates/uv-variants/src/lib.rs +++ b/crates/uv-variants/src/lib.rs @@ -39,7 +39,7 @@ pub fn score_variant( target_namespaces: &FxHashMap, variants_properties: &Variant, ) -> Option> { - for (namespace, features) in variants_properties { + for (namespace, features) in &**variants_properties { for (feature, properties) in features { let resolved_properties = target_namespaces .get(namespace) diff --git a/crates/uv-variants/src/variants_json.rs b/crates/uv-variants/src/variants_json.rs index b1e143149..2b481cf9f 100644 --- a/crates/uv-variants/src/variants_json.rs +++ b/crates/uv-variants/src/variants_json.rs @@ -1,18 +1,60 @@ +use std::ops::Deref; + use indoc::formatdoc; use rustc_hash::FxHashMap; use serde::{Deserialize, Serialize}; -use uv_pep508::Requirement; +use uv_pep508::{MarkerVariantsEnvironment, Requirement}; use uv_pypi_types::VerbatimParsedUrl; -/// Mapping of namespaces in a variant -pub type Variant = FxHashMap>>; - // TODO(konsti): Validate the string contents pub type VariantNamespace = String; pub type VariantFeature = String; pub type VariantProperty = String; +/// Mapping of namespaces in a variant +#[derive(Debug, Default, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(transparent)] +pub struct Variant(FxHashMap>>); + +impl MarkerVariantsEnvironment for Variant { + fn has_namespace(&self, namespace: &str) -> bool { + self.0.contains_key(namespace) + } + + fn has_feature(&self, namespace: &str, feature: &str) -> bool { + let Some(features) = self.0.get(namespace) else { + return false; + }; + + let Some(properties) = features.get(feature) else { + return false; + }; + + !properties.is_empty() + } + + fn has_property(&self, namespace: &str, feature: &str, property: &str) -> bool { + let Some(features) = self.0.get(namespace) else { + return false; + }; + + let Some(properties) = features.get(feature) else { + return false; + }; + + properties.iter().any(|p| p == property) + } +} + +impl Deref for Variant { + type Target = FxHashMap>>; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + /// Combined index metadata for wheel variants. /// /// See diff --git a/crates/uv/src/commands/pip/show.rs b/crates/uv/src/commands/pip/show.rs index 8c25f643d..47a02201b 100644 --- a/crates/uv/src/commands/pip/show.rs +++ b/crates/uv/src/commands/pip/show.rs @@ -13,6 +13,7 @@ use uv_fs::Simplified; use uv_install_wheel::read_record_file; use uv_installer::SitePackages; use uv_normalize::PackageName; +use uv_pep508::MarkerVariantsUniversal; use uv_python::{EnvironmentPreference, PythonEnvironment, PythonRequest}; use crate::commands::ExitStatus; @@ -101,7 +102,7 @@ pub(crate) fn pip_show( requires_map.insert( dist.name(), Box::into_iter(metadata.requires_dist) - .filter(|req| req.evaluate_markers(&markers, None, &[])) + .filter(|req| req.evaluate_markers(&markers, MarkerVariantsUniversal, &[])) .map(|req| req.name) .sorted_unstable() .dedup() @@ -117,7 +118,7 @@ pub(crate) fn pip_show( } if let Ok(metadata) = installed.metadata() { let requires = Box::into_iter(metadata.requires_dist) - .filter(|req| req.evaluate_markers(&markers, None, &[])) + .filter(|req| req.evaluate_markers(&markers, MarkerVariantsUniversal, &[])) .map(|req| req.name) .collect_vec(); if !requires.is_empty() { diff --git a/crates/uv/src/commands/pip/tree.rs b/crates/uv/src/commands/pip/tree.rs index 37401a0df..299179f1c 100644 --- a/crates/uv/src/commands/pip/tree.rs +++ b/crates/uv/src/commands/pip/tree.rs @@ -18,7 +18,7 @@ use uv_distribution_types::{Diagnostic, IndexCapabilities, IndexLocations, Name, use uv_installer::SitePackages; use uv_normalize::PackageName; use uv_pep440::Version; -use uv_pep508::{Requirement, VersionOrUrl}; +use uv_pep508::{MarkerVariantsUniversal, Requirement, VersionOrUrl}; use uv_pypi_types::{ResolutionMetadata, ResolverMarkerEnvironment, VerbatimParsedUrl}; use uv_python::{EnvironmentPreference, PythonEnvironment, PythonRequest}; use uv_resolver::{ExcludeNewer, PrereleaseMode}; @@ -254,7 +254,10 @@ impl<'env> DisplayDependencyGraph<'env> { if prune.contains(&requirement.name) { continue; } - if !requirement.marker.evaluate(markers, None, &[]) { + if !requirement + .marker + .evaluate(markers, MarkerVariantsUniversal, &[]) + { continue; } diff --git a/crates/uv/src/commands/project/sync.rs b/crates/uv/src/commands/project/sync.rs index b38d3fea7..fe67cc3e3 100644 --- a/crates/uv/src/commands/project/sync.rs +++ b/crates/uv/src/commands/project/sync.rs @@ -25,7 +25,7 @@ use uv_distribution_types::{ use uv_fs::{PortablePathBuf, Simplified}; use uv_installer::SitePackages; use uv_normalize::{DefaultExtras, DefaultGroups, PackageName}; -use uv_pep508::{MarkerTree, VersionOrUrl}; +use uv_pep508::{MarkerTree, MarkerVariantsUniversal, VersionOrUrl}; use uv_pypi_types::{ParsedArchiveUrl, ParsedGitUrl, ParsedUrl}; use uv_python::{PythonDownloads, PythonEnvironment, PythonPreference, PythonRequest}; use uv_resolver::{FlatIndex, Installable, Lock}; @@ -622,7 +622,7 @@ pub(super) async fn do_sync( if !environments.is_empty() { if !environments .iter() - .any(|env| env.evaluate(&marker_env, None, &[])) + .any(|env| env.evaluate(&marker_env, MarkerVariantsUniversal, &[])) { return Err(ProjectError::LockedPlatformIncompatibility( // For error reporting, we use the "simplified"