Resolve non-determistic behavior in preferences due to site-packages ordering (#2780)

Originally a regression test for #2779 but we found out that there's
some weird behavior where different `anyio` versions were preferred
based on the platform.
This commit is contained in:
Zanie Blue 2024-04-02 13:48:33 -05:00 committed by GitHub
parent 119d753cfe
commit 1ac9672b95
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 60 additions and 43 deletions

View file

@ -46,7 +46,13 @@ impl<'a> SitePackages<'a> {
for site_packages in venv.site_packages() { for site_packages in venv.site_packages() {
// Read the site-packages directory. // Read the site-packages directory.
let site_packages = match fs::read_dir(site_packages) { let site_packages = match fs::read_dir(site_packages) {
Ok(site_packages) => site_packages, Ok(site_packages) => {
let mut entries = site_packages.collect::<Result<Vec<_>, std::io::Error>>()?;
// TODO(zanieb): Consider filtering to just directories to reduce the size of the sort
// Sort for determinism, `read_dir` is different per-platform
entries.sort_by_key(fs_err::DirEntry::path);
entries
}
Err(err) if err.kind() == std::io::ErrorKind::NotFound => { Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
return Ok(Self { return Ok(Self {
venv, venv,
@ -60,7 +66,6 @@ impl<'a> SitePackages<'a> {
// Index all installed packages by name. // Index all installed packages by name.
for entry in site_packages { for entry in site_packages {
let entry = entry?;
if entry.file_type()?.is_dir() { if entry.file_type()?.is_dir() {
let path = entry.path(); let path = entry.path();

View file

@ -100,7 +100,9 @@ impl CandidateSelector {
} }
// We do not consider installed distributions with multiple versions because // We do not consider installed distributions with multiple versions because
// during installation these must be reinstalled from the remote // during installation these must be reinstalled from the remote
_ => {} _ => {
debug!("Ignoring installed versions of {package_name}: multiple distributions found");
}
} }
} }
@ -130,7 +132,9 @@ impl CandidateSelector {
} }
// We do not consider installed distributions with multiple versions because // We do not consider installed distributions with multiple versions because
// during installation these must be reinstalled from the remote // during installation these must be reinstalled from the remote
_ => {} _ => {
debug!("Ignoring installed versions of {package_name}: multiple distributions found");
}
} }
} }

View file

@ -79,6 +79,9 @@ impl Preferences {
markers: &MarkerEnvironment, markers: &MarkerEnvironment,
) -> Self { ) -> Self {
Self( Self(
// TODO(zanieb): We should explicitly ensure that when a package name is seen multiple times
// that the newest or oldest version is preferred dependning on the resolution strategy;
// right now, the order is dependent on the given iterator.
preferences preferences
.into_iter() .into_iter()
.filter_map(|preference| { .filter_map(|preference| {

View file

@ -3517,53 +3517,38 @@ fn already_installed_local_version_of_remote_package() {
#[test] #[test]
#[cfg(unix)] #[cfg(unix)]
fn already_installed_multiple_versions() -> Result<()> { fn already_installed_multiple_versions() -> Result<()> {
use crate::common::copy_dir_all; fn prepare(context: &TestContext) -> Result<()> {
use crate::common::copy_dir_all;
// Install a version // Install into the base environment
let context1 = TestContext::new("3.12"); context.install().arg("anyio==3.7.0").assert().success();
uv_snapshot!(context1.filters(), context1.install()
.arg("anyio==3.7.0"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr ----- // Install another version into another environment
Resolved 3 packages in [TIME] let context_duplicate = TestContext::new("3.12");
Downloaded 3 packages in [TIME] context_duplicate
Installed 3 packages in [TIME] .install()
+ anyio==3.7.0 .arg("anyio==4.0.0")
+ idna==3.6 .assert()
+ sniffio==1.3.1 .success();
"###
);
// Install another version // Copy the second version into the first environment
let context2 = TestContext::new("3.12"); copy_dir_all(
uv_snapshot!(context2.filters(), context2.install() context_duplicate
.arg("anyio==4.0.0"), @r###" .site_packages()
success: true .join("anyio-4.0.0.dist-info"),
exit_code: 0 context.site_packages().join("anyio-4.0.0.dist-info"),
----- stdout ----- )?;
----- stderr ----- Ok(())
Resolved 3 packages in [TIME] }
Downloaded 3 packages in [TIME]
Installed 3 packages in [TIME]
+ anyio==4.0.0
+ idna==3.6
+ sniffio==1.3.1
"###
);
// Copy the second version into the first environment let context = TestContext::new("3.12");
copy_dir_all(
context2.site_packages().join("anyio-4.0.0.dist-info"), prepare(&context)?;
context1.site_packages().join("anyio-4.0.0.dist-info"),
)?;
// Request the second anyio version again // Request the second anyio version again
// Should remove both previous versions and reinstall the second one // Should remove both previous versions and reinstall the second one
uv_snapshot!(context1.filters(), context1.install().arg("anyio==4.0.0"), @r###" uv_snapshot!(context.filters(), context.install().arg("anyio==4.0.0"), @r###"
success: true success: true
exit_code: 0 exit_code: 0
----- stdout ----- ----- stdout -----
@ -3578,6 +3563,26 @@ fn already_installed_multiple_versions() -> Result<()> {
"### "###
); );
// Reset the test context
prepare(&context)?;
// Request the anyio without a version specifier
// This is loosely a regression test for the ordering of the installation preferences
// from existing site-packages
uv_snapshot!(context.filters(), context.install().arg("anyio"), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 3 packages in [TIME]
Installed 1 package in [TIME]
- anyio==3.7.0
- anyio==4.0.0
+ anyio==4.0.0
"###
);
Ok(()) Ok(())
} }