Skip to content

Commit fad9d93

Browse files
charliermarshcharliermarsh
committed
Avoid reusing cached downloaded binaries with --no-binary
1 parent ada6b36 commit fad9d93

4 files changed

Lines changed: 146 additions & 63 deletions

File tree

‎crates/uv-distribution/src/index/registry_wheel_index.rs‎

Lines changed: 69 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
use std::collections::hash_map::Entry;
2-
use std::collections::BTreeMap;
32

43
use rustc_hash::FxHashMap;
54

65
use distribution_types::{CachedRegistryDist, Hashed, IndexLocations, IndexUrl};
7-
use pep440_rs::Version;
86
use platform_tags::Tags;
97
use uv_cache::{Cache, CacheBucket, WheelCache};
108
use uv_fs::{directories, files, symlinks};
@@ -14,14 +12,23 @@ use uv_types::HashStrategy;
1412
use crate::index::cached_wheel::CachedWheel;
1513
use crate::source::{HttpRevisionPointer, LocalRevisionPointer, HTTP_REVISION, LOCAL_REVISION};
1614

15+
/// An entry in the [`RegistryWheelIndex`].
16+
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
17+
pub struct IndexEntry {
18+
/// The cached distribution.
19+
pub dist: CachedRegistryDist,
20+
/// Whether the wheel was built from source (true), or downloaded from the registry directly (false).
21+
pub built: bool,
22+
}
23+
1724
/// A local index of distributions that originate from a registry, like `PyPI`.
1825
#[derive(Debug)]
1926
pub struct RegistryWheelIndex<'a> {
2027
cache: &'a Cache,
2128
tags: &'a Tags,
2229
index_locations: &'a IndexLocations,
2330
hasher: &'a HashStrategy,
24-
index: FxHashMap<&'a PackageName, BTreeMap<Version, CachedRegistryDist>>,
31+
index: FxHashMap<&'a PackageName, Vec<IndexEntry>>,
2532
}
2633

