From cf082869ee559c3a8b38d1bb981cc38b4486da65 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 28 Jul 2024 14:05:56 -0400 Subject: [PATCH] Ban --no-cache with --link-mode=symlink --- crates/install-wheel-rs/src/linker.rs | 5 ++++ crates/uv-cache/src/lib.rs | 13 ++++++--- crates/uv-dispatch/src/lib.rs | 1 + crates/uv-installer/src/installer.rs | 35 ++++++++++++++++++++++-- crates/uv/src/commands/pip/operations.rs | 1 + crates/uv/tests/pip_sync.rs | 28 +++++++++++++++++++ 6 files changed, 76 insertions(+), 7 deletions(-) diff --git a/crates/install-wheel-rs/src/linker.rs b/crates/install-wheel-rs/src/linker.rs index 1bc70aadf4b9..7d71f198ddde 100644 --- a/crates/install-wheel-rs/src/linker.rs +++ b/crates/install-wheel-rs/src/linker.rs @@ -258,6 +258,11 @@ impl LinkMode { Self::Symlink => symlink_wheel_files(site_packages, wheel, locks), } } + + /// Returns `true` if the link mode is [`LinkMode::Symlink`]. + pub fn is_symlink(&self) -> bool { + matches!(self, Self::Symlink) + } } /// Extract a wheel by cloning all of its files into site packages. The files will be cloned diff --git a/crates/uv-cache/src/lib.rs b/crates/uv-cache/src/lib.rs index 457a0513ee13..6c92df98aa3b 100644 --- a/crates/uv-cache/src/lib.rs +++ b/crates/uv-cache/src/lib.rs @@ -120,7 +120,7 @@ pub struct Cache { /// /// Included to ensure that the temporary directory exists for the length of the operation, but /// is dropped at the end as appropriate. - _temp_dir_drop: Option>, + temp_dir: Option>, } impl Cache { @@ -129,7 +129,7 @@ impl Cache { Self { root: root.into(), refresh: Refresh::None(Timestamp::now()), - _temp_dir_drop: None, + temp_dir: None, } } @@ -139,7 +139,7 @@ impl Cache { Ok(Self { root: temp_dir.path().to_path_buf(), refresh: Refresh::None(Timestamp::now()), - _temp_dir_drop: Some(Arc::new(temp_dir)), + temp_dir: Some(Arc::new(temp_dir)), }) } @@ -271,7 +271,12 @@ impl Cache { Ok(id) } - /// Initialize the cache. + /// Returns `true` if the [`Cache`] is temporary. + pub fn is_temporary(&self) -> bool { + self.temp_dir.is_some() + } + + /// Initialize the [`Cache`]. pub fn init(self) -> Result { let root = &self.root; diff --git a/crates/uv-dispatch/src/lib.rs b/crates/uv-dispatch/src/lib.rs index 3928e63b9666..77c7039206ac 100644 --- a/crates/uv-dispatch/src/lib.rs +++ b/crates/uv-dispatch/src/lib.rs @@ -288,6 +288,7 @@ impl<'a> BuildContext for BuildDispatch<'a> { ); wheels = Installer::new(venv) .with_link_mode(self.link_mode) + .with_cache(self.cache) .install(wheels) .await .context("Failed to install build dependencies")?; diff --git a/crates/uv-installer/src/installer.rs b/crates/uv-installer/src/installer.rs index 008b8d32543b..1e3b7a02a559 100644 --- a/crates/uv-installer/src/installer.rs +++ b/crates/uv-installer/src/installer.rs @@ -7,11 +7,13 @@ use tokio::sync::oneshot; use tracing::instrument; use distribution_types::CachedDist; +use uv_cache::Cache; use uv_python::PythonEnvironment; pub struct Installer<'a> { venv: &'a PythonEnvironment, link_mode: LinkMode, + cache: Option<&'a Cache>, reporter: Option>, installer_name: Option, } @@ -22,6 +24,7 @@ impl<'a> Installer<'a> { Self { venv, link_mode: LinkMode::default(), + cache: None, reporter: None, installer_name: Some("uv".to_string()), } @@ -33,6 +36,15 @@ impl<'a> Installer<'a> { Self { link_mode, ..self } } + /// Set the [`Cache`] to use for this installer. + #[must_use] + pub fn with_cache(self, cache: &'a Cache) -> Self { + Self { + cache: Some(cache), + ..self + } + } + /// Set the [`Reporter`] to use for this installer. #[must_use] pub fn with_reporter(self, reporter: impl Reporter + 'static) -> Self { @@ -54,16 +66,25 @@ impl<'a> Installer<'a> { /// Install a set of wheels into a Python virtual environment. #[instrument(skip_all, fields(num_wheels = %wheels.len()))] pub async fn install(self, wheels: Vec) -> Result> { - let (tx, rx) = oneshot::channel(); - let Self { venv, + cache, link_mode, reporter, installer_name, } = self; - let layout = venv.interpreter().layout(); + if cache.is_some_and(Cache::is_temporary) { + if link_mode.is_symlink() { + return Err(anyhow::anyhow!( + "Symlink-based installation is not supported with `--no-cache`. The created environment will be rendered unusable by the removal of the cache." + )); + } + } + + let (tx, rx) = oneshot::channel(); + + let layout = venv.interpreter().layout(); rayon::spawn(move || { let result = install(wheels, layout, installer_name, link_mode, reporter); tx.send(result).unwrap(); @@ -77,6 +98,14 @@ impl<'a> Installer<'a> { /// Install a set of wheels into a Python virtual environment synchronously. #[instrument(skip_all, fields(num_wheels = %wheels.len()))] pub fn install_blocking(self, wheels: Vec) -> Result> { + if self.cache.is_some_and(Cache::is_temporary) { + if self.link_mode.is_symlink() { + return Err(anyhow::anyhow!( + "Symlink-based installation is not supported with `--no-cache`. The created environment will be rendered unusable by the removal of the cache." + )); + } + } + install( wheels, self.venv.interpreter().layout(), diff --git a/crates/uv/src/commands/pip/operations.rs b/crates/uv/src/commands/pip/operations.rs index 5b57c600868d..6c7418161f90 100644 --- a/crates/uv/src/commands/pip/operations.rs +++ b/crates/uv/src/commands/pip/operations.rs @@ -463,6 +463,7 @@ pub(crate) async fn install( let start = std::time::Instant::now(); wheels = uv_installer::Installer::new(venv) .with_link_mode(link_mode) + .with_cache(cache) .with_reporter(InstallReporter::from(printer).with_length(wheels.len() as u64)) // This technically can block the runtime, but we are on the main thread and // have no other running tasks at this point, so this lets us avoid spawning a blocking diff --git a/crates/uv/tests/pip_sync.rs b/crates/uv/tests/pip_sync.rs index 7361a170c733..622937a0652d 100644 --- a/crates/uv/tests/pip_sync.rs +++ b/crates/uv/tests/pip_sync.rs @@ -236,6 +236,34 @@ fn install_symlink() -> Result<()> { Ok(()) } +/// Reject attempts to use symlink semantics with `--no-cache`. +#[test] +fn install_symlink_no_cache() -> Result<()> { + let context = TestContext::new("3.12"); + + let requirements_txt = context.temp_dir.child("requirements.txt"); + requirements_txt.write_str("MarkupSafe==2.1.3")?; + + uv_snapshot!(context.pip_sync() + .arg("requirements.txt") + .arg("--link-mode") + .arg("symlink") + .arg("--no-cache") + .arg("--strict"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + Resolved 1 package in [TIME] + Prepared 1 package in [TIME] + error: Symlink-based installation is not supported with `--no-cache`. The created environment will be rendered unusable by the removal of the cache. + "### + ); + + Ok(()) +} + /// Install multiple packages into a virtual environment. #[test] fn install_many() -> Result<()> {