Requirements with platform markers

This commit is contained in:
konstin 2024-03-06 09:55:28 +01:00
parent 3ca777673f
commit efb9a93306
10 changed files with 89 additions and 23 deletions

View File

@ -358,6 +358,18 @@ impl Requirement {
} }
} }
/// Returns whether the markers apply for the given environment
pub fn evaluate_markers2(&self, env: &MarkerEnvironment, extras: &[ExtraName]) -> bool {
if let Some(marker) = &self.marker {
marker.evaluate_extras_and_python_version(
&extras.into_iter().cloned().collect(),
&[env.python_version.version.clone()],
)
} else {
true
}
}
/// Returns whether the requirement would be satisfied, independent of environment markers, i.e. /// Returns whether the requirement would be satisfied, independent of environment markers, i.e.
/// if there is potentially an environment that could activate this requirement. /// if there is potentially an environment that could activate this requirement.
/// ///

View File

@ -114,7 +114,7 @@ pub(crate) async fn resolve_cli(args: ResolveCliArgs) -> Result<()> {
let graphviz = Dot::with_attr_getters( let graphviz = Dot::with_attr_getters(
resolution_graph.petgraph(), resolution_graph.petgraph(),
&[DotConfig::NodeNoLabel, DotConfig::EdgeNoLabel], &[DotConfig::NodeNoLabel, DotConfig::EdgeNoLabel],
&|_graph, edge_ref| format!("label={:?}", edge_ref.weight().to_string()), &|_graph, edge_ref| format!("label={:?}", edge_ref.weight().0.to_string()),
&|_graph, (_node_index, dist)| { &|_graph, (_node_index, dist)| {
format!("label={:?}", dist.to_string().replace("==", "\n")) format!("label={:?}", dist.to_string().replace("==", "\n"))
}, },

View File

@ -65,7 +65,7 @@ impl Preferences {
requirements requirements
.iter() .iter()
.filter_map(|requirement| { .filter_map(|requirement| {
if !requirement.evaluate_markers(markers, &[]) { if !requirement.evaluate_markers2(markers, &[]) {
return None; return None;
} }
let Some(VersionOrUrl::VersionSpecifier(version_specifiers)) = let Some(VersionOrUrl::VersionSpecifier(version_specifiers)) =

View File

@ -65,10 +65,10 @@ impl PreReleaseStrategy {
.iter() .iter()
.chain(manifest.constraints.iter()) .chain(manifest.constraints.iter())
.chain(manifest.overrides.iter()) .chain(manifest.overrides.iter())
.filter(|requirement| requirement.evaluate_markers(markers, &[])) .filter(|requirement| requirement.evaluate_markers2(markers, &[]))
.chain(manifest.editables.iter().flat_map(|(editable, metadata)| { .chain(manifest.editables.iter().flat_map(|(editable, metadata)| {
metadata.requires_dist.iter().filter(|requirement| { metadata.requires_dist.iter().filter(|requirement| {
requirement.evaluate_markers(markers, &editable.extras) requirement.evaluate_markers2(markers, &editable.extras)
}) })
})) }))
.filter(|requirement| { .filter(|requirement| {
@ -94,10 +94,10 @@ impl PreReleaseStrategy {
.iter() .iter()
.chain(manifest.constraints.iter()) .chain(manifest.constraints.iter())
.chain(manifest.overrides.iter()) .chain(manifest.overrides.iter())
.filter(|requirement| requirement.evaluate_markers(markers, &[])) .filter(|requirement| requirement.evaluate_markers2(markers, &[]))
.chain(manifest.editables.iter().flat_map(|(editable, metadata)| { .chain(manifest.editables.iter().flat_map(|(editable, metadata)| {
metadata.requires_dist.iter().filter(|requirement| { metadata.requires_dist.iter().filter(|requirement| {
requirement.evaluate_markers(markers, &editable.extras) requirement.evaluate_markers2(markers, &editable.extras)
}) })
})) }))
.filter(|requirement| { .filter(|requirement| {

View File

@ -34,10 +34,10 @@ impl PubGrubDependencies {
for requirement in overrides.apply(requirements) { for requirement in overrides.apply(requirements) {
// If the requirement isn't relevant for the current platform, skip it. // If the requirement isn't relevant for the current platform, skip it.
if let Some(extra) = source_extra { if let Some(extra) = source_extra {
if !requirement.evaluate_markers(env, std::slice::from_ref(extra)) { if !requirement.evaluate_markers2(env, std::slice::from_ref(extra)) {
continue; continue;
} }
} else if !requirement.evaluate_markers(env, &[]) { } else if !requirement.evaluate_markers2(env, &[]) {
continue; continue;
} }
@ -68,10 +68,10 @@ impl PubGrubDependencies {
for constraint in constraints.get(&requirement.name).into_iter().flatten() { for constraint in constraints.get(&requirement.name).into_iter().flatten() {
// If the requirement isn't relevant for the current platform, skip it. // If the requirement isn't relevant for the current platform, skip it.
if let Some(extra) = source_extra { if let Some(extra) = source_extra {
if !constraint.evaluate_markers(env, std::slice::from_ref(extra)) { if !constraint.evaluate_markers2(env, std::slice::from_ref(extra)) {
continue; continue;
} }
} else if !constraint.evaluate_markers(env, &[]) { } else if !constraint.evaluate_markers2(env, &[]) {
continue; continue;
} }

View File

@ -15,6 +15,7 @@ use url::Url;
use distribution_types::{Dist, DistributionMetadata, LocalEditable, Name, PackageId, Verbatim}; use distribution_types::{Dist, DistributionMetadata, LocalEditable, Name, PackageId, Verbatim};
use once_map::OnceMap; use once_map::OnceMap;
use pep440_rs::Version; use pep440_rs::Version;
use pep508_rs::MarkerTree;
use pypi_types::{Hashes, Metadata21}; use pypi_types::{Hashes, Metadata21};
use uv_normalize::{ExtraName, PackageName}; use uv_normalize::{ExtraName, PackageName};
@ -42,7 +43,8 @@ pub enum AnnotationStyle {
#[derive(Debug)] #[derive(Debug)]
pub struct ResolutionGraph { pub struct ResolutionGraph {
/// The underlying graph. /// The underlying graph.
petgraph: petgraph::graph::Graph<Dist, Range<Version>, petgraph::Directed>, petgraph:
petgraph::graph::Graph<Dist, (Range<Version>, Option<MarkerTree>), petgraph::Directed>,
/// The metadata for every distribution in this resolution. /// The metadata for every distribution in this resolution.
hashes: FxHashMap<PackageName, Vec<Hashes>>, hashes: FxHashMap<PackageName, Vec<Hashes>>,
/// The set of editable requirements in this resolution. /// The set of editable requirements in this resolution.
@ -64,7 +66,11 @@ impl ResolutionGraph {
) -> Result<Self, ResolveError> { ) -> Result<Self, ResolveError> {
// TODO(charlie): petgraph is a really heavy and unnecessary dependency here. We should // TODO(charlie): petgraph is a really heavy and unnecessary dependency here. We should
// write our own graph, given that our requirements are so simple. // write our own graph, given that our requirements are so simple.
let mut petgraph = petgraph::graph::Graph::with_capacity(selection.len(), selection.len()); let mut petgraph: petgraph::graph::Graph<
Dist,
(Range<Version>, Option<MarkerTree>),
petgraph::Directed,
> = petgraph::graph::Graph::with_capacity(selection.len(), selection.len());
let mut hashes = let mut hashes =
FxHashMap::with_capacity_and_hasher(selection.len(), BuildHasherDefault::default()); FxHashMap::with_capacity_and_hasher(selection.len(), BuildHasherDefault::default());
let mut diagnostics = Vec::new(); let mut diagnostics = Vec::new();
@ -207,13 +213,13 @@ impl ResolutionGraph {
for (package, version) in selection { for (package, version) in selection {
for id in &state.incompatibilities[package] { for id in &state.incompatibilities[package] {
if let Kind::FromDependencyOf( if let Kind::FromDependencyOf(
self_package, self_package2,
self_version, self_version,
dependency_package, dependency_package,
dependency_range, dependency_range,
) = &state.incompatibility_store[*id].kind ) = &state.incompatibility_store[*id].kind
{ {
let PubGrubPackage::Package(self_package, _, _) = self_package else { let PubGrubPackage::Package(self_package, _, _) = self_package2 else {
continue; continue;
}; };
let PubGrubPackage::Package(dependency_package, _, _) = dependency_package let PubGrubPackage::Package(dependency_package, _, _) = dependency_package
@ -226,13 +232,39 @@ impl ResolutionGraph {
continue; continue;
} }
let dist =
PubGrubDistribution::from_registry(self_package, &selection[self_package2]);
let requires_dist =
if let Some((_editable, metadata)) = editables.get(self_package) {
metadata.requires_dist.clone()
} else {
distributions
.get(&dist.package_id())
.expect("Missing metadata")
.requires_dist
.clone()
};
let markers: Vec<MarkerTree> = requires_dist
.iter()
.filter(|req| &req.name == dependency_package)
.filter_map(|req| req.marker.clone())
.collect();
let markers_joined = match markers.as_slice() {
[] => None,
[marker_tree] => Some(marker_tree.clone()),
// The package gets installed if any marker is fulfilled.
_ => Some(MarkerTree::Or(markers)),
};
if self_version.contains(version) { if self_version.contains(version) {
let self_index = &inverse[self_package]; let self_index = &inverse[self_package];
let dependency_index = &inverse[dependency_package]; let dependency_index = &inverse[dependency_package];
petgraph.update_edge( petgraph.update_edge(
*self_index, *self_index,
*dependency_index, *dependency_index,
dependency_range.clone(), (dependency_range.clone(), markers_joined),
); );
} }
} }
@ -270,7 +302,10 @@ impl ResolutionGraph {
} }
/// Return the underlying graph. /// Return the underlying graph.
pub fn petgraph(&self) -> &petgraph::graph::Graph<Dist, Range<Version>, petgraph::Directed> { pub fn petgraph(
&self,
) -> &petgraph::graph::Graph<Dist, (Range<Version>, Option<MarkerTree>), petgraph::Directed>
{
&self.petgraph &self.petgraph
} }
} }
@ -406,6 +441,16 @@ impl std::fmt::Display for DisplayResolutionGraph<'_> {
} }
} }
let markers: Vec<_> = self
.resolution
.petgraph
.edges_directed(index, Direction::Incoming)
.filter_map(|edge| edge.weight().1.clone())
.collect();
if !markers.is_empty() {
line.push_str(&format!(" ; {}", MarkerTree::Or(markers)));
}
// Determine the annotation comment and separator (between comment and requirement). // Determine the annotation comment and separator (between comment and requirement).
let mut annotation = None; let mut annotation = None;

View File

@ -45,10 +45,10 @@ impl ResolutionStrategy {
manifest manifest
.requirements .requirements
.iter() .iter()
.filter(|requirement| requirement.evaluate_markers(markers, &[])) .filter(|requirement| requirement.evaluate_markers2(markers, &[]))
.chain(manifest.editables.iter().flat_map(|(editable, metadata)| { .chain(manifest.editables.iter().flat_map(|(editable, metadata)| {
metadata.requires_dist.iter().filter(|requirement| { metadata.requires_dist.iter().filter(|requirement| {
requirement.evaluate_markers(markers, &editable.extras) requirement.evaluate_markers2(markers, &editable.extras)
}) })
})) }))
.map(|requirement| requirement.name.clone()) .map(|requirement| requirement.name.clone())

View File

@ -22,7 +22,7 @@ impl Urls {
.iter() .iter()
.chain(manifest.constraints.iter()) .chain(manifest.constraints.iter())
{ {
if !requirement.evaluate_markers(markers, &[]) { if !requirement.evaluate_markers2(markers, &[]) {
continue; continue;
} }
@ -56,7 +56,7 @@ impl Urls {
} }
for requirement in &metadata.requires_dist { for requirement in &metadata.requires_dist {
if !requirement.evaluate_markers(markers, &editable.extras) { if !requirement.evaluate_markers2(markers, &editable.extras) {
continue; continue;
} }
@ -79,7 +79,7 @@ impl Urls {
// Add any overrides. Conflicts here are fine, as the overrides are meant to be // Add any overrides. Conflicts here are fine, as the overrides are meant to be
// authoritative. // authoritative.
for requirement in &manifest.overrides { for requirement in &manifest.overrides {
if !requirement.evaluate_markers(markers, &[]) { if !requirement.evaluate_markers2(markers, &[]) {
continue; continue;
} }

View File

@ -20,12 +20,12 @@ impl AllowedYanks {
.chain(manifest.constraints.iter()) .chain(manifest.constraints.iter())
.chain(manifest.overrides.iter()) .chain(manifest.overrides.iter())
.chain(manifest.preferences.iter()) .chain(manifest.preferences.iter())
.filter(|requirement| requirement.evaluate_markers(markers, &[])) .filter(|requirement| requirement.evaluate_markers2(markers, &[]))
.chain(manifest.editables.iter().flat_map(|(editable, metadata)| { .chain(manifest.editables.iter().flat_map(|(editable, metadata)| {
metadata metadata
.requires_dist .requires_dist
.iter() .iter()
.filter(|requirement| requirement.evaluate_markers(markers, &editable.extras)) .filter(|requirement| requirement.evaluate_markers2(markers, &editable.extras))
})) }))
{ {
let Some(pep508_rs::VersionOrUrl::VersionSpecifier(specifiers)) = let Some(pep508_rs::VersionOrUrl::VersionSpecifier(specifiers)) =

View File

@ -163,6 +163,15 @@ pub(crate) async fn pip_sync(
) )
.await?; .await?;
let requirements: Vec<_> = requirements
.into_iter()
.filter(|req| {
req.marker.as_ref().map_or(true, |marker| {
marker.evaluate(venv.interpreter().markers(), &[])
})
})
.collect();
// Partition into those that should be linked from the cache (`local`), those that need to be // Partition into those that should be linked from the cache (`local`), those that need to be
// downloaded (`remote`), and those that should be removed (`extraneous`). // downloaded (`remote`), and those that should be removed (`extraneous`).
let Plan { let Plan {