2734
impl<'a> RegistryWheelIndex<'a> {
@@ -44,26 +51,12 @@ impl<'a> RegistryWheelIndex<'a> {
4451
/// Return an iterator over available wheels for a given package.
4552
///
4653
/// If the package is not yet indexed, this will index the package by reading from the cache.
47-
pub fn get(
48-
&mut self,
49-
name: &'a PackageName,
50-
) -> impl Iterator<Item = (&Version, &CachedRegistryDist)> {
54+
pub fn get(&mut self, name: &'a PackageName) -> impl Iterator<Item = &IndexEntry> {
5155
self.get_impl(name).iter().rev()
5256
}
5357

54-
/// Get the best wheel for the given package name and version.
55-
///
56-
/// If the package is not yet indexed, this will index the package by reading from the cache.
57-
pub fn get_version(
58-
&mut self,
59-
name: &'a PackageName,
60-
version: &Version,
61-
) -> Option<&CachedRegistryDist> {
62-
self.get_impl(name).get(version)
63-
}
64-
6558
/// Get an entry in the index.
66-
fn get_impl(&mut self, name: &'a PackageName) -> &BTreeMap<Version, CachedRegistryDist> {
59+
fn get_impl(&mut self, name: &'a PackageName) -> &[IndexEntry] {
6760
let versions = match self.index.entry(name) {
6861
Entry::Occupied(entry) => entry.into_mut(),
6962
Entry::Vacant(entry) => entry.insert(Self::index(
@@ -84,8 +77,8 @@ impl<'a> RegistryWheelIndex<'a> {
8477
tags: &Tags,
8578
index_locations: &IndexLocations,
8679
hasher: &HashStrategy,
87-
) -> BTreeMap<Version, CachedRegistryDist> {
88-
let mut versions = BTreeMap::new();
80+
) -> Vec<IndexEntry> {
81+
let mut entries = vec![];
8982

9083
// Collect into owned `IndexUrl`.
9184
let flat_index_urls: Vec<IndexUrl> = index_locations
@@ -113,12 +106,19 @@ impl<'a> RegistryWheelIndex<'a> {
113106
if let Some(wheel) =
114107
CachedWheel::from_http_pointer(wheel_dir.join(file), cache)
115108
{
116-
// Enforce hash-checking based on the built distribution.
117-
if wheel.satisfies(
118-
hasher
119-
.get_package(&wheel.filename.name, &wheel.filename.version),
120-
) {
121-
Self::add_wheel(wheel, tags, &mut versions);
109+
if wheel.filename.compatibility(tags).is_compatible() {
110+
// Enforce hash-checking based on the built distribution.
111+
if wheel.satisfies(
112+
hasher.get_package(
113+
&wheel.filename.name,
114+
&wheel.filename.version,
115+
),
116+
) {
117+
entries.push(IndexEntry {
118+
dist: wheel.into_registry_dist(),
119+
built: false,
120+
});
121+
}
122122
}
123123
}
124124
}
@@ -132,12 +132,19 @@ impl<'a> RegistryWheelIndex<'a> {
132132
if let Some(wheel) =
133133
CachedWheel::from_local_pointer(wheel_dir.join(file), cache)
134134
{
135-
// Enforce hash-checking based on the built distribution.
136-
if wheel.satisfies(
137-
hasher
138-
.get_package(&wheel.filename.name, &wheel.filename.version),
139-
) {
140-
Self::add_wheel(wheel, tags, &mut versions);
135+
if wheel.filename.compatibility(tags).is_compatible() {
136+
// Enforce hash-checking based on the built distribution.
137+
if wheel.satisfies(
138+
hasher.get_package(
139+
&wheel.filename.name,
140+
&wheel.filename.version,
141+
),
142+
) {
143+
entries.push(IndexEntry {
144+
dist: wheel.into_registry_dist(),
145+
built: false,
146+
});
147+
}
141148
}
142149
}
143150
}
@@ -182,38 +189,41 @@ impl<'a> RegistryWheelIndex<'a> {
182189
if let Some(revision) = revision {
183190
for wheel_dir in symlinks(cache_shard.join(revision.id())) {
184191
if let Some(wheel) = CachedWheel::from_built_source(wheel_dir) {
185-
// Enforce hash-checking based on the source distribution.
186-
if revision.satisfies(
187-
hasher.get_package(&wheel.filename.name, &wheel.filename.version),
188-
) {
189-
Self::add_wheel(wheel, tags, &mut versions);
192+
if wheel.filename.compatibility(tags).is_compatible() {
193+
// Enforce hash-checking based on the source distribution.
194+
if revision.satisfies(
195+
hasher
196+
.get_package(&wheel.filename.name, &wheel.filename.version),
197+
) {
198+
entries.push(IndexEntry {
199+
dist: wheel.into_registry_dist(),
200+
built: true,
201+
});
202+
}
190203
}
191204
}
192205
}
193206
}
194207
}
195208
}
196209

197-
versions
198-
}
199-
200-
/// Add the [`CachedWheel`] to the index.
201-
fn add_wheel(
202-
wheel: CachedWheel,
203-
tags: &Tags,
204-
versions: &mut BTreeMap<Version, CachedRegistryDist>,
205-
) {
206-
let dist_info = wheel.into_registry_dist();
207-
208-
// Pick the wheel with the highest priority
209-
let compatibility = dist_info.filename.compatibility(tags);
210-
if let Some(existing) = versions.get_mut(&dist_info.filename.version) {
211-
// Override if we have better compatibility
212-
if compatibility > existing.filename.compatibility(tags) {
213-
*existing = dist_info;
214-
}
215-
} else if compatibility.is_compatible() {
216-
versions.insert(dist_info.filename.version.clone(), dist_info);
217-
}
210+
// Sort the cached distributions by (1) version, (2) compatibility, and (3) build status.
211+
// We want the highest versions, with the greatest compatibility, that were built from source.
212+
// at the end of the list.
213+
entries.sort_unstable_by(|a, b| {
214+
a.dist
215+
.filename
216+
.version
217+
.cmp(&b.dist.filename.version)
218+
.then_with(|| {
219+
a.dist
220+
.filename
221+
.compatibility(tags)
222+
.cmp(&b.dist.filename.compatibility(tags))
223+
.then_with(|| a.built.cmp(&b.built))
224+
})
225+
});
226+
227+
entries
218228
}
219229
}

‎crates/uv-installer/src/plan.rs‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ impl<'a> Planner<'a> {
104104

105105
// Check if installation of a binary version of the package should be allowed.
106106
let no_binary = build_options.no_binary_package(&requirement.name);
107+
let no_build = build_options.no_build_package(&requirement.name);
107108

108109
let installed_dists = site_packages.remove_packages(&requirement.name);
109110

@@ -143,9 +144,19 @@ impl<'a> Planner<'a> {
143144
// Identify any cached distributions that satisfy the requirement.
144145
match &requirement.source {
145146
RequirementSource::Registry { specifier, .. } => {
146-
if let Some((_version, distribution)) = registry_index
147-
.get(&requirement.name)
148-
.find(|(version, _)| specifier.contains(version))
147+
if let Some(distribution) =
148+
registry_index.get(&requirement.name).find_map(|entry| {
149+
if !specifier.contains(&entry.dist.filename.version) {
150+
return None;
151+
};
152+
if entry.built && no_build {
153+
return None;
154+
}
155+
if !entry.built && no_binary {
156+
return None;
157+
}
158+
Some(&entry.dist)
159+
})
149160
{
150161
debug!("Requirement already cached: {distribution}");
151162
cached.push(CachedDist::Registry(distribution.clone()));

‎crates/uv/tests/lock.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7473,7 +7473,7 @@ fn lock_find_links_http_sdist() -> Result<()> {
74737473
----- stdout -----
74747474

74757475
----- stderr -----
7476-
Prepared 1 package in [TIME]
7476+
Prepared 2 packages in [TIME]
74777477
Installed 2 packages in [TIME]
74787478
+ packaging==23.2
74797479
+ project==0.1.0 (from file://[TEMP_DIR]/)

‎crates/uv/tests/pip_install.rs‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2010,6 +2010,68 @@ fn install_only_binary_all_and_no_binary_all() {
20102010
context.assert_command("import anyio").failure();
20112011
}
20122012

2013+
/// Binary dependencies in the cache should be reused when the user provides `--no-build`.
2014+
#[test]
2015+
fn install_no_binary_cache() -> Result<()> {
2016+
let context = TestContext::new("3.12");
2017+
2018+
// Install a binary distribution.
2019+
uv_snapshot!(
2020+
context.pip_install().arg("idna"),
2021+
@r###"
2022+
success: true
2023+
exit_code: 0
2024+
----- stdout -----
2025+
2026+
----- stderr -----
2027+
Resolved 1 package in [TIME]
2028+
Prepared 1 package in [TIME]
2029+
Installed 1 package in [TIME]
2030+
+ idna==3.6
2031+
"###
2032+
);
2033+
2034+
// Re-create the virtual environment.
2035+
context.venv().assert().success();
2036+
2037+
// Re-install. The distribution should be installed from the cache.
2038+
uv_snapshot!(
2039+
context.pip_install().arg("idna"),
2040+
@r###"
2041+
success: true
2042+
exit_code: 0
2043+
----- stdout -----
2044+
2045+
----- stderr -----
2046+
Resolved 1 package in [TIME]
2047+
Installed 1 package in [TIME]
2048+
+ idna==3.6
2049+
"###
2050+
);
2051+
2052+
// Re-create the virtual environment.
2053+
context.venv().assert().success();
2054+
2055+
// Install with `--no-binary`. The distribution should be built from source, despite a binary
2056+
// distribution being available in the cache.
2057+
uv_snapshot!(
2058+
context.pip_install().arg("idna").arg("--no-binary").arg(":all:"),
2059+
@r###"
2060+
success: true
2061+
exit_code: 0
2062+
----- stdout -----
2063+
2064+
----- stderr -----
2065+
Resolved 1 package in [TIME]
2066+
Prepared 1 package in [TIME]
2067+
Installed 1 package in [TIME]
2068+
+ idna==3.6
2069+
"###
2070+
);
2071+
2072+
Ok(())
2073+
}
2074+
20132075
/// Respect `--only-binary` flags in `requirements.txt`
20142076
#[test]
20152077
fn only_binary_requirements_txt() {

0 commit comments

Comments
 (0)