Skip to content

Commit

Permalink
Ban --no-cache with --link-mode=symlink (#5519)
Browse files Browse the repository at this point in the history
## Summary

Closes #5360.
  • Loading branch information
charliermarsh committed Jul 28, 2024
1 parent 4b41284 commit b0c841e
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 7 deletions.
5 changes: 5 additions & 0 deletions crates/install-wheel-rs/src/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 9 additions & 4 deletions crates/uv-cache/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Arc<tempfile::TempDir>>,
temp_dir: Option<Arc<tempfile::TempDir>>,
}

impl Cache {
Expand All @@ -129,7 +129,7 @@ impl Cache {
Self {
root: root.into(),
refresh: Refresh::None(Timestamp::now()),
_temp_dir_drop: None,
temp_dir: None,
}
}

Expand All @@ -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)),
})
}

Expand Down Expand Up @@ -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<Self, io::Error> {
let root = &self.root;

Expand Down
1 change: 1 addition & 0 deletions crates/uv-dispatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")?;
Expand Down
35 changes: 32 additions & 3 deletions crates/uv-installer/src/installer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Box<dyn Reporter>>,
installer_name: Option<String>,
}
Expand All @@ -22,6 +24,7 @@ impl<'a> Installer<'a> {
Self {
venv,
link_mode: LinkMode::default(),
cache: None,
reporter: None,
installer_name: Some("uv".to_string()),
}
Expand All @@ -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 {
Expand All @@ -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<CachedDist>) -> Result<Vec<CachedDist>> {
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();
Expand All @@ -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<CachedDist>) -> Result<Vec<CachedDist>> {
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(),
Expand Down
1 change: 1 addition & 0 deletions crates/uv/src/commands/pip/operations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 28 additions & 0 deletions crates/uv/tests/pip_sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<()> {
Expand Down

0 comments on commit b0c841e

Please sign in to comment.