Respect --no-build and --no-binary in --find-links (#2826)

## Summary

In working on `--require-hashes`, I noticed that we're missing some
incompatibility tracking for `--find-links` distributions. Specifically,
we don't respect `--no-build` or `--no-binary`, so if we select a wheel
due to `--find-links`, we then throw a hard error when trying to build
it later (if `--no-binary` is provided), rather than selecting the
source distribution instead.

Closes https://github.com/astral-sh/uv/issues/2827.
This commit is contained in:
Charlie Marsh 2024-04-04 22:00:39 -04:00 committed by GitHub
parent 365cb16fd6
commit 2ac562b40d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 150 additions and 43 deletions

View file

@ -8,10 +8,11 @@ use rustc_hash::FxHashMap;
use tracing::{debug, info_span, instrument, warn, Instrument}; use tracing::{debug, info_span, instrument, warn, Instrument};
use url::Url; use url::Url;
use distribution_filename::DistFilename; use distribution_filename::{DistFilename, SourceDistFilename, WheelFilename};
use distribution_types::{ use distribution_types::{
BuiltDist, Dist, File, FileLocation, FlatIndexLocation, IndexUrl, PrioritizedDist, BuiltDist, Dist, File, FileLocation, FlatIndexLocation, IncompatibleSource, IncompatibleWheel,
RegistryBuiltDist, RegistrySourceDist, SourceDist, SourceDistCompatibility, IndexUrl, PrioritizedDist, RegistryBuiltDist, RegistrySourceDist, SourceDist,
SourceDistCompatibility, WheelCompatibility,
}; };
use pep440_rs::Version; use pep440_rs::Version;
use pep508_rs::VerbatimUrl; use pep508_rs::VerbatimUrl;
@ -19,6 +20,7 @@ use platform_tags::Tags;
use pypi_types::Hashes; use pypi_types::Hashes;
use uv_cache::{Cache, CacheBucket}; use uv_cache::{Cache, CacheBucket};
use uv_normalize::PackageName; use uv_normalize::PackageName;
use uv_types::{NoBinary, NoBuild};
use crate::cached_client::{CacheControl, CachedClientError}; use crate::cached_client::{CacheControl, CachedClientError};
use crate::html::SimpleHtml; use crate::html::SimpleHtml;
@ -271,12 +273,25 @@ pub struct FlatIndex {
impl FlatIndex { impl FlatIndex {
/// Collect all files from a `--find-links` target into a [`FlatIndex`]. /// Collect all files from a `--find-links` target into a [`FlatIndex`].
#[instrument(skip_all)] #[instrument(skip_all)]
pub fn from_entries(entries: FlatIndexEntries, tags: &Tags) -> Self { pub fn from_entries(
entries: FlatIndexEntries,
tags: &Tags,
no_build: &NoBuild,
no_binary: &NoBinary,
) -> Self {
// Collect compatible distributions. // Collect compatible distributions.
let mut index = FxHashMap::default(); let mut index = FxHashMap::default();
for (filename, file, url) in entries.entries { for (filename, file, url) in entries.entries {
let distributions = index.entry(filename.name().clone()).or_default(); let distributions = index.entry(filename.name().clone()).or_default();
Self::add_file(distributions, file, filename, tags, url); Self::add_file(
distributions,
file,
filename,
tags,
no_build,
no_binary,
url,
);
} }
// Collect offline entries. // Collect offline entries.
@ -290,15 +305,17 @@ impl FlatIndex {
file: File, file: File,
filename: DistFilename, filename: DistFilename,
tags: &Tags, tags: &Tags,
no_build: &NoBuild,
no_binary: &NoBinary,
index: IndexUrl, index: IndexUrl,
) { ) {
// No `requires-python` here: for source distributions, we don't have that information; // No `requires-python` here: for source distributions, we don't have that information;
// for wheels, we read it lazily only when selected. // for wheels, we read it lazily only when selected.
match filename { match filename {
DistFilename::WheelFilename(filename) => { DistFilename::WheelFilename(filename) => {
let compatibility = filename.compatibility(tags);
let version = filename.version.clone(); let version = filename.version.clone();
let compatibility = Self::wheel_compatibility(&filename, tags, no_binary);
let dist = Dist::Built(BuiltDist::Registry(RegistryBuiltDist { let dist = Dist::Built(BuiltDist::Registry(RegistryBuiltDist {
filename, filename,
file: Box::new(file), file: Box::new(file),
@ -306,20 +323,15 @@ impl FlatIndex {
})); }));
match distributions.0.entry(version) { match distributions.0.entry(version) {
Entry::Occupied(mut entry) => { Entry::Occupied(mut entry) => {
entry entry.get_mut().insert_built(dist, None, compatibility);
.get_mut()
.insert_built(dist, None, compatibility.into());
} }
Entry::Vacant(entry) => { Entry::Vacant(entry) => {
entry.insert(PrioritizedDist::from_built( entry.insert(PrioritizedDist::from_built(dist, None, compatibility));
dist,
None,
compatibility.into(),
));
} }
} }
} }
DistFilename::SourceDistFilename(filename) => { DistFilename::SourceDistFilename(filename) => {
let compatibility = Self::source_dist_compatibility(&filename, no_build);
let dist = Dist::Source(SourceDist::Registry(RegistrySourceDist { let dist = Dist::Source(SourceDist::Registry(RegistrySourceDist {
filename: filename.clone(), filename: filename.clone(),
file: Box::new(file), file: Box::new(file),
@ -327,24 +339,54 @@ impl FlatIndex {
})); }));
match distributions.0.entry(filename.version) { match distributions.0.entry(filename.version) {
Entry::Occupied(mut entry) => { Entry::Occupied(mut entry) => {
entry.get_mut().insert_source( entry.get_mut().insert_source(dist, None, compatibility);
dist,
None,
SourceDistCompatibility::Compatible,
);
} }
Entry::Vacant(entry) => { Entry::Vacant(entry) => {
entry.insert(PrioritizedDist::from_source( entry.insert(PrioritizedDist::from_source(dist, None, compatibility));
dist,
None,
SourceDistCompatibility::Compatible,
));
} }
} }
} }
} }
} }
fn source_dist_compatibility(
filename: &SourceDistFilename,
no_build: &NoBuild,
) -> SourceDistCompatibility {
// Check if source distributions are allowed for this package.
let no_build = match no_build {
NoBuild::None => false,
NoBuild::All => true,
NoBuild::Packages(packages) => packages.contains(&filename.name),
};
if no_build {
return SourceDistCompatibility::Incompatible(IncompatibleSource::NoBuild);
}
SourceDistCompatibility::Compatible
}
fn wheel_compatibility(
filename: &WheelFilename,
tags: &Tags,
no_binary: &NoBinary,
) -> WheelCompatibility {
// Check if binaries are allowed for this package.
let no_binary = match no_binary {
NoBinary::None => false,
NoBinary::All => true,
NoBinary::Packages(packages) => packages.contains(&filename.name),
};
if no_binary {
return WheelCompatibility::Incompatible(IncompatibleWheel::NoBinary);
}
// Determine a compatibility for the wheel based on tags.
WheelCompatibility::from(filename.compatibility(tags))
}
/// Get the [`FlatDistributions`] for the given package name. /// Get the [`FlatDistributions`] for the given package name.
pub fn get(&self, package_name: &PackageName) -> Option<&FlatDistributions> { pub fn get(&self, package_name: &PackageName) -> Option<&FlatDistributions> {
self.index.get(package_name) self.index.get(package_name)

View file

@ -56,14 +56,6 @@ pub(crate) async fn resolve_cli(args: ResolveCliArgs) -> Result<()> {
let venv = PythonEnvironment::from_virtualenv(&cache)?; let venv = PythonEnvironment::from_virtualenv(&cache)?;
let index_locations = let index_locations =
IndexLocations::new(args.index_url, args.extra_index_url, args.find_links, false); IndexLocations::new(args.index_url, args.extra_index_url, args.find_links, false);
let client = RegistryClientBuilder::new(cache.clone())
.index_urls(index_locations.index_urls())
.build();
let flat_index = {
let client = FlatIndexClient::new(&client, &cache);
let entries = client.fetch(index_locations.flat_index()).await?;
FlatIndex::from_entries(entries, venv.interpreter().tags()?)
};
let index = InMemoryIndex::default(); let index = InMemoryIndex::default();
let in_flight = InFlight::default(); let in_flight = InFlight::default();
let no_build = if args.no_build { let no_build = if args.no_build {
@ -71,6 +63,19 @@ pub(crate) async fn resolve_cli(args: ResolveCliArgs) -> Result<()> {
} else { } else {
NoBuild::None NoBuild::None
}; };
let client = RegistryClientBuilder::new(cache.clone())
.index_urls(index_locations.index_urls())
.build();
let flat_index = {
let client = FlatIndexClient::new(&client, &cache);
let entries = client.fetch(index_locations.flat_index()).await?;
FlatIndex::from_entries(
entries,
venv.interpreter().tags()?,
&no_build,
&NoBinary::None,
)
};
let config_settings = ConfigSettings::default(); let config_settings = ConfigSettings::default();
let build_dispatch = BuildDispatch::new( let build_dispatch = BuildDispatch::new(

View file

@ -228,7 +228,7 @@ pub(crate) async fn pip_compile(
let flat_index = { let flat_index = {
let client = FlatIndexClient::new(&client, &cache); let client = FlatIndexClient::new(&client, &cache);
let entries = client.fetch(index_locations.flat_index()).await?; let entries = client.fetch(index_locations.flat_index()).await?;
FlatIndex::from_entries(entries, &tags) FlatIndex::from_entries(entries, &tags, &no_build, &NoBinary::None)
}; };
// Track in-flight downloads, builds, etc., across resolutions. // Track in-flight downloads, builds, etc., across resolutions.

View file

@ -206,7 +206,7 @@ pub(crate) async fn pip_install(
let flat_index = { let flat_index = {
let client = FlatIndexClient::new(&client, &cache); let client = FlatIndexClient::new(&client, &cache);
let entries = client.fetch(index_locations.flat_index()).await?; let entries = client.fetch(index_locations.flat_index()).await?;
FlatIndex::from_entries(entries, tags) FlatIndex::from_entries(entries, tags, &no_build, &no_binary)
}; };
// Determine whether to enable build isolation. // Determine whether to enable build isolation.

View file

@ -155,7 +155,7 @@ pub(crate) async fn pip_sync(
let flat_index = { let flat_index = {
let client = FlatIndexClient::new(&client, &cache); let client = FlatIndexClient::new(&client, &cache);
let entries = client.fetch(index_locations.flat_index()).await?; let entries = client.fetch(index_locations.flat_index()).await?;
FlatIndex::from_entries(entries, tags) FlatIndex::from_entries(entries, tags, &no_build, &no_binary)
}; };
// Create a shared in-memory index. // Create a shared in-memory index.

View file

@ -169,7 +169,7 @@ async fn venv_impl(
.fetch(index_locations.flat_index()) .fetch(index_locations.flat_index())
.await .await
.map_err(VenvError::FlatIndex)?; .map_err(VenvError::FlatIndex)?;
FlatIndex::from_entries(entries, tags) FlatIndex::from_entries(entries, tags, &NoBuild::All, &NoBinary::None)
}; };
// Create a shared in-memory index. // Create a shared in-memory index.

View file

@ -3600,7 +3600,7 @@ fn find_links_directory() -> Result<()> {
uv_snapshot!(context.filters(), context.compile() uv_snapshot!(context.filters(), context.compile()
.arg("requirements.in") .arg("requirements.in")
.arg("--find-links") .arg("--find-links")
.arg(context.workspace_root.join("scripts").join("wheels")), @r###" .arg(context.workspace_root.join("scripts").join("links")), @r###"
success: true success: true
exit_code: 0 exit_code: 0
----- stdout ----- ----- stdout -----

View file

@ -1854,7 +1854,7 @@ fn launcher() -> Result<()> {
uv_snapshot!( uv_snapshot!(
filters, filters,
context.install() context.install()
.arg(format!("simple_launcher@{}", project_root.join("scripts/wheels/simple_launcher-0.1.0-py3-none-any.whl").display())) .arg(format!("simple_launcher@{}", project_root.join("scripts/links/simple_launcher-0.1.0-py3-none-any.whl").display()))
.arg("--strict"), @r###" .arg("--strict"), @r###"
success: true success: true
exit_code: 0 exit_code: 0
@ -1899,7 +1899,7 @@ fn launcher_with_symlink() -> Result<()> {
uv_snapshot!(filters, uv_snapshot!(filters,
context.install() context.install()
.arg(format!("simple_launcher@{}", project_root.join("scripts/wheels/simple_launcher-0.1.0-py3-none-any.whl").display())) .arg(format!("simple_launcher@{}", project_root.join("scripts/links/simple_launcher-0.1.0-py3-none-any.whl").display()))
.arg("--strict"), .arg("--strict"),
@r###" @r###"
success: true success: true
@ -3739,3 +3739,63 @@ fn already_installed_remote_url() {
<uri>`) <uri>`)
"###); "###);
} }
/// Sync using `--find-links` with a local directory.
#[test]
fn find_links() -> Result<()> {
let context = TestContext::new("3.12");
let requirements_txt = context.temp_dir.child("requirements.txt");
requirements_txt.write_str(indoc! {r"
tqdm
"})?;
uv_snapshot!(context.filters(), context.install()
.arg("tqdm")
.arg("--find-links")
.arg(context.workspace_root.join("scripts/links/")), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 1 package in [TIME]
Downloaded 1 package in [TIME]
Installed 1 package in [TIME]
+ tqdm==1000.0.0
"###
);
Ok(())
}
/// Sync using `--find-links` with a local directory, with wheels disabled.
#[test]
fn find_links_no_binary() -> Result<()> {
let context = TestContext::new("3.12");
let requirements_txt = context.temp_dir.child("requirements.txt");
requirements_txt.write_str(indoc! {r"
tqdm
"})?;
uv_snapshot!(context.filters(), context.install()
.arg("tqdm")
.arg("--no-binary")
.arg(":all:")
.arg("--find-links")
.arg(context.workspace_root.join("scripts/links/")), @r###"
success: true
exit_code: 0
----- stdout -----
----- stderr -----
Resolved 1 package in [TIME]
Downloaded 1 package in [TIME]
Installed 1 package in [TIME]
+ tqdm==999.0.0
"###
);
Ok(())
}

View file

@ -2461,7 +2461,7 @@ fn find_links() -> Result<()> {
uv_snapshot!(context.filters(), command(&context) uv_snapshot!(context.filters(), command(&context)
.arg("requirements.txt") .arg("requirements.txt")
.arg("--find-links") .arg("--find-links")
.arg(context.workspace_root.join("scripts/wheels/")), @r###" .arg(context.workspace_root.join("scripts/links/")), @r###"
success: true success: true
exit_code: 0 exit_code: 0
----- stdout ----- ----- stdout -----
@ -2494,7 +2494,7 @@ fn find_links_no_index_match() -> Result<()> {
.arg("requirements.txt") .arg("requirements.txt")
.arg("--no-index") .arg("--no-index")
.arg("--find-links") .arg("--find-links")
.arg(context.workspace_root.join("scripts/wheels/")), @r###" .arg(context.workspace_root.join("scripts/links/")), @r###"
success: true success: true
exit_code: 0 exit_code: 0
----- stdout ----- ----- stdout -----
@ -2524,7 +2524,7 @@ fn find_links_offline_match() -> Result<()> {
.arg("requirements.txt") .arg("requirements.txt")
.arg("--offline") .arg("--offline")
.arg("--find-links") .arg("--find-links")
.arg(context.workspace_root.join("scripts/wheels/")), @r###" .arg(context.workspace_root.join("scripts/links/")), @r###"
success: true success: true
exit_code: 0 exit_code: 0
----- stdout ----- ----- stdout -----
@ -2555,7 +2555,7 @@ fn find_links_offline_no_match() -> Result<()> {
.arg("requirements.txt") .arg("requirements.txt")
.arg("--offline") .arg("--offline")
.arg("--find-links") .arg("--find-links")
.arg(context.workspace_root.join("scripts/wheels/")), @r###" .arg(context.workspace_root.join("scripts/links/")), @r###"
success: false success: false
exit_code: 2 exit_code: 2
----- stdout ----- ----- stdout -----

Binary file not shown.