fix the incorrect handling of markers in pip tree (#4611)

## Summary
resolves https://github.com/astral-sh/uv/issues/4609

previously, the implementation of `required_with_no_extra` was
incorrect, particularly when there are packages that do not require any
extras but have other types of markers.

## Test Plan
the existing tests also did cover this (my bad... missed it) but added a
smaller test since this bug would've been more obvious with this new
test.
This commit is contained in:
Chan Kang 2024-06-28 09:28:39 -04:00 committed by GitHub
parent bbd59ff455
commit b3b4b47394
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 97 additions and 37 deletions

View file

@ -2,6 +2,7 @@ use std::fmt::Write;
use distribution_types::{Diagnostic, InstalledDist, Name};
use owo_colors::OwoColorize;
use pep508_rs::MarkerEnvironment;
use tracing::debug;
use uv_cache::Cache;
use uv_configuration::PreviewMode;
@ -47,9 +48,15 @@ pub(crate) fn pip_tree(
// Build the installed index.
let site_packages = SitePackages::from_environment(&environment)?;
let rendered_tree = DisplayDependencyGraph::new(&site_packages, depth.into(), prune, no_dedupe)
.render()
.join("\n");
let rendered_tree = DisplayDependencyGraph::new(
&site_packages,
depth.into(),
prune,
no_dedupe,
environment.interpreter().markers(),
)
.render()
.join("\n");
writeln!(printer.stdout(), "{rendered_tree}").unwrap();
if rendered_tree.contains('*') {
writeln!(
@ -82,18 +89,16 @@ pub(crate) fn pip_tree(
// For example, `requests==2.32.3` requires `charset-normalizer`, `idna`, `urllib`, and `certifi` at
// all times, `PySocks` on `socks` extra and `chardet` on `use_chardet_on_py3` extra.
// This function will return `["charset-normalizer", "idna", "urllib", "certifi"]` for `requests`.
fn required_with_no_extra(dist: &InstalledDist) -> Vec<pep508_rs::Requirement<VerbatimParsedUrl>> {
fn required_with_no_extra(
dist: &InstalledDist,
marker_environment: &MarkerEnvironment,
) -> Vec<pep508_rs::Requirement<VerbatimParsedUrl>> {
let metadata = dist.metadata().unwrap();
return metadata
.requires_dist
.into_iter()
.filter(|r| {
r.marker.is_none()
|| !r
.marker
.as_ref()
.unwrap()
.evaluate_optional_environment(None, &metadata.provides_extras[..])
r.marker.is_none() || r.marker.as_ref().unwrap().evaluate(marker_environment, &[])
})
.collect::<Vec<_>>();
}
@ -116,6 +121,9 @@ struct DisplayDependencyGraph<'a> {
// Whether to de-duplicate the displayed dependencies.
no_dedupe: bool,
// The marker environment for the current interpreter.
marker_environment: &'a MarkerEnvironment,
}
impl<'a> DisplayDependencyGraph<'a> {
@ -125,6 +133,7 @@ impl<'a> DisplayDependencyGraph<'a> {
depth: usize,
prune: Vec<PackageName>,
no_dedupe: bool,
marker_environment: &'a MarkerEnvironment,
) -> DisplayDependencyGraph<'a> {
let mut dist_by_package_name = HashMap::new();
let mut required_packages = HashSet::new();
@ -132,7 +141,7 @@ impl<'a> DisplayDependencyGraph<'a> {
dist_by_package_name.insert(site_package.name(), site_package);
}
for site_package in site_packages.iter() {
for required in required_with_no_extra(site_package) {
for required in required_with_no_extra(site_package, marker_environment) {
required_packages.insert(required.name.clone());
}
}
@ -144,6 +153,7 @@ impl<'a> DisplayDependencyGraph<'a> {
depth,
prune,
no_dedupe,
marker_environment,
}
}
@ -182,7 +192,7 @@ impl<'a> DisplayDependencyGraph<'a> {
path.push(package_name.clone());
visited.insert(package_name.clone());
let required_packages = required_with_no_extra(installed_dist);
let required_packages = required_with_no_extra(installed_dist, self.marker_environment);
for (index, required_package) in required_packages.iter().enumerate() {
// Skip if the current package is not one of the installed distributions.
if !self

View file

@ -75,6 +75,54 @@ fn single_package() {
);
}
// `pandas` requires `numpy` with markers on Python version.
#[test]
#[cfg(not(windows))]
fn python_version_marker() {
let context = TestContext::new("3.12");
let requirements_txt = context.temp_dir.child("requirements.txt");
requirements_txt.write_str("pandas==2.2.1").unwrap();
uv_snapshot!(context
.pip_install()
.arg("-r")
.arg("requirements.txt")
.arg("--strict"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 6 packages in [TIME]
Prepared 6 packages in [TIME]
Installed 6 packages in [TIME]
+ numpy==1.26.4
+ pandas==2.2.1
+ python-dateutil==2.9.0.post0
+ pytz==2024.1
+ six==1.16.0
+ tzdata==2024.1
"###
);
uv_snapshot!(context.filters(), tree_command(&context), @r###"
success: true
exit_code: 0
----- stdout -----
pandas v2.2.1
numpy v1.26.4
python-dateutil v2.9.0.post0
six v1.16.0
pytz v2024.1
tzdata v2024.1
----- stderr -----
"###
);
}
#[test]
fn nested_dependencies() {
let context = TestContext::new("3.12");
@ -677,7 +725,7 @@ fn multiple_packages() {
let mut filters = context.filters();
if cfg!(windows) {
filters.push(("colorama v0.4.6\n", ""));
filters.push(("└── colorama v0.4.6\n", ""));
}
context.assert_command("import requests").success();
uv_snapshot!(filters, tree_command(&context), @r###"
@ -746,17 +794,17 @@ fn multiple_packages_shared_descendant() {
boto3 v1.34.69
botocore v1.34.69
jmespath v1.0.1
python-dateutil v2.9.0.post0
six v1.16.0
python-dateutil v2.9.0.post0
six v1.16.0
urllib3 v2.2.1
jmespath v1.0.1 (*)
s3transfer v0.10.1
botocore v1.34.69 (*)
pendulum v3.0.0
python-dateutil v2.9.0.post0 (*)
tzdata v2024.1
time-machine v2.14.1
python-dateutil v2.9.0.post0 (*)
urllib3 v2.2.1
tzdata v2024.1
time-machine v2.14.1
python-dateutil v2.9.0.post0 (*)
(*) Package tree already displayed
----- stderr -----
@ -838,22 +886,23 @@ fn no_dedupe_and_cycle() {
boto3 v1.34.69
botocore v1.34.69
jmespath v1.0.1
python-dateutil v2.9.0.post0
six v1.16.0
python-dateutil v2.9.0.post0
six v1.16.0
urllib3 v2.2.1
jmespath v1.0.1
s3transfer v0.10.1
botocore v1.34.69
jmespath v1.0.1
python-dateutil v2.9.0.post0
six v1.16.0
python-dateutil v2.9.0.post0
six v1.16.0
urllib3 v2.2.1
pendulum v3.0.0
python-dateutil v2.9.0.post0
six v1.16.0
tzdata v2024.1
time-machine v2.14.1
python-dateutil v2.9.0.post0
six v1.16.0
urllib3 v2.2.1
tzdata v2024.1
time-machine v2.14.1
python-dateutil v2.9.0.post0
six v1.16.0
uv-cyclic-dependencies-c v0.1.0
uv-cyclic-dependencies-a v0.1.0
uv-cyclic-dependencies-b v0.1.0
@ -915,22 +964,23 @@ fn no_dedupe() {
boto3 v1.34.69
botocore v1.34.69
jmespath v1.0.1
python-dateutil v2.9.0.post0
six v1.16.0
python-dateutil v2.9.0.post0
six v1.16.0
urllib3 v2.2.1
jmespath v1.0.1
s3transfer v0.10.1
botocore v1.34.69
jmespath v1.0.1
python-dateutil v2.9.0.post0
six v1.16.0
python-dateutil v2.9.0.post0
six v1.16.0
urllib3 v2.2.1
pendulum v3.0.0
python-dateutil v2.9.0.post0
six v1.16.0
tzdata v2024.1
time-machine v2.14.1
python-dateutil v2.9.0.post0
six v1.16.0
urllib3 v2.2.1
tzdata v2024.1
time-machine v2.14.1
python-dateutil v2.9.0.post0
six v1.16.0
----- stderr -----
"###