Skip to content

Commit

Permalink
uv-resolver: add new ConflictMarker type
Browse files Browse the repository at this point in the history
This is strictly a refactor. It doesn't change any behavior.

The purpose of this refactor is to make room for supporting more complex
conflict markers. Namely, we need to incorporate package names and then
eventually group names. But we also need sophisticated boolean algebraic
simplification, and the only (cheap, in terms of development cost) to do
that at the moment is to encode our semantics into PEP 508 markers. So
this type will own that encapsulation. (Right now, there's no real
translation happening because we only support extra names, which PEP 508
already supports.)
  • Loading branch information
BurntSushi committed Nov 23, 2024
1 parent e7093c6 commit a468f5a
Show file tree
Hide file tree
Showing 9 changed files with 239 additions and 67 deletions.
2 changes: 1 addition & 1 deletion crates/uv-resolver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ pub use resolver::{
PackageVersionsResult, Reporter as ResolverReporter, Resolver, ResolverEnvironment,
ResolverProvider, VersionsResponse, WheelMetadataResult,
};
pub use universal_marker::UniversalMarker;
pub use universal_marker::{ConflictMarker, UniversalMarker};
pub use version_map::VersionMap;
pub use yanks::AllowedYanks;

Expand Down
23 changes: 14 additions & 9 deletions crates/uv-resolver/src/lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +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::universal_marker::{ConflictMarker, UniversalMarker};
use crate::{
ExcludeNewer, InMemoryIndex, MetadataResponse, PrereleaseMode, RequiresPython, ResolutionMode,
ResolverOutput,
Expand Down Expand Up @@ -61,21 +61,21 @@ static LINUX_MARKERS: LazyLock<UniversalMarker> = LazyLock::new(|| {
"platform_system == 'Linux' and os_name == 'posix' and sys_platform == 'linux'",
)
.unwrap();
UniversalMarker::new(pep508, MarkerTree::TRUE)
UniversalMarker::new(pep508, ConflictMarker::TRUE)
});
static WINDOWS_MARKERS: LazyLock<UniversalMarker> = LazyLock::new(|| {
let pep508 = MarkerTree::from_str(
"platform_system == 'Windows' and os_name == 'nt' and sys_platform == 'win32'",
)
.unwrap();
UniversalMarker::new(pep508, MarkerTree::TRUE)
UniversalMarker::new(pep508, ConflictMarker::TRUE)
});
static MAC_MARKERS: LazyLock<UniversalMarker> = LazyLock::new(|| {
let pep508 = MarkerTree::from_str(
"platform_system == 'Darwin' and os_name == 'posix' and sys_platform == 'darwin'",
)
.unwrap();
UniversalMarker::new(pep508, MarkerTree::TRUE)
UniversalMarker::new(pep508, ConflictMarker::TRUE)
});

