From 3aaa9594be4727fb4a6260b1cc5782eb66e47284 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 6 Dec 2024 09:32:19 -0500 Subject: [PATCH] Add test coverage for build tag prioritization (#9680) ## Summary See: https://github.com/astral-sh/uv/pull/3781 and https://github.com/astral-sh/uv/pull/9677. --- crates/uv-client/src/flat_index.rs | 29 +++- crates/uv-distribution-filename/src/lib.rs | 2 +- .../src/source_dist.rs | 2 + crates/uv-distribution-filename/src/wheel.rs | 13 +- crates/uv-resolver/src/flat_index.rs | 10 +- crates/uv/tests/it/pip_install.rs | 40 +++++- crates/uv/tests/it/sync.rs | 127 +++++++++++++++++- .../build_tag-1.0.0-1-py2.py3-none-any.whl | Bin 0 -> 932 bytes .../build_tag-1.0.0-3-py2.py3-none-any.whl | Bin 0 -> 932 bytes .../build_tag-1.0.0-5-py2.py3-none-any.whl | Bin 0 -> 932 bytes 10 files changed, 205 insertions(+), 18 deletions(-) create mode 100644 scripts/links/build_tag-1.0.0-1-py2.py3-none-any.whl create mode 100644 scripts/links/build_tag-1.0.0-3-py2.py3-none-any.whl create mode 100644 scripts/links/build_tag-1.0.0-5-py2.py3-none-any.whl diff --git a/crates/uv-client/src/flat_index.rs b/crates/uv-client/src/flat_index.rs index 50620fe09..b0d41c930 100644 --- a/crates/uv-client/src/flat_index.rs +++ b/crates/uv-client/src/flat_index.rs @@ -34,10 +34,18 @@ pub enum FindLinksDirectoryError { VerbatimUrl(#[from] uv_pep508::VerbatimUrlError), } +/// An entry in a `--find-links` index. +#[derive(Debug, Clone)] +pub struct FlatIndexEntry { + pub filename: DistFilename, + pub file: File, + pub index: IndexUrl, +} + #[derive(Debug, Default, Clone)] pub struct FlatIndexEntries { /// The list of `--find-links` entries. - pub entries: Vec<(DistFilename, File, IndexUrl)>, + pub entries: Vec, /// Whether any `--find-links` entries could not be resolved due to a lack of network /// connectivity. pub offline: bool, @@ -45,7 +53,7 @@ pub struct FlatIndexEntries { impl FlatIndexEntries { /// Create a [`FlatIndexEntries`] from a list of `--find-links` entries. - fn from_entries(entries: Vec<(DistFilename, File, IndexUrl)>) -> Self { + fn from_entries(entries: Vec) -> Self { Self { entries, offline: false, @@ -130,6 +138,9 @@ impl<'a> FlatIndexClient<'a> { while let Some(entries) = fetches.next().await.transpose()? { results.extend(entries); } + results + .entries + .sort_by(|a, b| a.filename.cmp(&b.filename).then(a.index.cmp(&b.index))); Ok(results) } @@ -211,11 +222,11 @@ impl<'a> FlatIndexClient<'a> { .expect("archived version always deserializes") }) .filter_map(|file| { - Some(( - DistFilename::try_from_normalized_filename(&file.filename)?, + Some(FlatIndexEntry { + filename: DistFilename::try_from_normalized_filename(&file.filename)?, file, - flat_index.clone(), - )) + index: flat_index.clone(), + }) }) .collect(); Ok(FlatIndexEntries::from_entries(files)) @@ -283,7 +294,11 @@ impl<'a> FlatIndexClient<'a> { ); continue; }; - dists.push((filename, file, flat_index.clone())); + dists.push(FlatIndexEntry { + filename, + file, + index: flat_index.clone(), + }); } Ok(FlatIndexEntries::from_entries(dists)) } diff --git a/crates/uv-distribution-filename/src/lib.rs b/crates/uv-distribution-filename/src/lib.rs index 5edd1454a..f16d2c7c3 100644 --- a/crates/uv-distribution-filename/src/lib.rs +++ b/crates/uv-distribution-filename/src/lib.rs @@ -15,7 +15,7 @@ mod extension; mod source_dist; mod wheel; -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)] pub enum DistFilename { SourceDistFilename(SourceDistFilename), WheelFilename(WheelFilename), diff --git a/crates/uv-distribution-filename/src/source_dist.rs b/crates/uv-distribution-filename/src/source_dist.rs index a3920a32d..2dfacc090 100644 --- a/crates/uv-distribution-filename/src/source_dist.rs +++ b/crates/uv-distribution-filename/src/source_dist.rs @@ -14,6 +14,8 @@ use uv_pep440::{Version, VersionParseError}; Debug, PartialEq, Eq, + PartialOrd, + Ord, Serialize, Deserialize, rkyv::Archive, diff --git a/crates/uv-distribution-filename/src/wheel.rs b/crates/uv-distribution-filename/src/wheel.rs index a91c3e141..0d4762135 100644 --- a/crates/uv-distribution-filename/src/wheel.rs +++ b/crates/uv-distribution-filename/src/wheel.rs @@ -11,7 +11,18 @@ use uv_platform_tags::{TagCompatibility, Tags}; use crate::{BuildTag, BuildTagError}; -#[derive(Debug, Clone, Eq, PartialEq, Hash, rkyv::Archive, rkyv::Deserialize, rkyv::Serialize)] +#[derive( + Debug, + Clone, + Eq, + PartialEq, + Ord, + PartialOrd, + Hash, + rkyv::Archive, + rkyv::Deserialize, + rkyv::Serialize, +)] #[rkyv(derive(Debug))] pub struct WheelFilename { pub name: PackageName, diff --git a/crates/uv-resolver/src/flat_index.rs b/crates/uv-resolver/src/flat_index.rs index 8c47f4322..a7570f250 100644 --- a/crates/uv-resolver/src/flat_index.rs +++ b/crates/uv-resolver/src/flat_index.rs @@ -40,16 +40,16 @@ impl FlatIndex { ) -> Self { // Collect compatible distributions. let mut index = FxHashMap::default(); - for (filename, file, url) in entries.entries { - let distributions = index.entry(filename.name().clone()).or_default(); + for entry in entries.entries { + let distributions = index.entry(entry.filename.name().clone()).or_default(); Self::add_file( distributions, - file, - filename, + entry.file, + entry.filename, tags, hasher, build_options, - url, + entry.index, ); } diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index 7f7104ea4..416b80eb5 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -12,7 +12,8 @@ use predicates::prelude::predicate; use url::Url; use crate::common::{ - self, build_vendor_links_url, decode_token, get_bin, uv_snapshot, venv_bin_path, TestContext, + self, build_vendor_links_url, decode_token, get_bin, uv_snapshot, venv_bin_path, + venv_to_interpreter, TestContext, }; use uv_fs::Simplified; use uv_static::EnvVars; @@ -7516,3 +7517,40 @@ fn test_dynamic_version_sdist_wrong_version() -> Result<()> { Ok(()) } + +/// Install a package with multiple wheels at the same version, differing only in the build tag. We +/// should choose the wheel with the highest build tag. +#[test] +fn build_tag() { + let context = TestContext::new("3.12"); + + uv_snapshot!(context.filters(), context.pip_install() + .arg("build-tag") + .arg("--find-links") + .arg(context.workspace_root.join("scripts/links/")), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + Installed 1 package in [TIME] + + build-tag==1.0.0 + "### + ); + + // Ensure that we choose the highest build tag (5). + uv_snapshot!(Command::new(venv_to_interpreter(&context.venv)) + .arg("-B") + .arg("-c") + .arg("import build_tag; build_tag.main()") + .current_dir(&context.temp_dir), @r###" + success: true + exit_code: 0 + ----- stdout ----- + 5 + + ----- stderr ----- + "###); +} diff --git a/crates/uv/tests/it/sync.rs b/crates/uv/tests/it/sync.rs index 7f419b83e..1f9fd3abb 100644 --- a/crates/uv/tests/it/sync.rs +++ b/crates/uv/tests/it/sync.rs @@ -1,13 +1,13 @@ use anyhow::Result; use assert_cmd::prelude::*; use assert_fs::{fixture::ChildPath, prelude::*}; -use indoc::indoc; +use indoc::{formatdoc, indoc}; use insta::assert_snapshot; +use crate::common::{download_to_disk, uv_snapshot, venv_bin_path, TestContext}; use predicates::prelude::predicate; use tempfile::tempdir_in; - -use crate::common::{download_to_disk, uv_snapshot, venv_bin_path, TestContext}; +use uv_fs::Simplified; use uv_static::EnvVars; #[test] @@ -5589,3 +5589,124 @@ fn sync_git_path_dependency() -> Result<()> { Ok(()) } + +/// Sync a package with multiple wheels at the same version, differing only in the build tag. We +/// should choose the wheel with the highest build tag. +#[test] +fn sync_build_tag() -> Result<()> { + let context = TestContext::new("3.12"); + + // Populate the `--find-links` entries. + fs_err::create_dir_all(context.temp_dir.join("links"))?; + + for entry in fs_err::read_dir(context.workspace_root.join("scripts/links"))? { + let entry = entry?; + let path = entry.path(); + if path + .file_name() + .and_then(|file_name| file_name.to_str()) + .is_some_and(|file_name| file_name.starts_with("build_tag-")) + { + let dest = context + .temp_dir + .join("links") + .join(path.file_name().unwrap()); + fs_err::copy(&path, &dest)?; + } + } + + context + .temp_dir + .child("pyproject.toml") + .write_str(&formatdoc! { r#" + [project] + name = "project" + version = "0.1.0" + requires-python = ">=3.12" + dependencies = ["build-tag"] + + [tool.uv] + find-links = ["{}"] + "#, + context.temp_dir.join("links/").portable_display(), + })?; + + uv_snapshot!(context.filters(), context.lock(), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + "###); + + let lock = fs_err::read_to_string(context.temp_dir.child("uv.lock")).unwrap(); + + insta::with_settings!({ + filters => context.filters(), + }, { + assert_snapshot!( + lock, @r###" + version = 1 + requires-python = ">=3.12" + + [options] + exclude-newer = "2024-03-25T00:00:00Z" + + [[package]] + name = "build-tag" + version = "1.0.0" + source = { registry = "links" } + wheels = [ + { path = "build_tag-1.0.0-1-py2.py3-none-any.whl" }, + { path = "build_tag-1.0.0-3-py2.py3-none-any.whl" }, + { path = "build_tag-1.0.0-5-py2.py3-none-any.whl" }, + ] + + [[package]] + name = "project" + version = "0.1.0" + source = { virtual = "." } + dependencies = [ + { name = "build-tag" }, + ] + + [package.metadata] + requires-dist = [{ name = "build-tag" }] + "### + ); + }); + + // Re-run with `--locked`. + uv_snapshot!(context.filters(), context.lock().arg("--locked"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + "###); + + // Install from the lockfile. + uv_snapshot!(context.filters(), context.sync().arg("--frozen"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Installed 1 package in [TIME] + + build-tag==1.0.0 + "###); + + // Ensure that we choose the highest build tag (5). + uv_snapshot!(context.filters(), context.run().arg("--no-sync").arg("python").arg("-c").arg("import build_tag; build_tag.main()"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + 5 + + ----- stderr ----- + "###); + + Ok(()) +} diff --git a/scripts/links/build_tag-1.0.0-1-py2.py3-none-any.whl b/scripts/links/build_tag-1.0.0-1-py2.py3-none-any.whl new file mode 100644 index 0000000000000000000000000000000000000000..2a0bc4ae19ee8e8660906d5f0cf23242d3c7a69a GIT binary patch literal 932 zcmWIWW@Zs#U|`??VyA!&7kbj9fh-Uf1>&UA%$$_?lEifV`1s7c%#!$cy@E<_Pv77Z zzGu!{zOdy{5JOYbqBDDhcvYQMef1auP)uX0dHBu%Xo@KiDix@lL^G~c0#Y*WORK1)%H)SMzRby-oX%Ccxpkr1hu&z?SC z`bzCt=pmOLK_xVIoqRw2z!{L67#SE8@VYC=)!9GDrS{D3vL;}_UAUXydE)Vdp zH@YNz4qnn^?@|mrz{TkIqbfGSwtRo2`l&gmwnPMIWhrW2{U@4HnRV-Q|A!TKy004j zpBVmXy~puMEaji48^^qlv0*!VZJ(S((R}|FzRRft2jB` zrvAEt+f~o-l-u8DcVA4a-WSup$@;E{%Yh4T&2=65)btoOs|R>9GKnzbPGP{{0D~os zAPQ@GLpK6FSwRe7U|7QFhinASM22o2da^;7w-2v*;6#LO7H*a!FoAdg`yV0J literal 0 HcmV?d00001 diff --git a/scripts/links/build_tag-1.0.0-3-py2.py3-none-any.whl b/scripts/links/build_tag-1.0.0-3-py2.py3-none-any.whl new file mode 100644 index 0000000000000000000000000000000000000000..18ed315fc36d0da5a390daa47f5f8564e01b8ccf GIT binary patch literal 932 zcmWIWW@Zs#U|`??Vy6IWiPkgHKo$s#0&!AlW==|cNn*Nwe0*kJW=VX!UO}a|r*H5H z-!o?}U)b^}h@q)z(V0C$ysF8nzIqG+D5f#hJbY&WG{qE%m5@!-HPkZz!j#P765Y(a zw0wPE*APb+#}LQQzPg@1o;sdac)fMC&Yd~GImqCW@v~3fXMDE?d1#$I<9QJ%|Gddt z>XqaR$pS_+`_wjhL`MVd$pm6~y!M5AxVrjqpZD-Rdl6yN72^j*{%5^)y>w3MZwfl$ zq2qb(#K|+>>|PQxUrH9UK7Bs@MU~iVwkcvupQWfpYEBWEx~!;GWm&YQNQl(SXHTCm zeWms+^pH!Bpc0z9y7jb~&Vby+$iSe0*Ihxb&i+9zwP!XKH30+e!rgpMt_lZPt0MwY z3s<-}%q@B%;OThDOX1l1eDSdNZ~J-o=&saR$L&>lTy^PZdEQ^UHpqV88+c-;QsIYK z#gKxy{JkG%n)6$moaPmND7b0U;mQdTR&3kMlP{-66n~CCS)KV|olv*L@(b;na~`d@ zs-?DC?CPDqcab6CPnPTpOy~_|+uJYw|L(H$)~CJ)F{b}iva3t_W4iWep68*&2M05i zKRdG<*LeH*UVFS_SA;Eh_V--<_9Yhum`|6)ZDCm~`;l?RU&a7$MkWzv+$jtg9AL1d z5kz54Z|Fv#Co6~n3=B&c{g92onaI%1Lr*pc^Y-C251fe54MUG>gkfR8*oXTM!*P(9 U5AbGX1L!(C5h?!@$s2?nI-Y@dIgo{Hw15giS*CliR}@!A*e;p*zcecr?O>_vo4SBxJN`JeUH_0l=1zbWX1 zhmPmD6DQAjvwKO*d?{JX`tawC%m1WVIA|X;QpFMrP z^p)DP&_gagf=X!as`ym+`wYlUj0_A4c-g*rnQhR1&QImlL!-Ko&on8Vi?R=RX z+mx6zS3IA@-9BZ1Qike*rT+Kz*4*1(U&&soxaH3rZbq&x0-N)jZ+YE%_vz?IuXpc# z3p)9^*5^%kiSJ!>!-IX6hL+TZ$H$7DP9AB<$)D7JUF7_lde$vfT&r@PDSCJM2N@l6 z%x#;nK0B&W)@IWJt)G=Y7D?ZkF8Og=`47t_`}wPSzBb%BI{C+!V>jwzQnx(Vy{`G} z&sceTk&`!UH}cfK{ry#R`_FYz>yqxOnmbDzn)Z9i(;l}w?iruh1H2iTM3`}>Fko