Ban `--no-cache` with `--link-mode=symlink` (#5519)

## Summary

Closes https://github.com/astral-sh/uv/issues/5360.
This commit is contained in:
Charlie Marsh 2024-07-28 15:01:17 -04:00 committed by GitHub
parent 4b4128446d
commit b0c841ee3b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 76 additions and 7 deletions

View File

@ -258,6 +258,11 @@ impl LinkMode {
Self::Symlink => symlink_wheel_files(site_packages, wheel, locks), 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 /// Extract a wheel by cloning all of its files into site packages. The files will be cloned

View File

@ -120,7 +120,7 @@ pub struct Cache {
/// ///
/// Included to ensure that the temporary directory exists for the length of the operation, but /// Included to ensure that the temporary directory exists for the length of the operation, but
/// is dropped at the end as appropriate. /// is dropped at the end as appropriate.
_temp_dir_drop: Option<Arc<tempfile::TempDir>>, temp_dir: Option<Arc<tempfile::TempDir>>,
} }
impl Cache { impl Cache {
@ -129,7 +129,7 @@ impl Cache {
Self { Self {
root: root.into(), root: root.into(),
refresh: Refresh::None(Timestamp::now()), refresh: Refresh::None(Timestamp::now()),
_temp_dir_drop: None, temp_dir: None,
} }
} }
@ -139,7 +139,7 @@ impl Cache {
Ok(Self { Ok(Self {
root: temp_dir.path().to_path_buf(), root: temp_dir.path().to_path_buf(),
refresh: Refresh::None(Timestamp::now()), 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) 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> { pub fn init(self) -> Result<Self, io::Error> {
let root = &self.root; let root = &self.root;

View File

@ -288,6 +288,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
); );
wheels = Installer::new(venv) wheels = Installer::new(venv)
.with_link_mode(self.link_mode) .with_link_mode(self.link_mode)
.with_cache(self.cache)
.install(wheels) .install(wheels)
.await .await
.context("Failed to install build dependencies")?; .context("Failed to install build dependencies")?;

View File

@ -7,11 +7,13 @@ use tokio::sync::oneshot;
use tracing::instrument; use tracing::instrument;
use distribution_types::CachedDist; use distribution_types::CachedDist;
use uv_cache::Cache;
use uv_python::PythonEnvironment; use uv_python::PythonEnvironment;
pub struct Installer<'a> { pub struct Installer<'a> {
venv: &'a PythonEnvironment, venv: &'a PythonEnvironment,
link_mode: LinkMode, link_mode: LinkMode,
cache: Option<&'a Cache>,
reporter: Option<Box<dyn Reporter>>, reporter: Option<Box<dyn Reporter>>,
installer_name: Option<String>, installer_name: Option<String>,
} }
@ -22,6 +24,7 @@ impl<'a> Installer<'a> {
Self { Self {
venv, venv,
link_mode: LinkMode::default(), link_mode: LinkMode::default(),
cache: None,
reporter: None, reporter: None,
installer_name: Some("uv".to_string()), installer_name: Some("uv".to_string()),
} }
@ -33,6 +36,15 @@ impl<'a> Installer<'a> {
Self { link_mode, ..self } 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. /// Set the [`Reporter`] to use for this installer.
#[must_use] #[must_use]
pub fn with_reporter(self, reporter: impl Reporter + 'static) -> Self { 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. /// Install a set of wheels into a Python virtual environment.
#[instrument(skip_all, fields(num_wheels = %wheels.len()))] #[instrument(skip_all, fields(num_wheels = %wheels.len()))]
pub async fn install(self, wheels: Vec<CachedDist>) -> Result<Vec<CachedDist>> { pub async fn install(self, wheels: Vec<CachedDist>) -> Result<Vec<CachedDist>> {
let (tx, rx) = oneshot::channel();
let Self { let Self {
venv, venv,
cache,
link_mode, link_mode,
reporter, reporter,
installer_name, installer_name,
} = self; } = 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 || { rayon::spawn(move || {
let result = install(wheels, layout, installer_name, link_mode, reporter); let result = install(wheels, layout, installer_name, link_mode, reporter);
tx.send(result).unwrap(); tx.send(result).unwrap();
@ -77,6 +98,14 @@ impl<'a> Installer<'a> {
/// Install a set of wheels into a Python virtual environment synchronously. /// Install a set of wheels into a Python virtual environment synchronously.
#[instrument(skip_all, fields(num_wheels = %wheels.len()))] #[instrument(skip_all, fields(num_wheels = %wheels.len()))]
pub fn install_blocking(self, wheels: Vec<CachedDist>) -> Result<Vec<CachedDist>> { 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( install(
wheels, wheels,
self.venv.interpreter().layout(), self.venv.interpreter().layout(),

View File

@ -463,6 +463,7 @@ pub(crate) async fn install(
let start = std::time::Instant::now(); let start = std::time::Instant::now();
wheels = uv_installer::Installer::new(venv) wheels = uv_installer::Installer::new(venv)
.with_link_mode(link_mode) .with_link_mode(link_mode)
.with_cache(cache)
.with_reporter(InstallReporter::from(printer).with_length(wheels.len() as u64)) .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 // 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 // have no other running tasks at this point, so this lets us avoid spawning a blocking

View File

@ -236,6 +236,34 @@ fn install_symlink() -> Result<()> {
Ok(()) 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. /// Install multiple packages into a virtual environment.
#[test] #[test]
fn install_many() -> Result<()> { fn install_many() -> Result<()> {