From 2c04a793cd91c98e031d8fc40bcbe21610232847 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 6 Mar 2024 21:09:57 -0500 Subject: [PATCH] Disable installation --- crates/uv-build/src/lib.rs | 63 +++++++++-------- crates/uv-dispatch/src/lib.rs | 25 +------ crates/uv-distribution/src/source/mod.rs | 17 ----- .../uv-interpreter/src/python_environment.rs | 15 +--- crates/uv-resolver/tests/resolver.rs | 8 ++- crates/uv-traits/src/lib.rs | 14 +--- crates/uv-virtualenv/src/lib.rs | 3 +- crates/uv/src/commands/pip_compile.rs | 15 +++- crates/uv/src/commands/pip_install.rs | 12 +++- crates/uv/src/commands/pip_sync.rs | 10 ++- crates/uv/src/compat/mod.rs | 9 --- crates/uv/src/main.rs | 21 ++++++ crates/uv/tests/pip_install.rs | 68 +++++++++++++++++++ 13 files changed, 170 insertions(+), 110 deletions(-) diff --git a/crates/uv-build/src/lib.rs b/crates/uv-build/src/lib.rs index 719fc5fd4fa16..e4812b4de5983 100644 --- a/crates/uv-build/src/lib.rs +++ b/crates/uv-build/src/lib.rs @@ -417,19 +417,24 @@ impl SourceBuild { BuildIsolation::Shared(venv) => venv.clone(), }; - // Setup the build environment. - let resolved_requirements = Self::get_resolved_requirements( - build_context, - source_build_context, - &default_backend, - pep517_backend.as_ref(), - ) - .await?; + // Setup the build environment. If build isolation is disabled, we assume the build + // environment is already setup. + if build_isolation.is_isolated() { + let resolved_requirements = Self::get_resolved_requirements( + build_context, + source_build_context, + &default_backend, + pep517_backend.as_ref(), + ) + .await?; - build_context - .install(&resolved_requirements, &venv) - .await - .map_err(|err| Error::RequirementsInstall("build-system.requires (install)", err))?; + build_context + .install(&resolved_requirements, &venv) + .await + .map_err(|err| { + Error::RequirementsInstall("build-system.requires (install)", err) + })?; + } // Figure out what the modified path should be // Remove the PATH variable from the environment variables if it's there @@ -461,19 +466,23 @@ impl SourceBuild { OsString::from(venv.scripts()) }; - if let Some(pep517_backend) = &pep517_backend { - create_pep517_build_environment( - &source_tree, - &venv, - pep517_backend, - build_context, - &package_id, - build_kind, - &config_settings, - &environment_variables, - &modified_path, - ) - .await?; + // Create the PEP 517 build environment. If build isolation is disabled, we assume the build + // environment is already setup. + if build_isolation.is_isolated() { + if let Some(pep517_backend) = &pep517_backend { + create_pep517_build_environment( + &source_tree, + &venv, + pep517_backend, + build_context, + &package_id, + build_kind, + &config_settings, + &environment_variables, + &modified_path, + ) + .await?; + } } Ok(Self { @@ -925,10 +934,6 @@ async fn run_python_script( environment_variables: &FxHashMap, modified_path: &OsString, ) -> Result { - println!( - "Running python script: {}", - venv.python_executable().display() - ); Command::new(venv.python_executable()) .args(["-c", script]) .current_dir(source_tree.simplified()) diff --git a/crates/uv-dispatch/src/lib.rs b/crates/uv-dispatch/src/lib.rs index 7d6b32dbefc21..e43d2253b6e18 100644 --- a/crates/uv-dispatch/src/lib.rs +++ b/crates/uv-dispatch/src/lib.rs @@ -4,14 +4,12 @@ use std::ffi::OsStr; use std::path::Path; -use std::sync::Arc; use std::{ffi::OsString, future::Future}; use anyhow::{bail, Context, Result}; use futures::FutureExt; use itertools::Itertools; use rustc_hash::FxHashMap; -use tokio::sync::{Mutex, MutexGuard}; use tracing::{debug, instrument}; use distribution_types::{IndexLocations, Name, Resolution, SourceDist}; @@ -44,7 +42,6 @@ pub struct BuildDispatch<'a> { source_build_context: SourceBuildContext, options: Options, build_extra_env_vars: FxHashMap, - mutex: Arc>, } impl<'a> BuildDispatch<'a> { @@ -79,7 +76,6 @@ impl<'a> BuildDispatch<'a> { source_build_context: SourceBuildContext::default(), options: Options::default(), build_extra_env_vars: FxHashMap::default(), - mutex: Arc::new(Mutex::new(())), } } @@ -89,12 +85,6 @@ impl<'a> BuildDispatch<'a> { self } - #[must_use] - pub fn with_build_isolation(mut self, build_isolation: BuildIsolation<'a>) -> Self { - self.build_isolation = build_isolation; - self - } - /// Set the environment variables to be used when building a source distribution. #[must_use] pub fn with_build_extra_env_vars(mut self, sdist_build_env_variables: I) -> Self @@ -114,17 +104,6 @@ impl<'a> BuildDispatch<'a> { impl<'a> BuildContext for BuildDispatch<'a> { type SourceDistBuilder = SourceBuild; - fn mutex(&self) -> Arc> { - self.mutex.clone() - } - - fn lock(&self) -> impl Future>> + Send { - async move { - let guard = self.mutex.lock().await; - Some(guard) - } - } - fn cache(&self) -> &Cache { self.cache } @@ -165,7 +144,7 @@ impl<'a> BuildContext for BuildDispatch<'a> { self.client, self.flat_index, self.index, - self + self, )?; let graph = resolver.resolve().await.with_context(|| { format!( @@ -258,7 +237,7 @@ impl<'a> BuildContext for BuildDispatch<'a> { // Remove any unnecessary packages. if !reinstalls.is_empty() { - for dist_info in reinstalls.iter() { + for dist_info in &reinstalls { let summary = uv_installer::uninstall(dist_info) .await .context("Failed to uninstall build dependencies")?; diff --git a/crates/uv-distribution/src/source/mod.rs b/crates/uv-distribution/src/source/mod.rs index 99d498e19a7b8..5f8051ce1a29a 100644 --- a/crates/uv-distribution/src/source/mod.rs +++ b/crates/uv-distribution/src/source/mod.rs @@ -857,11 +857,6 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { return Err(Error::NoBuild); } - // Lock the build context. - println!("Locking build context"); - let mutex = self.build_context.mutex(); - let lock = mutex.lock().await; - // Build the wheel. fs::create_dir_all(&cache_shard) .await @@ -881,8 +876,6 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { .await .map_err(|err| Error::Build(dist.to_string(), err))?; - drop(lock); - // Read the metadata from the wheel. let filename = WheelFilename::from_str(&disk_filename)?; let metadata = read_wheel_metadata(&filename, cache_shard.join(&disk_filename))?; @@ -909,16 +902,6 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> { ) -> Result, Error> { debug!("Preparing metadata for: {dist}"); - // Lock the build context. - // let mutex; - // let _lock; - // if self.build_context.build_isolation().is_shared() { - // mutex = self.build_context.mutex(); - // _lock = mutex.lock().await; - // } - - // let _lock = self.build_context.lock().await; - // Setup the builder. let mut builder = self .build_context diff --git a/crates/uv-interpreter/src/python_environment.rs b/crates/uv-interpreter/src/python_environment.rs index 7a52a62b86087..44335a3bae4d5 100644 --- a/crates/uv-interpreter/src/python_environment.rs +++ b/crates/uv-interpreter/src/python_environment.rs @@ -1,6 +1,5 @@ use std::env; use std::path::{Path, PathBuf}; -use std::sync::{Arc, Mutex}; use tracing::debug; @@ -16,7 +15,6 @@ use crate::{find_default_python, find_requested_python, Error, Interpreter}; pub struct PythonEnvironment { root: PathBuf, interpreter: Interpreter, - lock: Arc>, } impl PythonEnvironment { @@ -39,7 +37,6 @@ impl PythonEnvironment { Ok(Self { root: venv, interpreter, - lock: Arc::new(Mutex::default()), }) } @@ -55,7 +52,6 @@ impl PythonEnvironment { Ok(Self { root: interpreter.prefix().to_path_buf(), interpreter, - lock: Arc::new(Mutex::default()), }) } @@ -65,16 +61,14 @@ impl PythonEnvironment { Ok(Self { root: interpreter.prefix().to_path_buf(), interpreter, - lock: Arc::new(Mutex::default()), }) } /// Create a [`PythonEnvironment`] from an existing [`Interpreter`] and root directory. - pub fn from_interpreter(interpreter: Interpreter, root: PathBuf) -> Self { + pub fn from_interpreter(interpreter: Interpreter) -> Self { Self { - root, + root: interpreter.prefix().to_path_buf(), interpreter, - lock: Arc::new(Mutex::default()), } } @@ -109,11 +103,6 @@ impl PythonEnvironment { self.interpreter.scripts() } - /// Grab an in-memory lock for the virtual environment to prevent concurrent writes across threads. - pub fn acquire(&self) -> Arc> { - self.lock.clone() - } - /// Grab a file lock for the virtual environment to prevent concurrent writes across processes. pub fn lock(&self) -> Result { if self.interpreter.is_virtualenv() { diff --git a/crates/uv-resolver/tests/resolver.rs b/crates/uv-resolver/tests/resolver.rs index 06e6a05695100..ff76fe17eae48 100644 --- a/crates/uv-resolver/tests/resolver.rs +++ b/crates/uv-resolver/tests/resolver.rs @@ -21,7 +21,9 @@ use uv_resolver::{ DisplayResolutionGraph, InMemoryIndex, Manifest, Options, OptionsBuilder, PreReleaseMode, ResolutionGraph, ResolutionMode, Resolver, }; -use uv_traits::{BuildContext, BuildKind, NoBinary, NoBuild, SetupPyStrategy, SourceBuildTrait}; +use uv_traits::{ + BuildContext, BuildIsolation, BuildKind, NoBinary, NoBuild, SetupPyStrategy, SourceBuildTrait, +}; // Exclude any packages uploaded after this date. static EXCLUDE_NEWER: Lazy> = Lazy::new(|| { @@ -57,6 +59,10 @@ impl BuildContext for DummyContext { &self.interpreter } + fn build_isolation(&self) -> BuildIsolation { + BuildIsolation::Isolated + } + fn no_build(&self) -> &NoBuild { &NoBuild::None } diff --git a/crates/uv-traits/src/lib.rs b/crates/uv-traits/src/lib.rs index 7e08e82d6e819..335195c937424 100644 --- a/crates/uv-traits/src/lib.rs +++ b/crates/uv-traits/src/lib.rs @@ -6,10 +6,8 @@ use std::fmt::{Display, Formatter}; use std::future::Future; use std::path::{Path, PathBuf}; use std::str::FromStr; -use std::sync::Arc; use anyhow::Result; -use tokio::sync::{Mutex, MutexGuard}; use distribution_types::{CachedDist, DistributionId, IndexLocations, Resolution, SourceDist}; use once_map::OnceMap; @@ -56,15 +54,9 @@ use uv_normalize::PackageName; /// `uv-build` to depend on `uv-resolver` which having actual crate dependencies between /// them. -// TODO(konstin): Proper error types pub trait BuildContext: Sync { type SourceDistBuilder: SourceBuildTrait + Send + Sync; - /// Return a mutex that can be used to lock the build context. - fn mutex(&self) -> Arc>; - - fn lock(&self) -> impl Future>> + Send; - /// Return a reference to the cache. fn cache(&self) -> &Cache; @@ -156,9 +148,9 @@ pub enum BuildIsolation<'a> { } impl<'a> BuildIsolation<'a> { - /// Returns `true` if build isolation is not enforced. - pub fn is_shared(&self) -> bool { - matches!(self, Self::Shared(_)) + /// Returns `true` if build isolation is enforced. + pub fn is_isolated(&self) -> bool { + matches!(self, Self::Isolated) } } diff --git a/crates/uv-virtualenv/src/lib.rs b/crates/uv-virtualenv/src/lib.rs index bc65c6dda2d48..800e1847b64e2 100644 --- a/crates/uv-virtualenv/src/lib.rs +++ b/crates/uv-virtualenv/src/lib.rs @@ -64,6 +64,5 @@ pub fn create_venv( // Create the corresponding `PythonEnvironment`. let interpreter = interpreter.with_virtualenv(virtualenv); - let root = interpreter.prefix().to_path_buf(); - Ok(PythonEnvironment::from_interpreter(interpreter, root)) + Ok(PythonEnvironment::from_interpreter(interpreter)) } diff --git a/crates/uv/src/commands/pip_compile.rs b/crates/uv/src/commands/pip_compile.rs index ceb125f5dbfc8..4d64fc9426d61 100644 --- a/crates/uv/src/commands/pip_compile.rs +++ b/crates/uv/src/commands/pip_compile.rs @@ -24,7 +24,7 @@ use uv_client::{Connectivity, FlatIndex, FlatIndexClient, RegistryClientBuilder} use uv_dispatch::BuildDispatch; use uv_fs::Simplified; use uv_installer::{Downloader, NoBinary}; -use uv_interpreter::{Interpreter, PythonVersion}; +use uv_interpreter::{Interpreter, PythonEnvironment, PythonVersion}; use uv_normalize::{ExtraName, PackageName}; use uv_resolver::{ AnnotationStyle, DependencyMode, DisplayResolutionGraph, InMemoryIndex, Manifest, @@ -62,6 +62,7 @@ pub(crate) async fn pip_compile( setup_py: SetupPyStrategy, config_settings: ConfigSettings, connectivity: Connectivity, + no_build_isolation: bool, no_build: &NoBuild, python_version: Option, exclude_newer: Option>, @@ -137,6 +138,7 @@ pub(crate) async fn pip_compile( interpreter.python_version(), interpreter.sys_executable().simplified_display().cyan() ); + if let Some(python_version) = python_version.as_ref() { // If the requested version does not match the version we're using warn the user // _unless_ they have not specified a patch version and that is the only difference @@ -203,6 +205,15 @@ pub(crate) async fn pip_compile( // Track in-flight downloads, builds, etc., across resolutions. let in_flight = InFlight::default(); + // Determine whether to enable build isolation. + let venv; + let build_isolation = if no_build_isolation { + venv = PythonEnvironment::from_interpreter(interpreter.clone()); + BuildIsolation::Shared(&venv) + } else { + BuildIsolation::Isolated + }; + let build_dispatch = BuildDispatch::new( &client, &cache, @@ -213,7 +224,7 @@ pub(crate) async fn pip_compile( &in_flight, setup_py, &config_settings, - BuildIsolation::Isolated, + build_isolation, no_build, &NoBinary::None, ) diff --git a/crates/uv/src/commands/pip_install.rs b/crates/uv/src/commands/pip_install.rs index cdcb47b922693..cd6bd75fd2fe2 100644 --- a/crates/uv/src/commands/pip_install.rs +++ b/crates/uv/src/commands/pip_install.rs @@ -59,6 +59,7 @@ pub(crate) async fn pip_install( setup_py: SetupPyStrategy, connectivity: Connectivity, config_settings: &ConfigSettings, + no_build_isolation: bool, no_build: &NoBuild, no_binary: &NoBinary, strict: bool, @@ -190,6 +191,13 @@ pub(crate) async fn pip_install( FlatIndex::from_entries(entries, tags) }; + // Determine whether to enable build isolation. + let build_isolation = if no_build_isolation { + BuildIsolation::Shared(&venv) + } else { + BuildIsolation::Isolated + }; + // Create a shared in-memory index. let index = InMemoryIndex::default(); @@ -206,7 +214,7 @@ pub(crate) async fn pip_install( &in_flight, setup_py, config_settings, - BuildIsolation::Shared(&venv), + build_isolation, no_build, no_binary, ) @@ -290,7 +298,7 @@ pub(crate) async fn pip_install( &in_flight, setup_py, config_settings, - BuildIsolation::Shared(&venv), + build_isolation, no_build, no_binary, ) diff --git a/crates/uv/src/commands/pip_sync.rs b/crates/uv/src/commands/pip_sync.rs index 4317575aae382..3daacbf02bd2b 100644 --- a/crates/uv/src/commands/pip_sync.rs +++ b/crates/uv/src/commands/pip_sync.rs @@ -38,6 +38,7 @@ pub(crate) async fn pip_sync( setup_py: SetupPyStrategy, connectivity: Connectivity, config_settings: &ConfigSettings, + no_build_isolation: bool, no_build: &NoBuild, no_binary: &NoBinary, strict: bool, @@ -135,6 +136,13 @@ pub(crate) async fn pip_sync( // Track in-flight downloads, builds, etc., across resolutions. let in_flight = InFlight::default(); + // Determine whether to enable build isolation. + let build_isolation = if no_build_isolation { + BuildIsolation::Shared(&venv) + } else { + BuildIsolation::Isolated + }; + // Prep the build context. let build_dispatch = BuildDispatch::new( &client, @@ -146,7 +154,7 @@ pub(crate) async fn pip_sync( &in_flight, setup_py, config_settings, - BuildIsolation::Isolated, + build_isolation, no_build, no_binary, ); diff --git a/crates/uv/src/compat/mod.rs b/crates/uv/src/compat/mod.rs index d95b3b9a13770..bcea487a70a98 100644 --- a/crates/uv/src/compat/mod.rs +++ b/crates/uv/src/compat/mod.rs @@ -30,9 +30,6 @@ pub(crate) struct PipCompileCompatArgs { #[clap(long, hide = true)] build_isolation: bool, - #[clap(long, hide = true)] - no_build_isolation: bool, - #[clap(long, hide = true)] resolver: Option, @@ -117,12 +114,6 @@ impl CompatArgs for PipCompileCompatArgs { ); } - if self.no_build_isolation { - return Err(anyhow!( - "pip-compile's `--no-build-isolation` is unsupported (uv always uses build isolation)." - )); - } - if let Some(resolver) = self.resolver { match resolver { Resolver::Backtracking => { diff --git a/crates/uv/src/main.rs b/crates/uv/src/main.rs index 904bc0f3cd3eb..4b4790452a9cf 100644 --- a/crates/uv/src/main.rs +++ b/crates/uv/src/main.rs @@ -370,6 +370,12 @@ struct PipCompileArgs { #[clap(long)] legacy_setup_py: bool, + /// Disable isolation when building source distributions. + /// + /// Assumes that build dependencies specified by PEP 518 are already installed. + #[clap(long)] + no_build_isolation: bool, + /// Don't build source distributions. /// /// When enabled, resolving will not run arbitrary code. The cached wheels of already-built @@ -550,6 +556,12 @@ struct PipSyncArgs { #[clap(long)] legacy_setup_py: bool, + /// Disable isolation when building source distributions. + /// + /// Assumes that build dependencies specified by PEP 518 are already installed. + #[clap(long)] + no_build_isolation: bool, + /// Don't build source distributions. /// /// When enabled, resolving will not run arbitrary code. The cached wheels of already-built @@ -790,6 +802,12 @@ struct PipInstallArgs { #[clap(long)] legacy_setup_py: bool, + /// Disable isolation when building source distributions. + /// + /// Assumes that build dependencies specified by PEP 518 are already installed. + #[clap(long)] + no_build_isolation: bool, + /// Don't build source distributions. /// /// When enabled, resolving will not run arbitrary code. The cached wheels of already-built @@ -1358,6 +1376,7 @@ async fn run() -> Result { } else { Connectivity::Online }, + args.no_build_isolation, &no_build, args.python_version, args.exclude_newer, @@ -1411,6 +1430,7 @@ async fn run() -> Result { Connectivity::Online }, &config_settings, + args.no_build_isolation, &no_build, &no_binary, args.strict, @@ -1504,6 +1524,7 @@ async fn run() -> Result { Connectivity::Online }, &config_settings, + args.no_build_isolation, &no_build, &no_binary, args.strict, diff --git a/crates/uv/tests/pip_install.rs b/crates/uv/tests/pip_install.rs index 527e7cda14f28..0a2aee8d5e059 100644 --- a/crates/uv/tests/pip_install.rs +++ b/crates/uv/tests/pip_install.rs @@ -2348,3 +2348,71 @@ requires-python = "<=3.8" Ok(()) } + +/// Install with `--no-build-isolation`, to disable isolation during PEP 517 builds. +#[test] +fn no_build_isolation() -> Result<()> { + let context = TestContext::new("3.12"); + let requirements_in = context.temp_dir.child("requirements.in"); + requirements_in.write_str("anyio @ https://files.pythonhosted.org/packages/db/4d/3970183622f0330d3c23d9b8a5f52e365e50381fd484d08e3285104333d3/anyio-4.3.0.tar.gz")?; + + // We expect the build to fail, because `setuptools` is not installed. + uv_snapshot!(command(&context) + .arg("-r") + .arg("requirements.in") + .arg("--no-build-isolation"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: Failed to download and build: anyio @ https://files.pythonhosted.org/packages/db/4d/3970183622f0330d3c23d9b8a5f52e365e50381fd484d08e3285104333d3/anyio-4.3.0.tar.gz + Caused by: Failed to build: anyio @ https://files.pythonhosted.org/packages/db/4d/3970183622f0330d3c23d9b8a5f52e365e50381fd484d08e3285104333d3/anyio-4.3.0.tar.gz + Caused by: Build backend failed to determine metadata through `prepare_metadata_for_build_wheel` with exit status: 1 + --- stdout: + + --- stderr: + Traceback (most recent call last): + File "", line 8, in + ModuleNotFoundError: No module named 'setuptools' + --- + "### + ); + + // Install `setuptools` and `wheel`. + uv_snapshot!(command(&context) + .arg("setuptools") + .arg("wheel"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 2 packages in [TIME] + Downloaded 2 packages in [TIME] + Installed 2 packages in [TIME] + + setuptools==68.2.2 + + wheel==0.41.3 + "###); + + // We expect the build to succeed, since `setuptools` is now installed. + uv_snapshot!(command(&context) + .arg("-r") + .arg("requirements.in") + .arg("--no-build-isolation"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Resolved 3 packages in [TIME] + Downloaded 3 packages in [TIME] + Installed 3 packages in [TIME] + + anyio==0.0.0 (from https://files.pythonhosted.org/packages/db/4d/3970183622f0330d3c23d9b8a5f52e365e50381fd484d08e3285104333d3/anyio-4.3.0.tar.gz) + + idna==3.4 + + sniffio==1.3.0 + "### + ); + + Ok(()) +}