Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ban --no-cache with --link-mode=symlink #5519

Merged
merged 1 commit into from
Jul 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading