Skip to content

Commit

Permalink
Use universal resolution in uv lock (#3969)
Browse files Browse the repository at this point in the history
## Summary

Wires up the optional markers in resolution, and adds
respecting-the-markers to `Lock:: to_resolution`.
  • Loading branch information
charliermarsh committed Jun 3, 2024
1 parent c500b78 commit a23ca5b
Show file tree
Hide file tree
Showing 10 changed files with 195 additions and 54 deletions.
21 changes: 14 additions & 7 deletions crates/uv-resolver/src/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use distribution_types::{
RegistrySourceDist, RemoteSource, Resolution, ResolvedDist, ToUrlError,
};
use pep440_rs::Version;
use pep508_rs::{MarkerEnvironment, VerbatimUrl};
use pep508_rs::{MarkerEnvironment, MarkerTree, VerbatimUrl};
use platform_tags::{TagCompatibility, TagPriority, Tags};
use pypi_types::{HashDigest, ParsedArchiveUrl, ParsedGitUrl};
use uv_git::{GitReference, GitSha, RepositoryReference, ResolvedRepositoryReference};
Expand Down Expand Up @@ -82,10 +82,16 @@ impl Lock {
while let Some(dist) = queue.pop_front() {
for dep in &dist.dependencies {
let dep_dist = self.find_by_id(&dep.id);
queue.push_back(dep_dist);
if dep_dist
.marker
.as_ref()
.map_or(true, |marker| marker.evaluate(marker_env, &[]))
{
queue.push_back(dep_dist);
}
}
let name = dist.id.name.clone();
let resolved_dist = ResolvedDist::Installable(dist.to_dist(marker_env, tags));
let resolved_dist = ResolvedDist::Installable(dist.to_dist(tags));
map.insert(name, resolved_dist);
}
let diagnostics = vec![];
Expand Down Expand Up @@ -280,7 +286,7 @@ pub struct Distribution {
#[serde(flatten)]
pub(crate) id: DistributionId,
#[serde(default)]
pub(crate) marker: Option<String>,
pub(crate) marker: Option<MarkerTree>,
#[serde(default)]
pub(crate) sdist: Option<SourceDist>,
#[serde(default, skip_serializing_if = "Vec::is_empty")]
Expand All @@ -294,11 +300,12 @@ impl Distribution {
annotated_dist: &AnnotatedDist,
) -> Result<Distribution, LockError> {
let id = DistributionId::from_annotated_dist(annotated_dist);
let wheels = Wheel::from_annotated_dist(annotated_dist)?;
let marker = annotated_dist.marker.clone();
let sdist = SourceDist::from_annotated_dist(annotated_dist)?;
let wheels = Wheel::from_annotated_dist(annotated_dist)?;
Ok(Distribution {
id,
marker: annotated_dist.marker.as_ref().map(ToString::to_string),
marker,
sdist,
wheels,
dependencies: vec![],
Expand All @@ -311,7 +318,7 @@ impl Distribution {
}

/// Convert the [`Distribution`] to a [`Dist`] that can be used in installation.
fn to_dist(&self, _marker_env: &MarkerEnvironment, tags: &Tags) -> Dist {
fn to_dist(&self, tags: &Tags) -> Dist {
if let Some(best_wheel_index) = self.find_best_wheel(tags) {
return match &self.id.source.kind {
SourceKind::Registry => {
Expand Down
11 changes: 7 additions & 4 deletions crates/uv-resolver/src/python_requirement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,18 @@ pub struct PythonRequirement {
}

impl PythonRequirement {
pub fn new(interpreter: &Interpreter, target: &StringVersion) -> Self {
pub fn from_marker_environment(interpreter: &Interpreter, env: &MarkerEnvironment) -> Self {
Self {
installed: interpreter.python_full_version().clone(),
target: target.clone(),
target: env.python_full_version().clone(),
}
}

pub fn from_marker_environment(interpreter: &Interpreter, env: &MarkerEnvironment) -> Self {
Self::new(interpreter, env.python_full_version())
pub fn from_interpreter(interpreter: &Interpreter) -> Self {
Self {
installed: interpreter.python_full_version().clone(),
target: interpreter.python_full_version().clone(),
}
}

/// Return the installed version of Python.
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/src/commands/pip/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ pub(crate) async fn pip_install(
&upgrade,
&interpreter,
&tags,
&markers,
Some(&markers),
&client,
&flat_index,
&index,
Expand Down
12 changes: 8 additions & 4 deletions crates/uv/src/commands/pip/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ pub(crate) async fn resolve<InstalledPackages: InstalledPackagesProvider>(
upgrade: &Upgrade,
interpreter: &Interpreter,
tags: &Tags,
markers: &MarkerEnvironment,
markers: Option<&MarkerEnvironment>,
client: &RegistryClient,
flat_index: &FlatIndex,
index: &InMemoryIndex,
Expand Down Expand Up @@ -182,7 +182,11 @@ pub(crate) async fn resolve<InstalledPackages: InstalledPackagesProvider>(
// Collect constraints and overrides.
let constraints = Constraints::from_requirements(constraints);
let overrides = Overrides::from_requirements(overrides);
let python_requirement = PythonRequirement::from_marker_environment(interpreter, markers);
let python_requirement = if let Some(markers) = markers {
PythonRequirement::from_marker_environment(interpreter, markers)
} else {
PythonRequirement::from_interpreter(interpreter)
};

// Determine any lookahead requirements.
let lookaheads = match options.dependency_mode {
Expand All @@ -196,7 +200,7 @@ pub(crate) async fn resolve<InstalledPackages: InstalledPackagesProvider>(
DistributionDatabase::new(client, build_dispatch, concurrency.downloads, preview),
)
.with_reporter(ResolverReporter::from(printer))
.resolve(Some(markers))
.resolve(markers)
.await?
}
DependencyMode::Direct => Vec::new(),
Expand Down Expand Up @@ -230,7 +234,7 @@ pub(crate) async fn resolve<InstalledPackages: InstalledPackagesProvider>(
manifest,
options,
&python_requirement,
Some(markers),
markers,
tags,
flat_index,
index,
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/src/commands/pip/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ pub(crate) async fn pip_sync(
&upgrade,
interpreter,
&tags,
&markers,
Some(&markers),
&client,
&flat_index,
&index,
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/src/commands/project/lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ pub(super) async fn do_lock(
&upgrade,
&interpreter,
tags,
markers,
None,
&client,
&flat_index,
&index,
Expand Down
2 changes: 1 addition & 1 deletion crates/uv/src/commands/project/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ pub(crate) async fn update_environment(
&upgrade,
&interpreter,
tags,
markers,
Some(markers),
&client,
&flat_index,
&index,
Expand Down
73 changes: 48 additions & 25 deletions crates/uv/tests/common/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,14 +514,20 @@ pub fn python_path_with_versions(
Ok(env::join_paths(selected_pythons)?)
}

#[derive(Debug, Copy, Clone)]
pub enum WindowsFilters {
Platform,
Universal,
}

/// Execute the command and format its output status, stdout and stderr into a snapshot string.
///
/// This function is derived from `insta_cmd`s `spawn_with_info`.
pub fn run_and_format<T: AsRef<str>>(
mut command: impl BorrowMut<std::process::Command>,
filters: impl AsRef<[(T, T)]>,
function_name: &str,
windows_filters: bool,
windows_filters: Option<WindowsFilters>,
) -> (String, Output) {
let program = command
.borrow_mut()
Expand Down Expand Up @@ -566,29 +572,40 @@ pub fn run_and_format<T: AsRef<str>>(
// pass whether it's on Windows or Unix. In particular, there are some very
// common Windows-only dependencies that, when removed from a resolution,
// cause the set of dependencies to be the same across platforms.
if cfg!(windows) && windows_filters {
// The optional leading +/- is for install logs, the optional next line is for lock files
let windows_only_deps = [
("( [+-] )?colorama==\\d+(\\.[\\d+])+\n( # via .*\n)?"),
("( [+-] )?colorama==\\d+(\\.[\\d+])+\\s+(# via .*\n)?"),
("( [+-] )?tzdata==\\d+(\\.[\\d+])+\n( # via .*\n)?"),
("( [+-] )?tzdata==\\d+(\\.[\\d+])+\\s+(# via .*\n)?"),
];
let mut removed_packages = 0;
for windows_only_dep in windows_only_deps {
// TODO(konstin): Cache regex compilation
let re = Regex::new(windows_only_dep).unwrap();
if re.is_match(&snapshot) {
snapshot = re.replace(&snapshot, "").to_string();
removed_packages += 1;
if cfg!(windows) {
if let Some(windows_filters) = windows_filters {
// The optional leading +/- is for install logs, the optional next line is for lock files
let windows_only_deps = [
("( [+-] )?colorama==\\d+(\\.[\\d+])+\n( # via .*\n)?"),
("( [+-] )?colorama==\\d+(\\.[\\d+])+\\s+(# via .*\n)?"),
("( [+-] )?tzdata==\\d+(\\.[\\d+])+\n( # via .*\n)?"),
("( [+-] )?tzdata==\\d+(\\.[\\d+])+\\s+(# via .*\n)?"),
];
let mut removed_packages = 0;
for windows_only_dep in windows_only_deps {
// TODO(konstin): Cache regex compilation
let re = Regex::new(windows_only_dep).unwrap();
if re.is_match(&snapshot) {
snapshot = re.replace(&snapshot, "").to_string();
removed_packages += 1;
}
}
}
if removed_packages > 0 {
for i in 1..20 {
snapshot = snapshot.replace(
&format!("{} packages", i + removed_packages),
&format!("{} package{}", i, if i > 1 { "s" } else { "" }),
);
if removed_packages > 0 {
for i in 1..20 {
for verb in match windows_filters {
WindowsFilters::Platform => {
["Resolved", "Downloaded", "Installed", "Uninstalled"].iter()
}
WindowsFilters::Universal => {
["Downloaded", "Installed", "Uninstalled"].iter()
}
} {
snapshot = snapshot.replace(
&format!("{verb} {} packages", i + removed_packages),
&format!("{verb} {} package{}", i, if i > 1 { "s" } else { "" }),
);
}
}
}
}
}
Expand Down Expand Up @@ -641,13 +658,19 @@ macro_rules! uv_snapshot {
}};
($filters:expr, $spawnable:expr, @$snapshot:literal) => {{
// Take a reference for backwards compatibility with the vec-expecting insta filters.
let (snapshot, output) = $crate::common::run_and_format($spawnable, &$filters, function_name!(), true);
let (snapshot, output) = $crate::common::run_and_format($spawnable, &$filters, function_name!(), Some($crate::common::WindowsFilters::Platform));
::insta::assert_snapshot!(snapshot, @$snapshot);
output
}};
($filters:expr, windows_filters=false, $spawnable:expr, @$snapshot:literal) => {{
// Take a reference for backwards compatibility with the vec-expecting insta filters.
let (snapshot, output) = $crate::common::run_and_format($spawnable, &$filters, function_name!(), false);
let (snapshot, output) = $crate::common::run_and_format($spawnable, &$filters, function_name!(), None);
::insta::assert_snapshot!(snapshot, @$snapshot);
output
}};
($filters:expr, universal_windows_filters=true, $spawnable:expr, @$snapshot:literal) => {{
// Take a reference for backwards compatibility with the vec-expecting insta filters.
let (snapshot, output) = $crate::common::run_and_format($spawnable, &$filters, function_name!(), Some($crate::common::WindowsFilters::Universal));
::insta::assert_snapshot!(snapshot, @$snapshot);
output
}};
Expand Down
Loading

0 comments on commit a23ca5b

Please sign in to comment.