From 79d99933a0abc8642a26d13121841b73143626ad Mon Sep 17 00:00:00 2001 From: F4RAN Date: Thu, 11 Dec 2025 20:10:03 +0330 Subject: [PATCH 01/10] Start implementing --force flag for python update-shell (#16782) --- crates/uv/src/commands/python/update_shell.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/uv/src/commands/python/update_shell.rs b/crates/uv/src/commands/python/update_shell.rs index 18757ff9e..434565804 100644 --- a/crates/uv/src/commands/python/update_shell.rs +++ b/crates/uv/src/commands/python/update_shell.rs @@ -42,6 +42,8 @@ pub(crate) async fn update_shell(printer: Printer) -> Result { return Ok(ExitStatus::Success); } + println!("[DEBUG] We are here and {:?}",executable_directory); + if Shell::contains_path(&executable_directory) { writeln!( printer.stderr(), @@ -51,6 +53,8 @@ pub(crate) async fn update_shell(printer: Printer) -> Result { return Ok(ExitStatus::Success); } + + // Determine the current shell. let Some(shell) = Shell::from_env() else { return Err(anyhow::anyhow!( From 08bebaa222152185dea1202273bce6cb2f85420b Mon Sep 17 00:00:00 2001 From: F4RAN Date: Thu, 11 Dec 2025 22:58:29 +0330 Subject: [PATCH 02/10] Add --force flag to python update-shell (#16782) This allows users to force prepending the executable directory to PATH even when it's already present, fixing shadowed binaries. --- crates/uv-cli/src/lib.rs | 13 +- crates/uv-shell/src/windows.rs | 23 ++++ crates/uv/src/commands/python/update_shell.rs | 126 ++++++++++++++---- crates/uv/src/commands/tool/update_shell.rs | 108 ++++++++++++--- crates/uv/src/lib.rs | 8 +- crates/uv/tests/it/common/mod.rs | 8 ++ crates/uv/tests/it/main.rs | 4 + crates/uv/tests/it/python_update_shell.rs | 106 +++++++++++++++ 8 files changed, 342 insertions(+), 54 deletions(-) create mode 100644 crates/uv/tests/it/python_update_shell.rs diff --git a/crates/uv-cli/src/lib.rs b/crates/uv-cli/src/lib.rs index 9f9d41a13..6a5ce65a4 100644 --- a/crates/uv-cli/src/lib.rs +++ b/crates/uv-cli/src/lib.rs @@ -5036,7 +5036,7 @@ pub enum ToolCommand { /// The tool executable directory is determined according to the XDG standard and can be /// retrieved with `uv tool dir --bin`. #[command(alias = "ensurepath")] - UpdateShell, + UpdateShell(PythonUpdateShellArgs), /// Show the path to the uv tools directory. /// /// The tools directory is used to store environments and metadata for installed tools. @@ -5790,7 +5790,16 @@ pub enum PythonCommand { /// The Python executable directory is determined according to the XDG standard and can be /// retrieved with `uv python dir --bin`. #[command(alias = "ensurepath")] - UpdateShell, + UpdateShell(PythonUpdateShellArgs), +} + +#[derive(Args)] +pub struct PythonUpdateShellArgs { + /// Force prepending the executable directory to PATH even if it's already present. + /// + /// This is useful when the directory is in PATH but binaries are shadowed by earlier entries. + #[arg(long)] + pub force: bool, } #[derive(Args)] diff --git a/crates/uv-shell/src/windows.rs b/crates/uv-shell/src/windows.rs index 818d5b999..464dc86be 100644 --- a/crates/uv-shell/src/windows.rs +++ b/crates/uv-shell/src/windows.rs @@ -84,3 +84,26 @@ fn prepend_to_path(existing_path: &HSTRING, path: HSTRING) -> Option { Some(HSTRING::from(new_path)) } } + + +/// Remove a path from the `PATH` variable string. +/// +/// Used with `--force` to remove an existing entry before prepending it again. +pub fn remove_from_path(existing_path: &HSTRING, path_to_remove: &Path) -> HSTRING { + if existing_path.is_empty() { + return existing_path.clone(); + } + + let path_str = path_to_remove.to_string_lossy(); + let paths: Vec<&str> = existing_path + .to_string_lossy() + .split(';') + .filter(|p| p.trim() != path_str.as_ref()) + .collect(); + + if paths.is_empty() { + HSTRING::new() + } else { + HSTRING::from(paths.join(";")) + } +} \ No newline at end of file diff --git a/crates/uv/src/commands/python/update_shell.rs b/crates/uv/src/commands/python/update_shell.rs index 434565804..986adb811 100644 --- a/crates/uv/src/commands/python/update_shell.rs +++ b/crates/uv/src/commands/python/update_shell.rs @@ -7,6 +7,7 @@ use owo_colors::OwoColorize; use tokio::io::AsyncWriteExt; use tracing::debug; +use uv_cli::PythonUpdateShellArgs; use uv_fs::Simplified; use uv_python::managed::python_executable_dir; use uv_shell::Shell; @@ -15,7 +16,7 @@ use crate::commands::ExitStatus; use crate::printer::Printer; /// Ensure that the executable directory is in PATH. -pub(crate) async fn update_shell(printer: Printer) -> Result { +pub(crate) async fn update_shell(args: PythonUpdateShellArgs, printer: Printer) -> Result { let executable_directory = python_executable_dir()?; debug!( "Ensuring that the executable directory is in PATH: {}", @@ -24,27 +25,56 @@ pub(crate) async fn update_shell(printer: Printer) -> Result { #[cfg(windows)] { - if uv_shell::windows::prepend_path(&executable_directory)? { - writeln!( - printer.stderr(), - "Updated PATH to include executable directory {}", - executable_directory.simplified_display().cyan() - )?; - writeln!(printer.stderr(), "Restart your shell to apply changes")?; - } else { + use windows::core::HSTRING; + + let is_in_path = !uv_shell::windows::prepend_path(&executable_directory)?; + + if is_in_path && !args.force { + // Already in PATH and not forcing - exit early writeln!( printer.stderr(), "Executable directory {} is already in PATH", executable_directory.simplified_display().cyan() )?; + return Ok(ExitStatus::Success); } - + + if is_in_path && args.force { + // Already in PATH but forcing - remove it first, then prepend + let windows_path = uv_shell::windows::get_windows_path_var()? + .unwrap_or_default(); + + // Remove the path if it exists + let new_path = uv_shell::windows::remove_from_path(&windows_path, &executable_directory); + + // Prepend the path + let final_path = uv_shell::windows::prepend_to_path(&new_path, HSTRING::from(&executable_directory)) + .unwrap_or_else(|| HSTRING::from(&executable_directory)); + + uv_shell::windows::apply_windows_path_var(&final_path)?; + + writeln!( + printer.stderr(), + "Force updated PATH to prioritize executable directory {}", + executable_directory.simplified_display().cyan() + )?; + writeln!(printer.stderr(), "Restart your shell to apply changes")?; + } else if !is_in_path { + // Not in PATH - normal prepend + if uv_shell::windows::prepend_path(&executable_directory)? { + writeln!( + printer.stderr(), + "Updated PATH to include executable directory {}", + executable_directory.simplified_display().cyan() + )?; + writeln!(printer.stderr(), "Restart your shell to apply changes")?; + } + } + return Ok(ExitStatus::Success); } - println!("[DEBUG] We are here and {:?}",executable_directory); - - if Shell::contains_path(&executable_directory) { + if Shell::contains_path(&executable_directory) && !args.force { writeln!( printer.stderr(), "Executable directory {} is already in PATH", @@ -52,7 +82,6 @@ pub(crate) async fn update_shell(printer: Printer) -> Result { )?; return Ok(ExitStatus::Success); } - // Determine the current shell. @@ -86,35 +115,74 @@ pub(crate) async fn update_shell(printer: Printer) -> Result { // Search for the command in the file, to avoid redundant updates. match fs_err::tokio::read_to_string(&file).await { Ok(contents) => { - if contents - .lines() - .map(str::trim) - .filter(|line| !line.starts_with('#')) - .any(|line| line.contains(&command)) - { - debug!( - "Skipping already-updated configuration file: {}", - file.simplified_display() - ); - continue; - } + // Check if command already exists + let command_exists = contents + .lines() + .map(str::trim) + .filter(|line| !line.starts_with('#')) + .any(|line| line.contains(&command)); - // Append the command to the file. + if command_exists { + if args.force { + // With --force: Remove old entry and add it again at the end (ensures first priority) + let new_contents: String = contents + .lines() + .filter(|line| { + let trimmed = line.trim(); + // Remove old # uv comments and lines containing the command + if trimmed == "# uv" || (!trimmed.starts_with('#') && trimmed.contains(&command)) { + false + } else { + true + } + }) + .collect::>() + .join("\n"); + + // Add the command at the end to ensure it's executed last (first in PATH) + let final_contents = format!("{new_contents}\n# uv\n{command}\n"); + fs_err::tokio::OpenOptions::new() .create(true) .truncate(true) .write(true) .open(&file) .await? - .write_all(format!("{contents}\n# uv\n{command}\n").as_bytes()) + .write_all(final_contents.as_bytes()) .await?; writeln!( printer.stderr(), - "Updated configuration file: {}", + "Force updated configuration file: {}", file.simplified_display().cyan() )?; updated = true; + } else { + // Without --force: Skip if already exists + debug!( + "Skipping already-updated configuration file: {}", + file.simplified_display() + ); + continue; + } + } else { + // Command doesn't exist: Add it normally + fs_err::tokio::OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .open(&file) + .await? + .write_all(format!("{contents}\n# uv\n{command}\n").as_bytes()) + .await?; + + writeln!( + printer.stderr(), + "Updated configuration file: {}", + file.simplified_display().cyan() + )?; + updated = true; + } } Err(err) if err.kind() == std::io::ErrorKind::NotFound => { // Ensure that the directory containing the file exists. diff --git a/crates/uv/src/commands/tool/update_shell.rs b/crates/uv/src/commands/tool/update_shell.rs index ab3b28a93..5a43f193e 100644 --- a/crates/uv/src/commands/tool/update_shell.rs +++ b/crates/uv/src/commands/tool/update_shell.rs @@ -7,6 +7,7 @@ use owo_colors::OwoColorize; use tokio::io::AsyncWriteExt; use tracing::debug; +use uv_cli::PythonUpdateShellArgs; use uv_fs::Simplified; use uv_shell::Shell; use uv_tool::tool_executable_dir; @@ -15,7 +16,7 @@ use crate::commands::ExitStatus; use crate::printer::Printer; /// Ensure that the executable directory is in PATH. -pub(crate) async fn update_shell(printer: Printer) -> Result { +pub(crate) async fn update_shell(args: PythonUpdateShellArgs, printer: Printer) -> Result { let executable_directory = tool_executable_dir()?; debug!( "Ensuring that the executable directory is in PATH: {}", @@ -24,25 +25,56 @@ pub(crate) async fn update_shell(printer: Printer) -> Result { #[cfg(windows)] { - if uv_shell::windows::prepend_path(&executable_directory)? { - writeln!( - printer.stderr(), - "Updated PATH to include executable directory {}", - executable_directory.simplified_display().cyan() - )?; - writeln!(printer.stderr(), "Restart your shell to apply changes")?; - } else { + use windows::core::HSTRING; + + let is_in_path = !uv_shell::windows::prepend_path(&executable_directory)?; + + if is_in_path && !args.force { + // Already in PATH and not forcing - exit early writeln!( printer.stderr(), "Executable directory {} is already in PATH", executable_directory.simplified_display().cyan() )?; + return Ok(ExitStatus::Success); } - + + if is_in_path && args.force { + // Already in PATH but forcing - remove it first, then prepend + let windows_path = uv_shell::windows::get_windows_path_var()? + .unwrap_or_default(); + + // Remove the path if it exists + let new_path = uv_shell::windows::remove_from_path(&windows_path, &executable_directory); + + // Prepend the path + let final_path = uv_shell::windows::prepend_to_path(&new_path, HSTRING::from(&executable_directory)) + .unwrap_or_else(|| HSTRING::from(&executable_directory)); + + uv_shell::windows::apply_windows_path_var(&final_path)?; + + writeln!( + printer.stderr(), + "Force updated PATH to prioritize executable directory {}", + executable_directory.simplified_display().cyan() + )?; + writeln!(printer.stderr(), "Restart your shell to apply changes")?; + } else if !is_in_path { + // Not in PATH - normal prepend + if uv_shell::windows::prepend_path(&executable_directory)? { + writeln!( + printer.stderr(), + "Updated PATH to include executable directory {}", + executable_directory.simplified_display().cyan() + )?; + writeln!(printer.stderr(), "Restart your shell to apply changes")?; + } + } + return Ok(ExitStatus::Success); } - if Shell::contains_path(&executable_directory) { + if Shell::contains_path(&executable_directory) && !args.force { writeln!( printer.stderr(), "Executable directory {} is already in PATH", @@ -82,20 +114,57 @@ pub(crate) async fn update_shell(printer: Printer) -> Result { // Search for the command in the file, to avoid redundant updates. match fs_err::tokio::read_to_string(&file).await { Ok(contents) => { - if contents - .lines() - .map(str::trim) - .filter(|line| !line.starts_with('#')) - .any(|line| line.contains(&command)) - { + // Check if command already exists + let command_exists = contents + .lines() + .map(str::trim) + .filter(|line| !line.starts_with('#')) + .any(|line| line.contains(&command)); + + if command_exists { + if args.force { + // With --force: Remove old entry and add it again at the end + let new_contents: String = contents + .lines() + .filter(|line| { + let trimmed = line.trim(); + // Remove old # uv comments and lines containing the command + if trimmed == "# uv" || (!trimmed.starts_with('#') && trimmed.contains(&command)) { + false + } else { + true + } + }) + .collect::>() + .join("\n"); + + let final_contents = format!("{new_contents}\n# uv\n{command}\n"); + + fs_err::tokio::OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .open(&file) + .await? + .write_all(final_contents.as_bytes()) + .await?; + + writeln!( + printer.stderr(), + "Force updated configuration file: {}", + file.simplified_display().cyan() + )?; + updated = true; + } else { + // Without --force: Skip if already exists debug!( "Skipping already-updated configuration file: {}", file.simplified_display() ); continue; } - - // Append the command to the file. + } else { + // Command doesn't exist: Add it normally fs_err::tokio::OpenOptions::new() .create(true) .truncate(true) @@ -112,6 +181,7 @@ pub(crate) async fn update_shell(printer: Printer) -> Result { )?; updated = true; } + } Err(err) if err.kind() == std::io::ErrorKind::NotFound => { // Ensure that the directory containing the file exists. if let Some(parent) = file.parent() { diff --git a/crates/uv/src/lib.rs b/crates/uv/src/lib.rs index d8d4dbb91..9e1b85bb0 100644 --- a/crates/uv/src/lib.rs +++ b/crates/uv/src/lib.rs @@ -1551,9 +1551,9 @@ async fn run(mut cli: Cli) -> Result { commands::tool_uninstall(args.name, printer).await } Commands::Tool(ToolNamespace { - command: ToolCommand::UpdateShell, + command: ToolCommand::UpdateShell(args), }) => { - commands::tool_update_shell(printer).await?; + commands::tool_update_shell(args, printer).await?; Ok(ExitStatus::Success) } Commands::Tool(ToolNamespace { @@ -1745,9 +1745,9 @@ async fn run(mut cli: Cli) -> Result { Ok(ExitStatus::Success) } Commands::Python(PythonNamespace { - command: PythonCommand::UpdateShell, + command: PythonCommand::UpdateShell(args), }) => { - commands::python_update_shell(printer).await?; + commands::python_update_shell(args, printer).await?; Ok(ExitStatus::Success) } Commands::Publish(args) => { diff --git a/crates/uv/tests/it/common/mod.rs b/crates/uv/tests/it/common/mod.rs index 46ea71091..a94023c30 100644 --- a/crates/uv/tests/it/common/mod.rs +++ b/crates/uv/tests/it/common/mod.rs @@ -1288,6 +1288,14 @@ impl TestContext { self.add_shared_options(&mut command, true); command } + + /// Create a `uv python update-shell` command with options shared across scenarios. + pub fn python_update_shell(&self) -> Command { + let mut command = Self::new_command(); + command.arg("python").arg("update-shell"); + self.add_shared_options(&mut command, true); + command + } /// Create a `uv run` command with options shared across scenarios. pub fn run(&self) -> Command { diff --git a/crates/uv/tests/it/main.rs b/crates/uv/tests/it/main.rs index b81292b49..51962c353 100644 --- a/crates/uv/tests/it/main.rs +++ b/crates/uv/tests/it/main.rs @@ -102,6 +102,10 @@ mod python_pin; #[cfg(feature = "python-managed")] mod python_upgrade; +#[cfg(feature = "python")] +mod python_update_shell; + + #[cfg(all(feature = "python", feature = "pypi"))] mod run; diff --git a/crates/uv/tests/it/python_update_shell.rs b/crates/uv/tests/it/python_update_shell.rs new file mode 100644 index 000000000..9520d5a3b --- /dev/null +++ b/crates/uv/tests/it/python_update_shell.rs @@ -0,0 +1,106 @@ +use assert_cmd::assert::OutputAssertExt; +use assert_fs::fixture::PathChild; +use std::fs; + +use uv_static::EnvVars; + +use crate::common::{TestContext, uv_snapshot}; + +#[test] +fn python_update_shell_not_in_path() { + let context = TestContext::new("3.12"); + + // Zsh uses .zshenv, not .zshrc + let shell_config = context.home_dir.child(".zshenv"); + + uv_snapshot!(context.filters(), context + .python_update_shell() + .env(EnvVars::HOME, context.home_dir.as_os_str()) + .env("SHELL", "/bin/zsh"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Created configuration file: [HOME]/.zshenv + Restart your shell to apply changes + "###); + + // Verify the file was created with the correct content + let contents = fs::read_to_string(shell_config.path()).unwrap(); + assert!(contents.contains("export PATH=")); + assert!(contents.contains("# uv")); +} + +#[test] +fn python_update_shell_already_in_path() { + let context = TestContext::new("3.12"); + + // Set a specific bin directory using UV_PYTHON_BIN_DIR + let bin_dir = context.home_dir.child("bin"); + fs::create_dir_all(bin_dir.path()).unwrap(); + + // Set PATH to include the bin directory so it's "already in PATH" + let path_with_bin = std::env::join_paths( + std::iter::once(bin_dir.path().to_path_buf()) + .chain(std::env::split_paths(&std::env::var(EnvVars::PATH).unwrap_or_default())) + ).unwrap(); + + // Run without --force - should skip because it's already in PATH + uv_snapshot!(context.filters(), context + .python_update_shell() + .env(EnvVars::HOME, context.home_dir.as_os_str()) + .env(EnvVars::UV_PYTHON_BIN_DIR, bin_dir.path()) + .env(EnvVars::PATH, path_with_bin) + .env("SHELL", "/bin/zsh"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Executable directory [HOME]/bin is already in PATH + "###); +} + +#[test] +fn python_update_shell_force() { + let context = TestContext::new("3.12"); + + // Zsh uses .zshenv, not .zshrc + let shell_config = context.home_dir.child(".zshenv"); + + // First run - add to PATH + context + .python_update_shell() + .env(EnvVars::HOME, context.home_dir.as_os_str()) + .env("SHELL", "/bin/zsh") + .assert() + .success(); + + let first_contents = fs::read_to_string(shell_config.path()).unwrap(); + let first_count = first_contents.matches("export PATH=").count(); + + // Second run with --force - should update + uv_snapshot!(context.filters(), context + .python_update_shell() + .arg("--force") + .env(EnvVars::HOME, context.home_dir.as_os_str()) + .env("SHELL", "/bin/zsh"), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Force updated configuration file: [HOME]/.zshenv + Restart your shell to apply changes + "###); + + // Verify only one PATH export exists (old one removed, new one added) + let second_contents = fs::read_to_string(shell_config.path()).unwrap(); + let second_count = second_contents.matches("export PATH=").count(); + assert_eq!(first_count, second_count, "Should have same number of PATH exports"); + + // Verify the command is at the end + assert!(second_contents.trim_end().ends_with("export PATH=") || + second_contents.trim_end().ends_with("\"")); +} \ No newline at end of file From e77d39a6e7264955e4e8ccdb5232663a97ddb768 Mon Sep 17 00:00:00 2001 From: F4RAN Date: Thu, 11 Dec 2025 23:04:00 +0330 Subject: [PATCH 03/10] fix: windows some formatting issue --- crates/uv-shell/src/windows.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/uv-shell/src/windows.rs b/crates/uv-shell/src/windows.rs index 464dc86be..3db227c15 100644 --- a/crates/uv-shell/src/windows.rs +++ b/crates/uv-shell/src/windows.rs @@ -93,14 +93,14 @@ pub fn remove_from_path(existing_path: &HSTRING, path_to_remove: &Path) -> HSTRI if existing_path.is_empty() { return existing_path.clone(); } - + let path_str = path_to_remove.to_string_lossy(); let paths: Vec<&str> = existing_path .to_string_lossy() .split(';') .filter(|p| p.trim() != path_str.as_ref()) .collect(); - + if paths.is_empty() { HSTRING::new() } else { From f87e02f1969304077a32b298a39c7d83a89875e4 Mon Sep 17 00:00:00 2001 From: F4RAN Date: Thu, 11 Dec 2025 23:39:44 +0330 Subject: [PATCH 04/10] Fix Windows --force: make helpers public, add read-only check, fix clippy warnings --- crates/uv-shell/src/windows.rs | 23 ++- crates/uv/src/commands/python/update_shell.rs | 177 +++++++++--------- crates/uv/src/commands/tool/update_shell.rs | 150 +++++++-------- crates/uv/tests/it/common/mod.rs | 2 +- crates/uv/tests/it/main.rs | 1 - crates/uv/tests/it/python_update_shell.rs | 45 +++-- 6 files changed, 208 insertions(+), 190 deletions(-) diff --git a/crates/uv-shell/src/windows.rs b/crates/uv-shell/src/windows.rs index 3db227c15..4202e5a63 100644 --- a/crates/uv-shell/src/windows.rs +++ b/crates/uv-shell/src/windows.rs @@ -37,7 +37,7 @@ pub fn prepend_path(path: &Path) -> anyhow::Result { } /// Set the windows `PATH` variable in the registry. -fn apply_windows_path_var(path: &HSTRING) -> anyhow::Result<()> { +pub fn apply_windows_path_var(path: &HSTRING) -> anyhow::Result<()> { let environment = CURRENT_USER.create("Environment")?; if path.is_empty() { @@ -52,7 +52,7 @@ fn apply_windows_path_var(path: &HSTRING) -> anyhow::Result<()> { /// Retrieve the windows `PATH` variable from the registry. /// /// Returns `Ok(None)` if the `PATH` variable is not a string. -fn get_windows_path_var() -> anyhow::Result> { +pub fn get_windows_path_var() -> anyhow::Result> { let environment = CURRENT_USER .create("Environment") .context("Failed to open `Environment` key")?; @@ -72,7 +72,7 @@ fn get_windows_path_var() -> anyhow::Result> { /// Prepend a path to the `PATH` variable in the Windows registry. /// /// Returns `Ok(None)` if the given path is already in `PATH`. -fn prepend_to_path(existing_path: &HSTRING, path: HSTRING) -> Option { +pub fn prepend_to_path(existing_path: &HSTRING, path: HSTRING) -> Option { if existing_path.is_empty() { Some(path) } else if existing_path.windows(path.len()).any(|p| *p == *path) { @@ -85,6 +85,21 @@ fn prepend_to_path(existing_path: &HSTRING, path: HSTRING) -> Option { } } +/// Check if a path is already in the Windows PATH variable (read-only). +pub fn contains_path(path: &Path) -> anyhow::Result { + let windows_path = get_windows_path_var()?.unwrap_or_default(); + if windows_path.is_empty() { + return Ok(false); + } + + let path_str = path.to_string_lossy(); + let path_string = windows_path.to_string_lossy(); + let is_in_path = path_string + .split(';') + .any(|p| p.trim() == path_str.as_ref()); + + Ok(is_in_path) +} /// Remove a path from the `PATH` variable string. /// @@ -106,4 +121,4 @@ pub fn remove_from_path(existing_path: &HSTRING, path_to_remove: &Path) -> HSTRI } else { HSTRING::from(paths.join(";")) } -} \ No newline at end of file +} diff --git a/crates/uv/src/commands/python/update_shell.rs b/crates/uv/src/commands/python/update_shell.rs index 986adb811..a0d6571a0 100644 --- a/crates/uv/src/commands/python/update_shell.rs +++ b/crates/uv/src/commands/python/update_shell.rs @@ -16,7 +16,10 @@ use crate::commands::ExitStatus; use crate::printer::Printer; /// Ensure that the executable directory is in PATH. -pub(crate) async fn update_shell(args: PythonUpdateShellArgs, printer: Printer) -> Result { +pub(crate) async fn update_shell( + args: PythonUpdateShellArgs, + printer: Printer, +) -> Result { let executable_directory = python_executable_dir()?; debug!( "Ensuring that the executable directory is in PATH: {}", @@ -26,9 +29,10 @@ pub(crate) async fn update_shell(args: PythonUpdateShellArgs, printer: Printer) #[cfg(windows)] { use windows::core::HSTRING; - - let is_in_path = !uv_shell::windows::prepend_path(&executable_directory)?; - + + // Check if path is in PATH (read-only, doesn't mutate) + let is_in_path = uv_shell::windows::contains_path(&executable_directory)?; + if is_in_path && !args.force { // Already in PATH and not forcing - exit early writeln!( @@ -38,21 +42,22 @@ pub(crate) async fn update_shell(args: PythonUpdateShellArgs, printer: Printer) )?; return Ok(ExitStatus::Success); } - + if is_in_path && args.force { // Already in PATH but forcing - remove it first, then prepend - let windows_path = uv_shell::windows::get_windows_path_var()? - .unwrap_or_default(); - + let windows_path = uv_shell::windows::get_windows_path_var()?.unwrap_or_default(); + // Remove the path if it exists - let new_path = uv_shell::windows::remove_from_path(&windows_path, &executable_directory); - + let new_path = + uv_shell::windows::remove_from_path(&windows_path, &executable_directory); + // Prepend the path - let final_path = uv_shell::windows::prepend_to_path(&new_path, HSTRING::from(&executable_directory)) - .unwrap_or_else(|| HSTRING::from(&executable_directory)); - + let final_path = + uv_shell::windows::prepend_to_path(&new_path, HSTRING::from(&executable_directory)) + .unwrap_or_else(|| HSTRING::from(&executable_directory)); + uv_shell::windows::apply_windows_path_var(&final_path)?; - + writeln!( printer.stderr(), "Force updated PATH to prioritize executable directory {}", @@ -61,16 +66,15 @@ pub(crate) async fn update_shell(args: PythonUpdateShellArgs, printer: Printer) writeln!(printer.stderr(), "Restart your shell to apply changes")?; } else if !is_in_path { // Not in PATH - normal prepend - if uv_shell::windows::prepend_path(&executable_directory)? { - writeln!( - printer.stderr(), - "Updated PATH to include executable directory {}", - executable_directory.simplified_display().cyan() - )?; - writeln!(printer.stderr(), "Restart your shell to apply changes")?; - } + uv_shell::windows::prepend_path(&executable_directory)?; + writeln!( + printer.stderr(), + "Updated PATH to include executable directory {}", + executable_directory.simplified_display().cyan() + )?; + writeln!(printer.stderr(), "Restart your shell to apply changes")?; } - + return Ok(ExitStatus::Success); } @@ -82,7 +86,6 @@ pub(crate) async fn update_shell(args: PythonUpdateShellArgs, printer: Printer) )?; return Ok(ExitStatus::Success); } - // Determine the current shell. let Some(shell) = Shell::from_env() else { @@ -115,74 +118,70 @@ pub(crate) async fn update_shell(args: PythonUpdateShellArgs, printer: Printer) // Search for the command in the file, to avoid redundant updates. match fs_err::tokio::read_to_string(&file).await { Ok(contents) => { - // Check if command already exists - let command_exists = contents - .lines() - .map(str::trim) - .filter(|line| !line.starts_with('#')) - .any(|line| line.contains(&command)); - - if command_exists { - if args.force { - // With --force: Remove old entry and add it again at the end (ensures first priority) - let new_contents: String = contents + // Check if command already exists + let command_exists = contents .lines() - .filter(|line| { - let trimmed = line.trim(); - // Remove old # uv comments and lines containing the command - if trimmed == "# uv" || (!trimmed.starts_with('#') && trimmed.contains(&command)) { - false - } else { - true - } - }) - .collect::>() - .join("\n"); - - // Add the command at the end to ensure it's executed last (first in PATH) - let final_contents = format!("{new_contents}\n# uv\n{command}\n"); - - fs_err::tokio::OpenOptions::new() - .create(true) - .truncate(true) - .write(true) - .open(&file) - .await? - .write_all(final_contents.as_bytes()) - .await?; + .map(str::trim) + .filter(|line| !line.starts_with('#')) + .any(|line| line.contains(&command)); - writeln!( - printer.stderr(), - "Force updated configuration file: {}", - file.simplified_display().cyan() - )?; - updated = true; - } else { - // Without --force: Skip if already exists - debug!( - "Skipping already-updated configuration file: {}", - file.simplified_display() - ); - continue; - } - } else { - // Command doesn't exist: Add it normally - fs_err::tokio::OpenOptions::new() - .create(true) - .truncate(true) - .write(true) - .open(&file) - .await? - .write_all(format!("{contents}\n# uv\n{command}\n").as_bytes()) - .await?; + if command_exists { + if args.force { + // With --force: Remove old entry and add it again at the end (ensures first priority) + let new_contents: String = contents + .lines() + .filter(|line| { + let trimmed = line.trim(); + // Remove old # uv comments and lines containing the command + !(trimmed == "# uv" + || (!trimmed.starts_with('#') && trimmed.contains(&command))) + }) + .collect::>() + .join("\n"); - writeln!( - printer.stderr(), - "Updated configuration file: {}", - file.simplified_display().cyan() - )?; - updated = true; - } + // Add the command at the end to ensure it's executed last (first in PATH) + let final_contents = format!("{new_contents}\n# uv\n{command}\n"); + + fs_err::tokio::OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .open(&file) + .await? + .write_all(final_contents.as_bytes()) + .await?; + + writeln!( + printer.stderr(), + "Force updated configuration file: {}", + file.simplified_display().cyan() + )?; + updated = true; + } else { + // Without --force: Skip if already exists + debug!( + "Skipping already-updated configuration file: {}", + file.simplified_display() + ); + } + } else { + // Command doesn't exist: Add it normally + fs_err::tokio::OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .open(&file) + .await? + .write_all(format!("{contents}\n# uv\n{command}\n").as_bytes()) + .await?; + + writeln!( + printer.stderr(), + "Updated configuration file: {}", + file.simplified_display().cyan() + )?; + updated = true; + } } Err(err) if err.kind() == std::io::ErrorKind::NotFound => { // Ensure that the directory containing the file exists. diff --git a/crates/uv/src/commands/tool/update_shell.rs b/crates/uv/src/commands/tool/update_shell.rs index 5a43f193e..32f7eae2a 100644 --- a/crates/uv/src/commands/tool/update_shell.rs +++ b/crates/uv/src/commands/tool/update_shell.rs @@ -16,7 +16,10 @@ use crate::commands::ExitStatus; use crate::printer::Printer; /// Ensure that the executable directory is in PATH. -pub(crate) async fn update_shell(args: PythonUpdateShellArgs, printer: Printer) -> Result { +pub(crate) async fn update_shell( + args: PythonUpdateShellArgs, + printer: Printer, +) -> Result { let executable_directory = tool_executable_dir()?; debug!( "Ensuring that the executable directory is in PATH: {}", @@ -26,9 +29,10 @@ pub(crate) async fn update_shell(args: PythonUpdateShellArgs, printer: Printer) #[cfg(windows)] { use windows::core::HSTRING; - - let is_in_path = !uv_shell::windows::prepend_path(&executable_directory)?; - + + // Check if path is in PATH (read-only, doesn't mutate) + let is_in_path = uv_shell::windows::contains_path(&executable_directory)?; + if is_in_path && !args.force { // Already in PATH and not forcing - exit early writeln!( @@ -38,21 +42,22 @@ pub(crate) async fn update_shell(args: PythonUpdateShellArgs, printer: Printer) )?; return Ok(ExitStatus::Success); } - + if is_in_path && args.force { // Already in PATH but forcing - remove it first, then prepend - let windows_path = uv_shell::windows::get_windows_path_var()? - .unwrap_or_default(); - + let windows_path = uv_shell::windows::get_windows_path_var()?.unwrap_or_default(); + // Remove the path if it exists - let new_path = uv_shell::windows::remove_from_path(&windows_path, &executable_directory); - + let new_path = + uv_shell::windows::remove_from_path(&windows_path, &executable_directory); + // Prepend the path - let final_path = uv_shell::windows::prepend_to_path(&new_path, HSTRING::from(&executable_directory)) - .unwrap_or_else(|| HSTRING::from(&executable_directory)); - + let final_path = + uv_shell::windows::prepend_to_path(&new_path, HSTRING::from(&executable_directory)) + .unwrap_or_else(|| HSTRING::from(&executable_directory)); + uv_shell::windows::apply_windows_path_var(&final_path)?; - + writeln!( printer.stderr(), "Force updated PATH to prioritize executable directory {}", @@ -61,16 +66,15 @@ pub(crate) async fn update_shell(args: PythonUpdateShellArgs, printer: Printer) writeln!(printer.stderr(), "Restart your shell to apply changes")?; } else if !is_in_path { // Not in PATH - normal prepend - if uv_shell::windows::prepend_path(&executable_directory)? { - writeln!( - printer.stderr(), - "Updated PATH to include executable directory {}", - executable_directory.simplified_display().cyan() - )?; - writeln!(printer.stderr(), "Restart your shell to apply changes")?; - } + uv_shell::windows::prepend_path(&executable_directory)?; + writeln!( + printer.stderr(), + "Updated PATH to include executable directory {}", + executable_directory.simplified_display().cyan() + )?; + writeln!(printer.stderr(), "Restart your shell to apply changes")?; } - + return Ok(ExitStatus::Success); } @@ -114,73 +118,69 @@ pub(crate) async fn update_shell(args: PythonUpdateShellArgs, printer: Printer) // Search for the command in the file, to avoid redundant updates. match fs_err::tokio::read_to_string(&file).await { Ok(contents) => { - // Check if command already exists - let command_exists = contents - .lines() - .map(str::trim) - .filter(|line| !line.starts_with('#')) - .any(|line| line.contains(&command)); + // Check if command already exists + let command_exists = contents + .lines() + .map(str::trim) + .filter(|line| !line.starts_with('#')) + .any(|line| line.contains(&command)); - if command_exists { - if args.force { - // With --force: Remove old entry and add it again at the end - let new_contents: String = contents - .lines() - .filter(|line| { - let trimmed = line.trim(); - // Remove old # uv comments and lines containing the command - if trimmed == "# uv" || (!trimmed.starts_with('#') && trimmed.contains(&command)) { - false - } else { - true - } - }) - .collect::>() - .join("\n"); - - let final_contents = format!("{new_contents}\n# uv\n{command}\n"); - + if command_exists { + if args.force { + // With --force: Remove old entry and add it again at the end + let new_contents: String = contents + .lines() + .filter(|line| { + let trimmed = line.trim(); + // Remove old # uv comments and lines containing the command + !(trimmed == "# uv" + || (!trimmed.starts_with('#') && trimmed.contains(&command))) + }) + .collect::>() + .join("\n"); + + let final_contents = format!("{new_contents}\n# uv\n{command}\n"); + + fs_err::tokio::OpenOptions::new() + .create(true) + .truncate(true) + .write(true) + .open(&file) + .await? + .write_all(final_contents.as_bytes()) + .await?; + + writeln!( + printer.stderr(), + "Force updated configuration file: {}", + file.simplified_display().cyan() + )?; + updated = true; + } else { + // Without --force: Skip if already exists + debug!( + "Skipping already-updated configuration file: {}", + file.simplified_display() + ); + } + } else { + // Command doesn't exist: Add it normally fs_err::tokio::OpenOptions::new() .create(true) .truncate(true) .write(true) .open(&file) .await? - .write_all(final_contents.as_bytes()) + .write_all(format!("{contents}\n# uv\n{command}\n").as_bytes()) .await?; writeln!( printer.stderr(), - "Force updated configuration file: {}", + "Updated configuration file: {}", file.simplified_display().cyan() )?; updated = true; - } else { - // Without --force: Skip if already exists - debug!( - "Skipping already-updated configuration file: {}", - file.simplified_display() - ); - continue; } - } else { - // Command doesn't exist: Add it normally - fs_err::tokio::OpenOptions::new() - .create(true) - .truncate(true) - .write(true) - .open(&file) - .await? - .write_all(format!("{contents}\n# uv\n{command}\n").as_bytes()) - .await?; - - writeln!( - printer.stderr(), - "Updated configuration file: {}", - file.simplified_display().cyan() - )?; - updated = true; - } } Err(err) if err.kind() == std::io::ErrorKind::NotFound => { // Ensure that the directory containing the file exists. diff --git a/crates/uv/tests/it/common/mod.rs b/crates/uv/tests/it/common/mod.rs index a94023c30..3010c01ad 100644 --- a/crates/uv/tests/it/common/mod.rs +++ b/crates/uv/tests/it/common/mod.rs @@ -1288,7 +1288,7 @@ impl TestContext { self.add_shared_options(&mut command, true); command } - + /// Create a `uv python update-shell` command with options shared across scenarios. pub fn python_update_shell(&self) -> Command { let mut command = Self::new_command(); diff --git a/crates/uv/tests/it/main.rs b/crates/uv/tests/it/main.rs index 51962c353..ade100216 100644 --- a/crates/uv/tests/it/main.rs +++ b/crates/uv/tests/it/main.rs @@ -105,7 +105,6 @@ mod python_upgrade; #[cfg(feature = "python")] mod python_update_shell; - #[cfg(all(feature = "python", feature = "pypi"))] mod run; diff --git a/crates/uv/tests/it/python_update_shell.rs b/crates/uv/tests/it/python_update_shell.rs index 9520d5a3b..3c696ad57 100644 --- a/crates/uv/tests/it/python_update_shell.rs +++ b/crates/uv/tests/it/python_update_shell.rs @@ -9,10 +9,10 @@ use crate::common::{TestContext, uv_snapshot}; #[test] fn python_update_shell_not_in_path() { let context = TestContext::new("3.12"); - + // Zsh uses .zshenv, not .zshrc let shell_config = context.home_dir.child(".zshenv"); - + uv_snapshot!(context.filters(), context .python_update_shell() .env(EnvVars::HOME, context.home_dir.as_os_str()) @@ -25,7 +25,7 @@ fn python_update_shell_not_in_path() { Created configuration file: [HOME]/.zshenv Restart your shell to apply changes "###); - + // Verify the file was created with the correct content let contents = fs::read_to_string(shell_config.path()).unwrap(); assert!(contents.contains("export PATH=")); @@ -35,17 +35,17 @@ fn python_update_shell_not_in_path() { #[test] fn python_update_shell_already_in_path() { let context = TestContext::new("3.12"); - + // Set a specific bin directory using UV_PYTHON_BIN_DIR let bin_dir = context.home_dir.child("bin"); fs::create_dir_all(bin_dir.path()).unwrap(); - + // Set PATH to include the bin directory so it's "already in PATH" - let path_with_bin = std::env::join_paths( - std::iter::once(bin_dir.path().to_path_buf()) - .chain(std::env::split_paths(&std::env::var(EnvVars::PATH).unwrap_or_default())) - ).unwrap(); - + let path_with_bin = std::env::join_paths(std::iter::once(bin_dir.path().to_path_buf()).chain( + std::env::split_paths(&std::env::var(EnvVars::PATH).unwrap_or_default()), + )) + .unwrap(); + // Run without --force - should skip because it's already in PATH uv_snapshot!(context.filters(), context .python_update_shell() @@ -65,10 +65,10 @@ fn python_update_shell_already_in_path() { #[test] fn python_update_shell_force() { let context = TestContext::new("3.12"); - + // Zsh uses .zshenv, not .zshrc let shell_config = context.home_dir.child(".zshenv"); - + // First run - add to PATH context .python_update_shell() @@ -76,10 +76,10 @@ fn python_update_shell_force() { .env("SHELL", "/bin/zsh") .assert() .success(); - + let first_contents = fs::read_to_string(shell_config.path()).unwrap(); let first_count = first_contents.matches("export PATH=").count(); - + // Second run with --force - should update uv_snapshot!(context.filters(), context .python_update_shell() @@ -94,13 +94,18 @@ fn python_update_shell_force() { Force updated configuration file: [HOME]/.zshenv Restart your shell to apply changes "###); - + // Verify only one PATH export exists (old one removed, new one added) let second_contents = fs::read_to_string(shell_config.path()).unwrap(); let second_count = second_contents.matches("export PATH=").count(); - assert_eq!(first_count, second_count, "Should have same number of PATH exports"); - + assert_eq!( + first_count, second_count, + "Should have same number of PATH exports" + ); + // Verify the command is at the end - assert!(second_contents.trim_end().ends_with("export PATH=") || - second_contents.trim_end().ends_with("\"")); -} \ No newline at end of file + assert!( + second_contents.trim_end().ends_with("export PATH=") + || second_contents.trim_end().ends_with("\"") + ); +} From 241e9fb29178d62a09458a7a4a808bae6b9967e7 Mon Sep 17 00:00:00 2001 From: F4RAN Date: Fri, 12 Dec 2025 00:58:10 +0330 Subject: [PATCH 05/10] Address review feedback: fix Windows implementation and clippy warnings - Make Windows helpers public, add read-only PATH check - Fix clippy warnings and test file issues --- crates/uv-shell/src/windows.rs | 4 ++-- crates/uv/tests/it/python_update_shell.rs | 13 ++++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/crates/uv-shell/src/windows.rs b/crates/uv-shell/src/windows.rs index 4202e5a63..a9525c0fa 100644 --- a/crates/uv-shell/src/windows.rs +++ b/crates/uv-shell/src/windows.rs @@ -110,8 +110,8 @@ pub fn remove_from_path(existing_path: &HSTRING, path_to_remove: &Path) -> HSTRI } let path_str = path_to_remove.to_string_lossy(); - let paths: Vec<&str> = existing_path - .to_string_lossy() + let path_string = existing_path.to_string_lossy(); + let paths: Vec<&str> = path_string .split(';') .filter(|p| p.trim() != path_str.as_ref()) .collect(); diff --git a/crates/uv/tests/it/python_update_shell.rs b/crates/uv/tests/it/python_update_shell.rs index 3c696ad57..d79076968 100644 --- a/crates/uv/tests/it/python_update_shell.rs +++ b/crates/uv/tests/it/python_update_shell.rs @@ -1,6 +1,5 @@ use assert_cmd::assert::OutputAssertExt; use assert_fs::fixture::PathChild; -use std::fs; use uv_static::EnvVars; @@ -27,7 +26,7 @@ fn python_update_shell_not_in_path() { "###); // Verify the file was created with the correct content - let contents = fs::read_to_string(shell_config.path()).unwrap(); + let contents = fs_err::read_to_string(shell_config.path()).unwrap(); assert!(contents.contains("export PATH=")); assert!(contents.contains("# uv")); } @@ -38,7 +37,7 @@ fn python_update_shell_already_in_path() { // Set a specific bin directory using UV_PYTHON_BIN_DIR let bin_dir = context.home_dir.child("bin"); - fs::create_dir_all(bin_dir.path()).unwrap(); + fs_err::create_dir_all(bin_dir.path()).unwrap(); // Set PATH to include the bin directory so it's "already in PATH" let path_with_bin = std::env::join_paths(std::iter::once(bin_dir.path().to_path_buf()).chain( @@ -77,7 +76,7 @@ fn python_update_shell_force() { .assert() .success(); - let first_contents = fs::read_to_string(shell_config.path()).unwrap(); + let first_contents = fs_err::read_to_string(shell_config.path()).unwrap(); let first_count = first_contents.matches("export PATH=").count(); // Second run with --force - should update @@ -96,7 +95,7 @@ fn python_update_shell_force() { "###); // Verify only one PATH export exists (old one removed, new one added) - let second_contents = fs::read_to_string(shell_config.path()).unwrap(); + let second_contents = fs_err::read_to_string(shell_config.path()).unwrap(); let second_count = second_contents.matches("export PATH=").count(); assert_eq!( first_count, second_count, @@ -106,6 +105,6 @@ fn python_update_shell_force() { // Verify the command is at the end assert!( second_contents.trim_end().ends_with("export PATH=") - || second_contents.trim_end().ends_with("\"") + || second_contents.trim_end().ends_with('"') ); -} +} \ No newline at end of file From 4c677cae202d68397c290727821bbcf162eaf4ed Mon Sep 17 00:00:00 2001 From: F4RAN Date: Fri, 12 Dec 2025 00:59:33 +0330 Subject: [PATCH 06/10] fix: formatter --- crates/uv/tests/it/python_update_shell.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/uv/tests/it/python_update_shell.rs b/crates/uv/tests/it/python_update_shell.rs index d79076968..01d168c9e 100644 --- a/crates/uv/tests/it/python_update_shell.rs +++ b/crates/uv/tests/it/python_update_shell.rs @@ -107,4 +107,4 @@ fn python_update_shell_force() { second_contents.trim_end().ends_with("export PATH=") || second_contents.trim_end().ends_with('"') ); -} \ No newline at end of file +} From aa7ef6204e468302d34f26be9eea22604f1dfcef Mon Sep 17 00:00:00 2001 From: F4RAN Date: Fri, 12 Dec 2025 01:08:28 +0330 Subject: [PATCH 07/10] fix: Change use windows::core::HSTRING; to use windows_registry::HSTRING; in both files --- crates/uv/src/commands/python/update_shell.rs | 8 +++++--- crates/uv/src/commands/tool/update_shell.rs | 8 +++++--- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/uv/src/commands/python/update_shell.rs b/crates/uv/src/commands/python/update_shell.rs index a0d6571a0..6b0fdaac4 100644 --- a/crates/uv/src/commands/python/update_shell.rs +++ b/crates/uv/src/commands/python/update_shell.rs @@ -52,9 +52,11 @@ pub(crate) async fn update_shell( uv_shell::windows::remove_from_path(&windows_path, &executable_directory); // Prepend the path - let final_path = - uv_shell::windows::prepend_to_path(&new_path, HSTRING::from(&executable_directory)) - .unwrap_or_else(|| HSTRING::from(&executable_directory)); + let final_path = uv_shell::windows::prepend_to_path( + &new_path, + HSTRING::from(executable_directory.as_path()), + ) + .unwrap_or_else(|| HSTRING::from(executable_directory.as_path())); uv_shell::windows::apply_windows_path_var(&final_path)?; diff --git a/crates/uv/src/commands/tool/update_shell.rs b/crates/uv/src/commands/tool/update_shell.rs index 32f7eae2a..9b9c876f0 100644 --- a/crates/uv/src/commands/tool/update_shell.rs +++ b/crates/uv/src/commands/tool/update_shell.rs @@ -52,9 +52,11 @@ pub(crate) async fn update_shell( uv_shell::windows::remove_from_path(&windows_path, &executable_directory); // Prepend the path - let final_path = - uv_shell::windows::prepend_to_path(&new_path, HSTRING::from(&executable_directory)) - .unwrap_or_else(|| HSTRING::from(&executable_directory)); + let final_path = uv_shell::windows::prepend_to_path( + &new_path, + HSTRING::from(executable_directory.as_path()), + ) + .unwrap_or_else(|| HSTRING::from(executable_directory.as_path())); uv_shell::windows::apply_windows_path_var(&final_path)?; From f2a90e4f7a06e40a6e3c8b696e08adb0524c137c Mon Sep 17 00:00:00 2001 From: F4RAN Date: Fri, 12 Dec 2025 01:13:41 +0330 Subject: [PATCH 08/10] fix: imports HSTRING --- crates/uv/src/commands/python/update_shell.rs | 2 +- crates/uv/src/commands/tool/update_shell.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/uv/src/commands/python/update_shell.rs b/crates/uv/src/commands/python/update_shell.rs index 6b0fdaac4..bba0563ec 100644 --- a/crates/uv/src/commands/python/update_shell.rs +++ b/crates/uv/src/commands/python/update_shell.rs @@ -28,7 +28,7 @@ pub(crate) async fn update_shell( #[cfg(windows)] { - use windows::core::HSTRING; + use windows_registry::HSTRING; // Check if path is in PATH (read-only, doesn't mutate) let is_in_path = uv_shell::windows::contains_path(&executable_directory)?; diff --git a/crates/uv/src/commands/tool/update_shell.rs b/crates/uv/src/commands/tool/update_shell.rs index 9b9c876f0..44ed73f85 100644 --- a/crates/uv/src/commands/tool/update_shell.rs +++ b/crates/uv/src/commands/tool/update_shell.rs @@ -28,7 +28,7 @@ pub(crate) async fn update_shell( #[cfg(windows)] { - use windows::core::HSTRING; + use windows_registry::HSTRING; // Check if path is in PATH (read-only, doesn't mutate) let is_in_path = uv_shell::windows::contains_path(&executable_directory)?; From d4f84d6c894d1113af6bc0596c256014ebaa54b6 Mon Sep 17 00:00:00 2001 From: F4RAN Date: Fri, 12 Dec 2025 01:24:00 +0330 Subject: [PATCH 09/10] replace import with a helper --- crates/uv-shell/src/windows.rs | 19 +++++++++++++++++++ crates/uv/src/commands/python/update_shell.rs | 17 +---------------- crates/uv/src/commands/tool/update_shell.rs | 17 +---------------- 3 files changed, 21 insertions(+), 32 deletions(-) diff --git a/crates/uv-shell/src/windows.rs b/crates/uv-shell/src/windows.rs index a9525c0fa..df0d8a83c 100644 --- a/crates/uv-shell/src/windows.rs +++ b/crates/uv-shell/src/windows.rs @@ -85,6 +85,25 @@ pub fn prepend_to_path(existing_path: &HSTRING, path: HSTRING) -> Option anyhow::Result<()> { + let windows_path = get_windows_path_var()?.unwrap_or_default(); + + // Remove the path if it exists + let new_path = remove_from_path(&windows_path, path); + + // Prepend the path + let final_path = + prepend_to_path(&new_path, HSTRING::from(path)).unwrap_or_else(|| HSTRING::from(path)); + + // Set the `PATH` variable in the registry. + apply_windows_path_var(&final_path)?; + + Ok(()) +} + /// Check if a path is already in the Windows PATH variable (read-only). pub fn contains_path(path: &Path) -> anyhow::Result { let windows_path = get_windows_path_var()?.unwrap_or_default(); diff --git a/crates/uv/src/commands/python/update_shell.rs b/crates/uv/src/commands/python/update_shell.rs index bba0563ec..46fb1fd2b 100644 --- a/crates/uv/src/commands/python/update_shell.rs +++ b/crates/uv/src/commands/python/update_shell.rs @@ -28,8 +28,6 @@ pub(crate) async fn update_shell( #[cfg(windows)] { - use windows_registry::HSTRING; - // Check if path is in PATH (read-only, doesn't mutate) let is_in_path = uv_shell::windows::contains_path(&executable_directory)?; @@ -45,20 +43,7 @@ pub(crate) async fn update_shell( if is_in_path && args.force { // Already in PATH but forcing - remove it first, then prepend - let windows_path = uv_shell::windows::get_windows_path_var()?.unwrap_or_default(); - - // Remove the path if it exists - let new_path = - uv_shell::windows::remove_from_path(&windows_path, &executable_directory); - - // Prepend the path - let final_path = uv_shell::windows::prepend_to_path( - &new_path, - HSTRING::from(executable_directory.as_path()), - ) - .unwrap_or_else(|| HSTRING::from(executable_directory.as_path())); - - uv_shell::windows::apply_windows_path_var(&final_path)?; + uv_shell::windows::force_prepend_path(&executable_directory)?; writeln!( printer.stderr(), diff --git a/crates/uv/src/commands/tool/update_shell.rs b/crates/uv/src/commands/tool/update_shell.rs index 44ed73f85..91fe063b8 100644 --- a/crates/uv/src/commands/tool/update_shell.rs +++ b/crates/uv/src/commands/tool/update_shell.rs @@ -28,8 +28,6 @@ pub(crate) async fn update_shell( #[cfg(windows)] { - use windows_registry::HSTRING; - // Check if path is in PATH (read-only, doesn't mutate) let is_in_path = uv_shell::windows::contains_path(&executable_directory)?; @@ -45,20 +43,7 @@ pub(crate) async fn update_shell( if is_in_path && args.force { // Already in PATH but forcing - remove it first, then prepend - let windows_path = uv_shell::windows::get_windows_path_var()?.unwrap_or_default(); - - // Remove the path if it exists - let new_path = - uv_shell::windows::remove_from_path(&windows_path, &executable_directory); - - // Prepend the path - let final_path = uv_shell::windows::prepend_to_path( - &new_path, - HSTRING::from(executable_directory.as_path()), - ) - .unwrap_or_else(|| HSTRING::from(executable_directory.as_path())); - - uv_shell::windows::apply_windows_path_var(&final_path)?; + uv_shell::windows::force_prepend_path(&executable_directory)?; writeln!( printer.stderr(), From 63a06880ff187d4cc6c77fed7cf96df9cee5ab20 Mon Sep 17 00:00:00 2001 From: F4RAN Date: Fri, 12 Dec 2025 01:38:21 +0330 Subject: [PATCH 10/10] fix: edit tests to be windows-compatible --- crates/uv/tests/it/python_update_shell.rs | 208 ++++++++++++++-------- 1 file changed, 137 insertions(+), 71 deletions(-) diff --git a/crates/uv/tests/it/python_update_shell.rs b/crates/uv/tests/it/python_update_shell.rs index 01d168c9e..858af3d7a 100644 --- a/crates/uv/tests/it/python_update_shell.rs +++ b/crates/uv/tests/it/python_update_shell.rs @@ -9,26 +9,45 @@ use crate::common::{TestContext, uv_snapshot}; fn python_update_shell_not_in_path() { let context = TestContext::new("3.12"); - // Zsh uses .zshenv, not .zshrc - let shell_config = context.home_dir.child(".zshenv"); + #[cfg(not(windows))] + { + // Zsh uses .zshenv, not .zshrc + let shell_config = context.home_dir.child(".zshenv"); - uv_snapshot!(context.filters(), context - .python_update_shell() - .env(EnvVars::HOME, context.home_dir.as_os_str()) - .env("SHELL", "/bin/zsh"), @r###" - success: true - exit_code: 0 - ----- stdout ----- + uv_snapshot!(context.filters(), context + .python_update_shell() + .env(EnvVars::HOME, context.home_dir.as_os_str()) + .env("SHELL", "/bin/zsh"), @r###" + success: true + exit_code: 0 + ----- stdout ----- - ----- stderr ----- - Created configuration file: [HOME]/.zshenv - Restart your shell to apply changes - "###); + ----- stderr ----- + Created configuration file: [HOME]/.zshenv + Restart your shell to apply changes + "###); - // Verify the file was created with the correct content - let contents = fs_err::read_to_string(shell_config.path()).unwrap(); - assert!(contents.contains("export PATH=")); - assert!(contents.contains("# uv")); + // Verify the file was created with the correct content + let contents = fs_err::read_to_string(shell_config.path()).unwrap(); + assert!(contents.contains("export PATH=")); + assert!(contents.contains("# uv")); + } + + #[cfg(windows)] + { + // On Windows, PATH is updated in the registry, not a config file + uv_snapshot!(context.filters(), context + .python_update_shell() + .env(EnvVars::HOME, context.home_dir.as_os_str()), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Updated PATH to include executable directory [USER_CONFIG_DIR]/data/../bin + Restart your shell to apply changes + "###); + } } #[test] @@ -39,72 +58,119 @@ fn python_update_shell_already_in_path() { let bin_dir = context.home_dir.child("bin"); fs_err::create_dir_all(bin_dir.path()).unwrap(); - // Set PATH to include the bin directory so it's "already in PATH" - let path_with_bin = std::env::join_paths(std::iter::once(bin_dir.path().to_path_buf()).chain( - std::env::split_paths(&std::env::var(EnvVars::PATH).unwrap_or_default()), - )) - .unwrap(); + #[cfg(not(windows))] + { + // Set PATH to include the bin directory so it's "already in PATH" + let path_with_bin = + std::env::join_paths(std::iter::once(bin_dir.path().to_path_buf()).chain( + std::env::split_paths(&std::env::var(EnvVars::PATH).unwrap_or_default()), + )) + .unwrap(); - // Run without --force - should skip because it's already in PATH - uv_snapshot!(context.filters(), context - .python_update_shell() - .env(EnvVars::HOME, context.home_dir.as_os_str()) - .env(EnvVars::UV_PYTHON_BIN_DIR, bin_dir.path()) - .env(EnvVars::PATH, path_with_bin) - .env("SHELL", "/bin/zsh"), @r###" - success: true - exit_code: 0 - ----- stdout ----- + // Run without --force - should skip because it's already in PATH + uv_snapshot!(context.filters(), context + .python_update_shell() + .env(EnvVars::HOME, context.home_dir.as_os_str()) + .env(EnvVars::UV_PYTHON_BIN_DIR, bin_dir.path()) + .env(EnvVars::PATH, path_with_bin) + .env("SHELL", "/bin/zsh"), @r###" + success: true + exit_code: 0 + ----- stdout ----- - ----- stderr ----- - Executable directory [HOME]/bin is already in PATH - "###); + ----- stderr ----- + Executable directory [HOME]/bin is already in PATH + "###); + } + + #[cfg(windows)] + { + // On Windows, contains_path checks the registry, not env vars + // Since we can't easily set up the registry in tests, we'll test + // that the command succeeds. The "already in PATH" check will + // depend on the actual registry state, which we can't control. + // This test verifies the command works even if the path is already there. + context + .python_update_shell() + .env(EnvVars::HOME, context.home_dir.as_os_str()) + .env(EnvVars::UV_PYTHON_BIN_DIR, bin_dir.path()) + .assert() + .success(); + } } #[test] fn python_update_shell_force() { let context = TestContext::new("3.12"); - // Zsh uses .zshenv, not .zshrc - let shell_config = context.home_dir.child(".zshenv"); + #[cfg(not(windows))] + { + // Zsh uses .zshenv, not .zshrc + let shell_config = context.home_dir.child(".zshenv"); - // First run - add to PATH - context - .python_update_shell() - .env(EnvVars::HOME, context.home_dir.as_os_str()) - .env("SHELL", "/bin/zsh") - .assert() - .success(); + // First run - add to PATH + context + .python_update_shell() + .env(EnvVars::HOME, context.home_dir.as_os_str()) + .env("SHELL", "/bin/zsh") + .assert() + .success(); - let first_contents = fs_err::read_to_string(shell_config.path()).unwrap(); - let first_count = first_contents.matches("export PATH=").count(); + let first_contents = fs_err::read_to_string(shell_config.path()).unwrap(); + let first_count = first_contents.matches("export PATH=").count(); - // Second run with --force - should update - uv_snapshot!(context.filters(), context - .python_update_shell() - .arg("--force") - .env(EnvVars::HOME, context.home_dir.as_os_str()) - .env("SHELL", "/bin/zsh"), @r###" - success: true - exit_code: 0 - ----- stdout ----- + // Second run with --force - should update + uv_snapshot!(context.filters(), context + .python_update_shell() + .arg("--force") + .env(EnvVars::HOME, context.home_dir.as_os_str()) + .env("SHELL", "/bin/zsh"), @r###" + success: true + exit_code: 0 + ----- stdout ----- - ----- stderr ----- - Force updated configuration file: [HOME]/.zshenv - Restart your shell to apply changes - "###); + ----- stderr ----- + Force updated configuration file: [HOME]/.zshenv + Restart your shell to apply changes + "###); - // Verify only one PATH export exists (old one removed, new one added) - let second_contents = fs_err::read_to_string(shell_config.path()).unwrap(); - let second_count = second_contents.matches("export PATH=").count(); - assert_eq!( - first_count, second_count, - "Should have same number of PATH exports" - ); + // Verify only one PATH export exists (old one removed, new one added) + let second_contents = fs_err::read_to_string(shell_config.path()).unwrap(); + let second_count = second_contents.matches("export PATH=").count(); + assert_eq!( + first_count, second_count, + "Should have same number of PATH exports" + ); - // Verify the command is at the end - assert!( - second_contents.trim_end().ends_with("export PATH=") - || second_contents.trim_end().ends_with('"') - ); + // Verify the command is at the end + assert!( + second_contents.trim_end().ends_with("export PATH=") + || second_contents.trim_end().ends_with('"') + ); + } + + #[cfg(windows)] + { + // On Windows, --force updates the registry + // First run - add to PATH + context + .python_update_shell() + .env(EnvVars::HOME, context.home_dir.as_os_str()) + .assert() + .success(); + + // Second run with --force - should update registry + uv_snapshot!(context.filters(), context + .python_update_shell() + .arg("--force") + .env(EnvVars::HOME, context.home_dir.as_os_str()), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + ----- stderr ----- + Force updated PATH to prioritize executable directory [USER_CONFIG_DIR]/data/../bin + Restart your shell to apply changes + "###); + } }