From 9ccb632dbb860c1df1d3a5c8927cb303fecabe0d Mon Sep 17 00:00:00 2001 From: Zanie Blue Date: Thu, 26 Jun 2025 12:22:38 -0500 Subject: [PATCH] Require `--global` for removal of the global Python pin (#14169) While reviewing https://github.com/astral-sh/uv/pull/14107, @oconnor663 pointed out a bug where we allow `uv python pin --rm` to delete the global pin without the `--global` flag. I think that shouldn't be allowed? I'm not 100% certain though. --- Cargo.lock | 1 - crates/uv-python/src/version_files.rs | 16 +++++++++++++ crates/uv/Cargo.toml | 1 - crates/uv/src/commands/python/pin.rs | 22 ++++++++++++------ crates/uv/tests/it/python_pin.rs | 33 +++++++++++++++++++++++++-- 5 files changed, 62 insertions(+), 11 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0069cbb65..e0979cb0a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4695,7 +4695,6 @@ dependencies = [ "uv-client", "uv-configuration", "uv-console", - "uv-dirs", "uv-dispatch", "uv-distribution", "uv-distribution-filename", diff --git a/crates/uv-python/src/version_files.rs b/crates/uv-python/src/version_files.rs index a9cd05b7e..595a18f0f 100644 --- a/crates/uv-python/src/version_files.rs +++ b/crates/uv-python/src/version_files.rs @@ -217,6 +217,19 @@ impl PythonVersionFile { } } + /// Create a new representation of a global Python version file. + /// + /// Returns [`None`] if the user configuration directory cannot be determined. + pub fn global() -> Option { + let path = user_uv_config_dir()?.join(PYTHON_VERSION_FILENAME); + Some(Self::new(path)) + } + + /// Returns `true` if the version file is a global version file. + pub fn is_global(&self) -> bool { + PythonVersionFile::global().is_some_and(|global| self.path() == global.path()) + } + /// Return the first request declared in the file, if any. pub fn version(&self) -> Option<&PythonRequest> { self.versions.first() @@ -260,6 +273,9 @@ impl PythonVersionFile { /// Update the version file on the file system. pub async fn write(&self) -> Result<(), std::io::Error> { debug!("Writing Python versions to `{}`", self.path.display()); + if let Some(parent) = self.path.parent() { + fs_err::tokio::create_dir_all(parent).await?; + } fs::tokio::write( &self.path, self.versions diff --git a/crates/uv/Cargo.toml b/crates/uv/Cargo.toml index 904cc8fc3..3ea17877b 100644 --- a/crates/uv/Cargo.toml +++ b/crates/uv/Cargo.toml @@ -24,7 +24,6 @@ uv-cli = { workspace = true } uv-client = { workspace = true } uv-configuration = { workspace = true } uv-console = { workspace = true } -uv-dirs = { workspace = true } uv-dispatch = { workspace = true } uv-distribution = { workspace = true } uv-distribution-filename = { workspace = true } diff --git a/crates/uv/src/commands/python/pin.rs b/crates/uv/src/commands/python/pin.rs index f4d10cdfa..0e78e6b5c 100644 --- a/crates/uv/src/commands/python/pin.rs +++ b/crates/uv/src/commands/python/pin.rs @@ -9,7 +9,6 @@ use tracing::debug; use uv_cache::Cache; use uv_client::BaseClientBuilder; use uv_configuration::{DependencyGroupsWithDefaults, PreviewMode}; -use uv_dirs::user_uv_config_dir; use uv_fs::Simplified; use uv_python::{ EnvironmentPreference, PYTHON_VERSION_FILENAME, PythonDownloads, PythonInstallation, @@ -72,10 +71,20 @@ pub(crate) async fn pin( } bail!("No Python version file found"); }; + + if !global && file.is_global() { + bail!("No Python version file found; use `--rm --global` to remove the global pin"); + } + fs_err::tokio::remove_file(file.path()).await?; writeln!( printer.stdout(), - "Removed Python version file at `{}`", + "Removed {} at `{}`", + if global { + "global Python pin" + } else { + "Python version file" + }, file.path().user_display() )?; return Ok(ExitStatus::Success); @@ -194,12 +203,11 @@ pub(crate) async fn pin( let existing = version_file.ok().flatten(); // TODO(zanieb): Allow updating the discovered version file with an `--update` flag. let new = if global { - let Some(config_dir) = user_uv_config_dir() else { - return Err(anyhow::anyhow!("No user-level config directory found.")); + let Some(new) = PythonVersionFile::global() else { + // TODO(zanieb): We should find a nice way to surface that as an error + bail!("Failed to determine directory for global Python pin"); }; - fs_err::tokio::create_dir_all(&config_dir).await?; - PythonVersionFile::new(config_dir.join(PYTHON_VERSION_FILENAME)) - .with_versions(vec![request]) + new.with_versions(vec![request]) } else { PythonVersionFile::new(project_dir.join(PYTHON_VERSION_FILENAME)) .with_versions(vec![request]) diff --git a/crates/uv/tests/it/python_pin.rs b/crates/uv/tests/it/python_pin.rs index cf8849f42..97093831c 100644 --- a/crates/uv/tests/it/python_pin.rs +++ b/crates/uv/tests/it/python_pin.rs @@ -847,7 +847,7 @@ fn python_pin_rm() { error: No Python version file found "); - // Remove the local pin + // Create and remove a local pin context.python_pin().arg("3.12").assert().success(); uv_snapshot!(context.filters(), context.python_pin().arg("--rm"), @r" success: true @@ -884,12 +884,41 @@ fn python_pin_rm() { .arg("--global") .assert() .success(); + uv_snapshot!(context.filters(), context.python_pin().arg("--rm").arg("--global"), @r" success: true exit_code: 0 ----- stdout ----- - Removed Python version file at `[UV_USER_CONFIG_DIR]/.python-version` + Removed global Python pin at `[UV_USER_CONFIG_DIR]/.python-version` ----- stderr ----- "); + + // Add the global pin again + context + .python_pin() + .arg("3.12") + .arg("--global") + .assert() + .success(); + + // Remove the local pin + uv_snapshot!(context.filters(), context.python_pin().arg("--rm"), @r" + success: true + exit_code: 0 + ----- stdout ----- + Removed Python version file at `.python-version` + + ----- stderr ----- + "); + + // The global pin should not be removed without `--global` + uv_snapshot!(context.filters(), context.python_pin().arg("--rm"), @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + error: No Python version file found; use `--rm --global` to remove the global pin + "); }