From 3218e364ae97d8bf1fe2007af7ec243e05b44da2 Mon Sep 17 00:00:00 2001 From: konsti Date: Mon, 5 May 2025 18:29:27 +0200 Subject: [PATCH] Use `fs_err` for paths in symlinking errors (#13303) In #13302, there was an IO error without context. This error seems to be caused by a symlink error. Switching as symlinking to `fs_err` ensures these errors will carry context in the future. --- clippy.toml | 3 +++ crates/uv-cache/src/lib.rs | 4 ++-- crates/uv-fs/src/lib.rs | 4 ++-- crates/uv-install-wheel/src/linker.rs | 6 +++--- crates/uv-python/src/downloads.rs | 2 +- crates/uv-virtualenv/src/virtualenv.rs | 2 +- crates/uv/tests/it/build.rs | 2 +- crates/uv/tests/it/pip_install.rs | 6 +++--- crates/uv/tests/it/python_find.rs | 3 ++- 9 files changed, 18 insertions(+), 14 deletions(-) diff --git a/clippy.toml b/clippy.toml index a20fb1d23..191195e33 100644 --- a/clippy.toml +++ b/clippy.toml @@ -35,4 +35,7 @@ disallowed-methods = [ "std::fs::soft_link", "std::fs::symlink_metadata", "std::fs::write", + "std::os::unix::fs::symlink", + "std::os::windows::fs::symlink_dir", + "std::os::windows::fs::symlink_file", ] diff --git a/crates/uv-cache/src/lib.rs b/crates/uv-cache/src/lib.rs index ab0b43123..8b68d4ff3 100644 --- a/crates/uv-cache/src/lib.rs +++ b/crates/uv-cache/src/lib.rs @@ -653,13 +653,13 @@ impl Cache { let dst = dst.as_ref(); // Attempt to create the symlink directly. - match std::os::unix::fs::symlink(&src, dst) { + match fs_err::os::unix::fs::symlink(&src, dst) { Ok(()) => Ok(()), Err(err) if err.kind() == io::ErrorKind::AlreadyExists => { // Create a symlink, using a temporary file to ensure atomicity. let temp_dir = tempfile::tempdir_in(dst.parent().unwrap())?; let temp_file = temp_dir.path().join("link"); - std::os::unix::fs::symlink(&src, &temp_file)?; + fs_err::os::unix::fs::symlink(&src, &temp_file)?; // Move the symlink into the target location. fs_err::rename(&temp_file, dst)?; diff --git a/crates/uv-fs/src/lib.rs b/crates/uv-fs/src/lib.rs index f06a8f817..90838aceb 100644 --- a/crates/uv-fs/src/lib.rs +++ b/crates/uv-fs/src/lib.rs @@ -121,13 +121,13 @@ pub fn replace_symlink(src: impl AsRef, dst: impl AsRef) -> std::io: #[cfg(unix)] pub fn replace_symlink(src: impl AsRef, dst: impl AsRef) -> std::io::Result<()> { // Attempt to create the symlink directly. - match std::os::unix::fs::symlink(src.as_ref(), dst.as_ref()) { + match fs_err::os::unix::fs::symlink(src.as_ref(), dst.as_ref()) { Ok(()) => Ok(()), Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => { // Create a symlink, using a temporary file to ensure atomicity. let temp_dir = tempfile::tempdir_in(dst.as_ref().parent().unwrap())?; let temp_file = temp_dir.path().join("link"); - std::os::unix::fs::symlink(src, &temp_file)?; + fs_err::os::unix::fs::symlink(src, &temp_file)?; // Move the symlink into the target location. fs_err::rename(&temp_file, dst.as_ref())?; diff --git a/crates/uv-install-wheel/src/linker.rs b/crates/uv-install-wheel/src/linker.rs index 5159def6b..4634349ef 100644 --- a/crates/uv-install-wheel/src/linker.rs +++ b/crates/uv-install-wheel/src/linker.rs @@ -493,14 +493,14 @@ fn synchronized_copy(from: &Path, to: &Path, locks: &Locks) -> std::io::Result<( #[cfg(unix)] fn create_symlink, Q: AsRef>(original: P, link: Q) -> std::io::Result<()> { - std::os::unix::fs::symlink(original, link) + fs_err::os::unix::fs::symlink(original, link) } #[cfg(windows)] fn create_symlink, Q: AsRef>(original: P, link: Q) -> std::io::Result<()> { if original.as_ref().is_dir() { - std::os::windows::fs::symlink_dir(original, link) + fs_err::os::windows::fs::symlink_dir(original, link) } else { - std::os::windows::fs::symlink_file(original, link) + fs_err::os::windows::fs::symlink_file(original, link) } } diff --git a/crates/uv-python/src/downloads.rs b/crates/uv-python/src/downloads.rs index 8095bbe07..32cbdb3f9 100644 --- a/crates/uv-python/src/downloads.rs +++ b/crates/uv-python/src/downloads.rs @@ -761,7 +761,7 @@ impl ManagedPythonDownload { // to that date do not. #[cfg(unix)] { - match std::os::unix::fs::symlink( + match fs_err::os::unix::fs::symlink( format!("python{}.{}", self.key.major, self.key.minor), extracted.join("bin").join("python"), ) { diff --git a/crates/uv-virtualenv/src/virtualenv.rs b/crates/uv-virtualenv/src/virtualenv.rs index 5c7d3cb1a..498ae4246 100644 --- a/crates/uv-virtualenv/src/virtualenv.rs +++ b/crates/uv-virtualenv/src/virtualenv.rs @@ -384,7 +384,7 @@ pub(crate) fn create( && interpreter.markers().os_name() == "posix" && interpreter.markers().sys_platform() != "darwin" { - match std::os::unix::fs::symlink("lib", location.join("lib64")) { + match fs_err::os::unix::fs::symlink("lib", location.join("lib64")) { Ok(()) => {} Err(err) if err.kind() == io::ErrorKind::AlreadyExists => {} Err(err) => { diff --git a/crates/uv/tests/it/build.rs b/crates/uv/tests/it/build.rs index ca7b82dc1..53600105b 100644 --- a/crates/uv/tests/it/build.rs +++ b/crates/uv/tests/it/build.rs @@ -1765,7 +1765,7 @@ fn build_with_symlink() -> Result<()> { requires = ["hatchling"] build-backend = "hatchling.build" "#})?; - std::os::unix::fs::symlink( + fs_err::os::unix::fs::symlink( context.temp_dir.child("pyproject.toml.real"), context.temp_dir.child("pyproject.toml"), )?; diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index a3868a58e..132d029d6 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/crates/uv/tests/it/pip_install.rs @@ -3723,7 +3723,7 @@ fn launcher_with_symlink() -> Result<()> { ); #[cfg(windows)] - if let Err(error) = std::os::windows::fs::symlink_file( + if let Err(error) = fs_err::os::windows::fs::symlink_file( context.venv.join("Scripts\\simple_launcher.exe"), context.temp_dir.join("simple_launcher.exe"), ) { @@ -3736,7 +3736,7 @@ fn launcher_with_symlink() -> Result<()> { } #[cfg(unix)] - std::os::unix::fs::symlink( + fs_err::os::unix::fs::symlink( context.venv.join("bin/simple_launcher"), context.temp_dir.join("simple_launcher"), )?; @@ -8123,7 +8123,7 @@ fn install_relocatable() -> Result<()> { #[cfg(unix)] { let script_symlink_path = context.temp_dir.join("black"); - std::os::unix::fs::symlink(script_path, script_symlink_path.clone())?; + fs_err::os::unix::fs::symlink(script_path, script_symlink_path.clone())?; Command::new(script_symlink_path.as_os_str()) .assert() .success() diff --git a/crates/uv/tests/it/python_find.rs b/crates/uv/tests/it/python_find.rs index 0acf825c3..99611c418 100644 --- a/crates/uv/tests/it/python_find.rs +++ b/crates/uv/tests/it/python_find.rs @@ -746,7 +746,8 @@ fn python_required_python_major_minor() { // Symlink it to `python3.11`. fs_err::create_dir_all(context.temp_dir.child("child")).unwrap(); - std::os::unix::fs::symlink(path, context.temp_dir.child("child").join("python3.11")).unwrap(); + fs_err::os::unix::fs::symlink(path, context.temp_dir.child("child").join("python3.11")) + .unwrap(); // Find `python3.11`, which is `>=3.11.4`. uv_snapshot!(context.filters(), context.python_find().arg(">=3.11.4, <3.12").env(EnvVars::UV_TEST_PYTHON_PATH, context.temp_dir.child("child").path()), @r###"