#[derive(Clone, Debug, serde::Deserialize)]
Expand Down Expand Up @@ -1446,7 +1446,9 @@ impl TryFrom<LockWire> for Lock {
.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))
.map(|complexified_marker| {
UniversalMarker::new(complexified_marker, ConflictMarker::TRUE)
})
.collect();
let lock = Lock::new(
wire.version,
Expand Down Expand Up @@ -2245,7 +2247,7 @@ impl PackageWire {
// 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)
UniversalMarker::new(complexified_marker, ConflictMarker::TRUE)
})
.collect(),
dependencies: unwire_deps(self.dependencies)?,
Expand Down Expand Up @@ -3580,8 +3582,11 @@ impl Dependency {
if let Some(marker) = self.simplified_marker.try_to_string() {
table.insert("marker", value(marker));
}
if let Some(conflict_marker) = self.complexified_marker.conflict().try_to_string() {
table.insert("conflict-marker", value(conflict_marker));
if !self.complexified_marker.conflict().is_true() {
table.insert(
"conflict-marker",
value(self.complexified_marker.conflict().to_string()),
);
}

table
Expand Down Expand Up @@ -3619,7 +3624,7 @@ struct DependencyWire {
#[serde(default)]
marker: SimplifiedMarkerTree,
#[serde(rename = "conflict-marker", default)]
conflict_marker: MarkerTree,
conflict_marker: ConflictMarker,
}

impl DependencyWire {
Expand Down
6 changes: 3 additions & 3 deletions crates/uv-resolver/src/lock/requirements_txt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use uv_pypi_types::{ParsedArchiveUrl, ParsedGitUrl};

use crate::graph_ops::marker_reachability;
use crate::lock::{LockErrorKind, Package, PackageId, Source};
use crate::universal_marker::UniversalMarker;
use crate::universal_marker::{ConflictMarker, UniversalMarker};
use crate::{InstallTarget, LockError};

/// An export of a [`Lock`] that renders in `requirements.txt` format.
Expand Down Expand Up @@ -123,7 +123,7 @@ impl<'lock> RequirementsTxtExport<'lock> {
// `marker_reachability` wants and it (probably) isn't
// worth inventing a new abstraction so that it can accept
// graphs with either `MarkerTree` or `UniversalMarker`.
MarkerTree::TRUE,
ConflictMarker::TRUE,
),
);

Expand Down Expand Up @@ -172,7 +172,7 @@ impl<'lock> RequirementsTxtExport<'lock> {
dep.simplified_marker.as_simplified_marker_tree().clone(),
// See note above for other `UniversalMarker::new` for
// why this is OK.
MarkerTree::TRUE,
ConflictMarker::TRUE,
),
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ Ok(
),
complexified_marker: UniversalMarker {
pep508_marker: python_full_version >= '3.12',
conflict_marker: true,
conflict_marker: ConflictMarker(true),
},
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ Ok(
),
complexified_marker: UniversalMarker {
pep508_marker: python_full_version >= '3.12',
conflict_marker: true,
conflict_marker: ConflictMarker(true),
},
},
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ Ok(
),
complexified_marker: UniversalMarker {
pep508_marker: python_full_version >= '3.12',
conflict_marker: true,
conflict_marker: ConflictMarker(true),
},
},
],
Expand Down
17 changes: 5 additions & 12 deletions crates/uv-resolver/src/resolver/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use uv_pypi_types::{ConflictItem, ConflictItemRef, ResolverMarkerEnvironment};
use crate::pubgrub::{PubGrubDependency, PubGrubPackage};
use crate::requires_python::RequiresPythonRange;
use crate::resolver::ForkState;
use crate::universal_marker::UniversalMarker;
use crate::universal_marker::{ConflictMarker, UniversalMarker};
use crate::PythonRequirement;

/// Represents one or more marker environments for a resolution.
Expand Down Expand Up @@ -380,23 +380,16 @@ impl ResolverEnvironment {
..
} => {
// FIXME: Account for groups here in addition to extras.
let mut conflict_marker = MarkerTree::TRUE;
let mut conflict_marker = ConflictMarker::TRUE;
for item in exclude.iter() {
if let Some(extra) = item.extra() {
let operator = uv_pep508::ExtraOperator::NotEqual;
let name = uv_pep508::MarkerValueExtra::Extra(extra.clone());
let expr = uv_pep508::MarkerExpression::Extra { operator, name };
let exclude_extra_marker = MarkerTree::expression(expr);
conflict_marker.and(exclude_extra_marker);
conflict_marker =
conflict_marker.and(ConflictMarker::extra(extra.clone()).negate());
}
}
for item in include.iter() {
if let Some(extra) = item.extra() {
let operator = uv_pep508::ExtraOperator::Equal;
let name = uv_pep508::MarkerValueExtra::Extra(extra.clone());
let expr = uv_pep508::MarkerExpression::Extra { operator, name };
let exclude_extra_marker = MarkerTree::expression(expr);
conflict_marker.and(exclude_extra_marker);
conflict_marker = conflict_marker.and(ConflictMarker::extra(extra.clone()));
}
}
Some(UniversalMarker::new(markers.clone(), conflict_marker))
Expand Down
4 changes: 2 additions & 2 deletions crates/uv-resolver/src/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub(crate) use crate::resolver::availability::{
};
use crate::resolver::batch_prefetch::BatchPrefetcher;
pub use crate::resolver::derivation::DerivationChainBuilder;
use crate::universal_marker::UniversalMarker;
use crate::universal_marker::{ConflictMarker, UniversalMarker};

use crate::resolver::groups::Groups;
pub use crate::resolver::index::InMemoryIndex;
Expand Down Expand Up @@ -2617,7 +2617,7 @@ impl ResolutionDependencyEdge {
// We specifically do not account for conflict
// markers here. Instead, those are computed via
// a traversal on the resolution graph.
UniversalMarker::new(self.marker.clone(), MarkerTree::TRUE)
UniversalMarker::new(self.marker.clone(), ConflictMarker::TRUE)
}
}

Expand Down
Loading

0 comments on commit a468f5a

Please sign in to comment.