Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use universal resolution in uv lock #3969

Merged
merged 1 commit into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading