Skip to content

Commit

Permalink
uv-resolver: introduce new UniversalMarker type
Browse files Browse the repository at this point in the history
This effectively combines a PEP 508 marker and an as-yet-specified
marker for expressing conflicts among extras and groups.

This just defines the type and threads it through most of the various
points in the code that previously used `MarkerTree` only. Some parts
do still continue to use `MarkerTree` specifically, e.g., when dealing
with non-universal resolution or exporting to `requirements.txt`.

This doesn't change any behavior.
  • Loading branch information
BurntSushi committed Nov 21, 2024
1 parent 6b7f4e3 commit b396cf4
Show file tree
Hide file tree
Showing 18 changed files with 376 additions and 91 deletions.
8 changes: 4 additions & 4 deletions crates/uv-resolver/src/candidate_selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@ use uv_distribution_types::{CompatibleDist, IncompatibleDist, IncompatibleSource
use uv_distribution_types::{DistributionMetadata, IncompatibleWheel, Name, PrioritizedDist};
use uv_normalize::PackageName;
use uv_pep440::Version;
use uv_pep508::MarkerTree;
use uv_types::InstalledPackagesProvider;

use crate::preferences::Preferences;
use crate::prerelease::{AllowPrerelease, PrereleaseStrategy};
use crate::resolution_mode::ResolutionStrategy;
use crate::universal_marker::UniversalMarker;
use crate::version_map::{VersionMap, VersionMapDistHandle};
use crate::{Exclusions, Manifest, Options, ResolverEnvironment};

Expand Down Expand Up @@ -140,10 +140,10 @@ impl CandidateSelector {
// first has the matching half and then the mismatching half.
let preferences_match = preferences
.get(package_name)
.filter(|(marker, _index, _version)| env.included_by_marker(marker));
.filter(|(marker, _index, _version)| env.included_by_marker(marker.pep508()));
let preferences_mismatch = preferences
.get(package_name)
.filter(|(marker, _index, _version)| !env.included_by_marker(marker));
.filter(|(marker, _index, _version)| !env.included_by_marker(marker.pep508()));
let preferences = preferences_match.chain(preferences_mismatch).filter_map(
|(marker, source, version)| {
// If the package is mapped to an explicit index, only consider preferences that
Expand All @@ -167,7 +167,7 @@ impl CandidateSelector {
/// Return the first preference that satisfies the current range and is allowed.
fn get_preferred_from_iter<'a, InstalledPackages: InstalledPackagesProvider>(
&'a self,
preferences: impl Iterator<Item = (&'a MarkerTree, &'a Version)>,
preferences: impl Iterator<Item = (&'a UniversalMarker, &'a Version)>,
package_name: &'a PackageName,
range: &Range<Version>,
version_maps: &'a [VersionMap],
Expand Down
15 changes: 8 additions & 7 deletions crates/uv-resolver/src/graph_ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ use petgraph::visit::EdgeRef;
use petgraph::{Direction, Graph};
use rustc_hash::{FxBuildHasher, FxHashMap};
use std::collections::hash_map::Entry;
use uv_pep508::MarkerTree;

use crate::universal_marker::UniversalMarker;

/// Determine the markers under which a package is reachable in the dependency tree.
///
Expand All @@ -13,9 +14,9 @@ use uv_pep508::MarkerTree;
/// whenever we re-reach a node through a cycle the marker we have is a more
/// specific marker/longer path, so we don't update the node and don't re-queue it.
pub(crate) fn marker_reachability<T>(
graph: &Graph<T, MarkerTree>,
fork_markers: &[MarkerTree],
) -> FxHashMap<NodeIndex, MarkerTree> {
graph: &Graph<T, UniversalMarker>,
fork_markers: &[UniversalMarker],
) -> FxHashMap<NodeIndex, UniversalMarker> {
// Note that we build including the virtual packages due to how we propagate markers through
// the graph, even though we then only read the markers for base packages.
let mut reachability = FxHashMap::with_capacity_and_hasher(graph.node_count(), FxBuildHasher);
Expand All @@ -36,12 +37,12 @@ pub(crate) fn marker_reachability<T>(

// The root nodes are always applicable, unless the user has restricted resolver
// environments with `tool.uv.environments`.
let root_markers: MarkerTree = if fork_markers.is_empty() {
MarkerTree::TRUE
let root_markers = if fork_markers.is_empty() {
UniversalMarker::TRUE
} else {
fork_markers
.iter()
.fold(MarkerTree::FALSE, |mut acc, marker| {
.fold(UniversalMarker::FALSE, |mut acc, marker| {
acc.or(marker.clone());
acc
})
Expand Down
1 change: 1 addition & 0 deletions crates/uv-resolver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,5 +56,6 @@ mod requires_python;
mod resolution;
mod resolution_mode;
mod resolver;
mod universal_marker;
mod version_map;
mod yanks;
92 changes: 62 additions & 30 deletions crates/uv-resolver/src/lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub use crate::lock::target::InstallTarget;
pub use crate::lock::tree::TreeDisplay;
use crate::requires_python::SimplifiedMarkerTree;
use crate::resolution::{AnnotatedDist, ResolutionGraphNode};
use crate::universal_marker::UniversalMarker;
use crate::{
ExcludeNewer, InMemoryIndex, MetadataResponse, PrereleaseMode, RequiresPython, ResolutionMode,
ResolverOutput,
Expand Down Expand Up @@ -55,23 +56,26 @@ mod tree;
/// The current version of the lockfile format.
pub const VERSION: u32 = 1;

static LINUX_MARKERS: LazyLock<MarkerTree> = LazyLock::new(|| {
MarkerTree::from_str(
static LINUX_MARKERS: LazyLock<UniversalMarker> = LazyLock::new(|| {
let pep508 = MarkerTree::from_str(
"platform_system == 'Linux' and os_name == 'posix' and sys_platform == 'linux'",
)
.unwrap()
.unwrap();
UniversalMarker::new(pep508, MarkerTree::TRUE)
});
static WINDOWS_MARKERS: LazyLock<MarkerTree> = LazyLock::new(|| {
MarkerTree::from_str(
static WINDOWS_MARKERS: LazyLock<UniversalMarker> = LazyLock::new(|| {
let pep508 = MarkerTree::from_str(
"platform_system == 'Windows' and os_name == 'nt' and sys_platform == 'win32'",
)
.unwrap()
.unwrap();
UniversalMarker::new(pep508, MarkerTree::TRUE)
});
static MAC_MARKERS: LazyLock<MarkerTree> = LazyLock::new(|| {
MarkerTree::from_str(
static MAC_MARKERS: LazyLock<UniversalMarker> = LazyLock::new(|| {
let pep508 = MarkerTree::from_str(
"platform_system == 'Darwin' and os_name == 'posix' and sys_platform == 'darwin'",
)
.unwrap()
.unwrap();
UniversalMarker::new(pep508, MarkerTree::TRUE)
});

#[derive(Clone, Debug, serde::Deserialize)]
Expand All @@ -80,7 +84,7 @@ pub struct Lock {
version: u32,
/// If this lockfile was built from a forking resolution with non-identical forks, store the
/// forks in the lockfile so we can recreate them in subsequent resolutions.
fork_markers: Vec<MarkerTree>,
fork_markers: Vec<UniversalMarker>,
/// The conflicting groups/extras specified by the user.
conflicts: Conflicts,
/// The list of supported environments specified by the user.
Expand Down Expand Up @@ -315,7 +319,7 @@ impl Lock {
manifest: ResolverManifest,
conflicts: Conflicts,
supported_environments: Vec<MarkerTree>,
fork_markers: Vec<MarkerTree>,
fork_markers: Vec<UniversalMarker>,
) -> Result<Self, LockError> {
// Put all dependencies for each package in a canonical order and
// check for duplicates.
Expand Down Expand Up @@ -597,7 +601,7 @@ impl Lock {

/// If this lockfile was built from a forking resolution with non-identical forks, return the
/// markers of those forks, otherwise `None`.
pub fn fork_markers(&self) -> &[MarkerTree] {
pub fn fork_markers(&self) -> &[UniversalMarker] {
self.fork_markers.as_slice()
}

Expand All @@ -614,7 +618,13 @@ impl Lock {
let fork_markers = each_element_on_its_line_array(
self.fork_markers
.iter()
.map(|marker| SimplifiedMarkerTree::new(&self.requires_python, marker.clone()))
.map(|marker| {
// TODO(ag): Consider whether `resolution-markers` should actually
// include conflicting marker info. In which case, we should serialize
// the entire `UniversalMarker` (taking care to still make the PEP 508
// simplified).
SimplifiedMarkerTree::new(&self.requires_python, marker.pep508().clone())
})
.filter_map(|marker| marker.try_to_string()),
);
doc.insert("resolution-markers", value(fork_markers));
Expand Down Expand Up @@ -1434,6 +1444,9 @@ impl TryFrom<LockWire> for Lock {
.fork_markers
.into_iter()
.map(|simplified_marker| simplified_marker.into_marker(&wire.requires_python))
// TODO(ag): Consider whether this should also deserialize a conflict marker.
// We currently aren't serializing. Dropping it completely is likely to be wrong.
.map(|complexified_marker| UniversalMarker::new(complexified_marker, MarkerTree::TRUE))
.collect();
let lock = Lock::new(
wire.version,
Expand Down Expand Up @@ -1475,7 +1488,7 @@ pub struct Package {
/// the next resolution.
///
/// Named `resolution-markers` in `uv.lock`.
fork_markers: Vec<MarkerTree>,
fork_markers: Vec<UniversalMarker>,
/// The resolved dependencies of the package.
dependencies: Vec<Dependency>,
/// The resolved optional dependencies of the package.
Expand All @@ -1489,7 +1502,7 @@ pub struct Package {
impl Package {
fn from_annotated_dist(
annotated_dist: &AnnotatedDist,
fork_markers: Vec<MarkerTree>,
fork_markers: Vec<UniversalMarker>,
root: &Path,
) -> Result<Self, LockError> {
let id = PackageId::from_annotated_dist(annotated_dist, root)?;
Expand Down Expand Up @@ -1549,7 +1562,7 @@ impl Package {
&mut self,
requires_python: &RequiresPython,
annotated_dist: &AnnotatedDist,
marker: MarkerTree,
marker: UniversalMarker,
root: &Path,
) -> Result<(), LockError> {
let new_dep =
Expand Down Expand Up @@ -1595,7 +1608,7 @@ impl Package {
requires_python: &RequiresPython,
extra: ExtraName,
annotated_dist: &AnnotatedDist,
marker: MarkerTree,
marker: UniversalMarker,
root: &Path,
) -> Result<(), LockError> {
let dep = Dependency::from_annotated_dist(requires_python, annotated_dist, marker, root)?;
Expand All @@ -1621,7 +1634,7 @@ impl Package {
requires_python: &RequiresPython,
group: GroupName,
annotated_dist: &AnnotatedDist,
marker: MarkerTree,
marker: UniversalMarker,
root: &Path,
) -> Result<(), LockError> {
let dep = Dependency::from_annotated_dist(requires_python, annotated_dist, marker, root)?;
Expand Down Expand Up @@ -1957,7 +1970,13 @@ impl Package {
let wheels = each_element_on_its_line_array(
self.fork_markers
.iter()
.map(|marker| SimplifiedMarkerTree::new(requires_python, marker.clone()))
// TODO(ag): Consider whether `resolution-markers` should actually
// include conflicting marker info. In which case, we should serialize
// the entire `UniversalMarker` (taking care to still make the PEP 508
// simplified).
.map(|marker| {
SimplifiedMarkerTree::new(requires_python, marker.pep508().clone())
})
.filter_map(|marker| marker.try_to_string()),
);
table.insert("resolution-markers", value(wheels));
Expand Down Expand Up @@ -2112,7 +2131,7 @@ impl Package {
}

/// Return the fork markers for this package, if any.
pub fn fork_markers(&self) -> &[MarkerTree] {
pub fn fork_markers(&self) -> &[UniversalMarker] {
self.fork_markers.as_slice()
}

Expand Down Expand Up @@ -2223,6 +2242,11 @@ impl PackageWire {
.fork_markers
.into_iter()
.map(|simplified_marker| simplified_marker.into_marker(requires_python))
// TODO(ag): Consider whether this should also deserialize a conflict marker.
// We currently aren't serializing. Dropping it completely is likely to be wrong.
.map(|complexified_marker| {
UniversalMarker::new(complexified_marker, MarkerTree::TRUE)
})
.collect(),
dependencies: unwire_deps(self.dependencies)?,
optional_dependencies: self
Expand Down Expand Up @@ -3477,8 +3501,9 @@ impl TryFrom<WheelWire> for Wheel {
struct Dependency {
package_id: PackageId,
extra: BTreeSet<ExtraName>,
/// A marker simplified by assuming `requires-python` is satisfied.
/// So if `requires-python = '>=3.8'`, then
/// A marker simplified from the PEP 508 marker in `complexified_marker`
/// by assuming `requires-python` is satisfied. So if
/// `requires-python = '>=3.8'`, then
/// `python_version >= '3.8' and python_version < '3.12'`
/// gets simplfiied to `python_version < '3.12'`.
///
Expand All @@ -3496,21 +3521,21 @@ struct Dependency {
/// `requires-python` applies to the entire lock file, it's
/// acceptable to do comparisons on the simplified form.
simplified_marker: SimplifiedMarkerTree,
/// The "complexified" marker is a marker that can stand on its
/// own independent of `requires-python`. It can be safely used
/// for any kind of marker algebra.
complexified_marker: MarkerTree,
/// The "complexified" marker is a universal marker whose PEP 508
/// marker can stand on its own independent of `requires-python`.
/// It can be safely used for any kind of marker algebra.
complexified_marker: UniversalMarker,
}

impl Dependency {
fn new(
requires_python: &RequiresPython,
package_id: PackageId,
extra: BTreeSet<ExtraName>,
complexified_marker: MarkerTree,
complexified_marker: UniversalMarker,
) -> Dependency {
let simplified_marker =
SimplifiedMarkerTree::new(requires_python, complexified_marker.clone());
SimplifiedMarkerTree::new(requires_python, complexified_marker.pep508().clone());
Dependency {
package_id,
extra,
Expand All @@ -3522,7 +3547,7 @@ impl Dependency {
fn from_annotated_dist(
requires_python: &RequiresPython,
annotated_dist: &AnnotatedDist,
complexified_marker: MarkerTree,
complexified_marker: UniversalMarker,
root: &Path,
) -> Result<Dependency, LockError> {
let package_id = PackageId::from_annotated_dist(annotated_dist, root)?;
Expand Down Expand Up @@ -3590,6 +3615,7 @@ struct DependencyWire {
extra: BTreeSet<ExtraName>,
#[serde(default)]
marker: SimplifiedMarkerTree,
// FIXME: Add support for representing conflict markers.
}

impl DependencyWire {
Expand All @@ -3603,7 +3629,8 @@ impl DependencyWire {
package_id: self.package_id.unwire(unambiguous_package_ids)?,
extra: self.extra,
simplified_marker: self.marker,
complexified_marker,
// FIXME: Support reading conflict markers.
complexified_marker: UniversalMarker::new(complexified_marker, MarkerTree::TRUE),
})
}
}
Expand Down Expand Up @@ -4103,6 +4130,11 @@ enum LockErrorKind {
#[source]
err: DependencyGroupError,
},
/// An error that occurs when trying to export a `uv.lock` with
/// conflicting extras/groups specified to `requirements.txt`.
/// (Because `requirements.txt` cannot encode them.)
#[error("Cannot represent `uv.lock` with conflicting extras or groups as `requirements.txt`")]
ConflictsNotAllowedInRequirementsTxt,
}

/// An error that occurs when a source string could not be parsed.
Expand Down
Loading

0 comments on commit b396cf4

Please sign in to comment.