From c0786832170ec0eac636d19a40a2b1ad7d82f389 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 1 Jul 2025 12:50:19 -0400 Subject: [PATCH] Only drop build directories on program exit (#14304) ## Summary This PR ensures that we avoid cleaning up build directories until the end of a resolve-and-install cycle. It's not bulletproof (since we could still run into issues with `uv lock` followed by `uv sync` whereby a build directory gets cleaned up that's still referenced in the `build` artifacts), but it at least gets PyTorch building without error with `uv pip install .`, which is a case that's been reported several times. Closes https://github.com/astral-sh/uv/issues/14269. --- Cargo.lock | 2 ++ crates/uv-build-frontend/src/lib.rs | 4 ++++ crates/uv-dispatch/src/lib.rs | 22 +++++++++++++++++----- crates/uv-distribution/src/source/mod.rs | 24 +++++++++++++++++++----- crates/uv-types/Cargo.toml | 2 ++ crates/uv-types/src/builds.rs | 13 +++++++++++++ crates/uv-types/src/traits.rs | 8 ++++++++ 7 files changed, 65 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f961b2b5a..fa75564dc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5937,7 +5937,9 @@ name = "uv-types" version = "0.0.1" dependencies = [ "anyhow", + "boxcar", "rustc-hash", + "tempfile", "thiserror 2.0.12", "uv-cache", "uv-configuration", diff --git a/crates/uv-build-frontend/src/lib.rs b/crates/uv-build-frontend/src/lib.rs index df6fc09cf..d35526951 100644 --- a/crates/uv-build-frontend/src/lib.rs +++ b/crates/uv-build-frontend/src/lib.rs @@ -862,6 +862,10 @@ impl SourceBuild { } impl SourceBuildTrait for SourceBuild { + fn into_build_dir(self) -> TempDir { + self.temp_dir + } + async fn metadata(&mut self) -> Result, AnyErrorBuild> { Ok(self.get_metadata_without_build().await?) } diff --git a/crates/uv-dispatch/src/lib.rs b/crates/uv-dispatch/src/lib.rs index 207f241ad..d95a4f39a 100644 --- a/crates/uv-dispatch/src/lib.rs +++ b/crates/uv-dispatch/src/lib.rs @@ -2,15 +2,15 @@ //! [installer][`uv_installer`] and [build][`uv_build`] through [`BuildDispatch`] //! implementing [`BuildContext`]. -use std::ffi::{OsStr, OsString}; -use std::path::Path; - use anyhow::{Context, Result}; use futures::FutureExt; use itertools::Itertools; use rustc_hash::FxHashMap; +use std::ffi::{OsStr, OsString}; +use std::path::Path; use thiserror::Error; use tracing::{debug, instrument, trace}; + use uv_build_backend::check_direct_build; use uv_build_frontend::{SourceBuild, SourceBuildContext}; use uv_cache::Cache; @@ -35,8 +35,8 @@ use uv_resolver::{ PythonRequirement, Resolver, ResolverEnvironment, }; use uv_types::{ - AnyErrorBuild, BuildContext, BuildIsolation, BuildStack, EmptyInstalledPackages, HashStrategy, - InFlight, + AnyErrorBuild, BuildArena, BuildContext, BuildIsolation, BuildStack, EmptyInstalledPackages, + HashStrategy, InFlight, }; use uv_workspace::WorkspaceCache; @@ -179,6 +179,10 @@ impl BuildContext for BuildDispatch<'_> { &self.shared_state.git } + fn build_arena(&self) -> &BuildArena { + &self.shared_state.build_arena + } + fn capabilities(&self) -> &IndexCapabilities { &self.shared_state.capabilities } @@ -521,6 +525,8 @@ pub struct SharedState { index: InMemoryIndex, /// The downloaded distributions. in_flight: InFlight, + /// Build directories for any PEP 517 builds executed during resolution or installation. + build_arena: BuildArena, } impl SharedState { @@ -533,6 +539,7 @@ impl SharedState { Self { git: self.git.clone(), capabilities: self.capabilities.clone(), + build_arena: self.build_arena.clone(), ..Default::default() } } @@ -556,4 +563,9 @@ impl SharedState { pub fn capabilities(&self) -> &IndexCapabilities { &self.capabilities } + + /// Return the [`BuildArena`] used by the [`SharedState`]. + pub fn build_arena(&self) -> &BuildArena { + &self.build_arena + } } diff --git a/crates/uv-distribution/src/source/mod.rs b/crates/uv-distribution/src/source/mod.rs index 7be26fece..5116ce72b 100644 --- a/crates/uv-distribution/src/source/mod.rs +++ b/crates/uv-distribution/src/source/mod.rs @@ -2276,6 +2276,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { fs::create_dir_all(&cache_shard) .await .map_err(Error::CacheWrite)?; + // Try a direct build if that isn't disabled and the uv build backend is used. let disk_filename = if let Some(name) = self .build_context @@ -2296,7 +2297,8 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { // In the uv build backend, the normalized filename and the disk filename are the same. name.to_string() } else { - self.build_context + let builder = self + .build_context .setup_build( source_root, subdirectory, @@ -2313,10 +2315,17 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { self.build_stack.cloned().unwrap_or_default(), ) .await - .map_err(|err| Error::Build(err.into()))? - .wheel(temp_dir.path()) - .await - .map_err(Error::Build)? + .map_err(|err| Error::Build(err.into()))?; + + // Build the wheel. + let wheel = builder.wheel(temp_dir.path()).await.map_err(Error::Build)?; + + // Store a reference to the build context. + self.build_context + .build_arena() + .push(builder.into_build_dir()); + + wheel }; // Read the metadata from the wheel. @@ -2398,6 +2407,11 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { return Ok(None); }; + // Store a reference to the build context. + self.build_context + .build_arena() + .push(builder.into_build_dir()); + // Read the metadata from disk. debug!("Prepared metadata for: {source}"); let content = fs::read(dist_info.join("METADATA")) diff --git a/crates/uv-types/Cargo.toml b/crates/uv-types/Cargo.toml index 0973a5218..7c1ca60b3 100644 --- a/crates/uv-types/Cargo.toml +++ b/crates/uv-types/Cargo.toml @@ -31,7 +31,9 @@ uv-redacted = { workspace = true } uv-workspace = { workspace = true } anyhow = { workspace = true } +boxcar = { workspace = true } rustc-hash = { workspace = true } +tempfile = { workspace = true } thiserror = { workspace = true } [features] diff --git a/crates/uv-types/src/builds.rs b/crates/uv-types/src/builds.rs index ea5e0b6a3..759dd4135 100644 --- a/crates/uv-types/src/builds.rs +++ b/crates/uv-types/src/builds.rs @@ -1,3 +1,5 @@ +use std::sync::Arc; +use tempfile::TempDir; use uv_pep508::PackageName; use uv_python::PythonEnvironment; @@ -37,3 +39,14 @@ impl BuildIsolation<'_> { } } } + +/// An arena of temporary directories used for builds. +#[derive(Default, Debug, Clone)] +pub struct BuildArena(Arc>); + +impl BuildArena { + /// Push a new temporary directory into the arena. + pub fn push(&self, temp_dir: TempDir) { + self.0.push(temp_dir); + } +} diff --git a/crates/uv-types/src/traits.rs b/crates/uv-types/src/traits.rs index 6f724b27a..16f58fe13 100644 --- a/crates/uv-types/src/traits.rs +++ b/crates/uv-types/src/traits.rs @@ -5,7 +5,9 @@ use std::path::{Path, PathBuf}; use anyhow::Result; use rustc_hash::FxHashSet; +use tempfile::TempDir; +use crate::BuildArena; use uv_cache::Cache; use uv_configuration::{BuildKind, BuildOptions, BuildOutput, ConfigSettings, SourceStrategy}; use uv_distribution_filename::DistFilename; @@ -67,6 +69,9 @@ pub trait BuildContext { /// Return a reference to the Git resolver. fn git(&self) -> &GitResolver; + /// Return a reference to the build arena. + fn build_arena(&self) -> &BuildArena; + /// Return a reference to the discovered registry capabilities. fn capabilities(&self) -> &IndexCapabilities; @@ -148,6 +153,9 @@ pub trait BuildContext { /// You can either call only `wheel()` to build the wheel directly, call only `metadata()` to get /// the metadata without performing the actual or first call `metadata()` and then `wheel()`. pub trait SourceBuildTrait { + /// Return the temporary build directory. + fn into_build_dir(self) -> TempDir; + /// A wrapper for `uv_build::SourceBuild::get_metadata_without_build`. /// /// For PEP 517 builds, this calls `prepare_metadata_for_build_wheel`