From dd4aeb7b3561112069a5e7957f1bfacf88364a8f Mon Sep 17 00:00:00 2001 From: akrm al-hakimi Date: Fri, 16 Jan 2026 18:40:05 -0500 Subject: [PATCH 1/2] fix: preserve symlinks in copy_dir instead of following them Previously, copy_dir would follow symlinks and copy the target contents. Now it preserves symlinks by reading the link target and creating a new symlink with the same target. --- src/utils/raw.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/utils/raw.rs b/src/utils/raw.rs index 7a26b74063..c54fcdee4b 100644 --- a/src/utils/raw.rs +++ b/src/utils/raw.rs @@ -284,7 +284,10 @@ pub(crate) fn copy_dir(src: &Path, dest: &Path) -> io::Result<()> { let kind = entry.file_type()?; let src = entry.path(); let dest = dest.join(entry.file_name()); - if kind.is_dir() { + // Check for symlinks first - is_dir() follows symlinks + if kind.is_symlink() { + copy_symlink(&src, &dest)?; + } else if kind.is_dir() { copy_dir(&src, &dest)?; } else { fs::copy(&src, &dest)?; @@ -293,6 +296,25 @@ pub(crate) fn copy_dir(src: &Path, dest: &Path) -> io::Result<()> { Ok(()) } +/// Copy a symlink, preserving its target +fn copy_symlink(src: &Path, dest: &Path) -> io::Result<()> { + let target = fs::read_link(src)?; + #[cfg(unix)] + { + std::os::unix::fs::symlink(&target, dest) + } + #[cfg(windows)] + { + // Determine symlink type by checking what the source symlink points to + let meta = fs::metadata(src); + if meta.map(|m| m.is_dir()).unwrap_or(false) { + std::os::windows::fs::symlink_dir(&target, dest) + } else { + std::os::windows::fs::symlink_file(&target, dest) + } + } +} + #[cfg(not(windows))] fn has_cmd(cmd: &str, process: &Process) -> bool { let cmd = format!("{}{}", cmd, env::consts::EXE_SUFFIX); From 19eada1321f3540aa78f6bc56b26a1dc8f680e2a Mon Sep 17 00:00:00 2001 From: akrm al-hakimi Date: Fri, 16 Jan 2026 18:40:12 -0500 Subject: [PATCH 2/2] fix: add copy_file_symlink_to_source for self-installation - copy_file() preserves symlinks (for component installation) - copy_file_symlink_to_source() follows symlinks (for self-installation) Uses copy_file in Transaction::copy_file and modify_file. Uses copy_file_symlink_to_source in self_update.rs and windows.rs. Adds regression tests for symlink preservation. --- src/cli/self_update.rs | 2 +- src/cli/self_update/windows.rs | 2 +- src/dist/component/tests.rs | 205 +++++++++++++++++++++++++++++++++ src/utils/mod.rs | 20 +++- 4 files changed, 225 insertions(+), 4 deletions(-) diff --git a/src/cli/self_update.rs b/src/cli/self_update.rs index 7f09e5050b..eec1849f2d 100644 --- a/src/cli/self_update.rs +++ b/src/cli/self_update.rs @@ -842,7 +842,7 @@ fn install_bins(process: &Process) -> Result<()> { if rustup_path.exists() { utils::remove_file("rustup-bin", &rustup_path)?; } - utils::copy_file(&this_exe_path, &rustup_path)?; + utils::copy_file_symlink_to_source(&this_exe_path, &rustup_path)?; utils::make_executable(&rustup_path)?; install_proxies(process) } diff --git a/src/cli/self_update/windows.rs b/src/cli/self_update/windows.rs index 973fd58d85..9487976337 100644 --- a/src/cli/self_update/windows.rs +++ b/src/cli/self_update/windows.rs @@ -706,7 +706,7 @@ pub(crate) fn delete_rustup_and_cargo_home(process: &Process) -> Result<()> { let numbah: u32 = rand::random(); let gc_exe = work_path.join(format!("rustup-gc-{numbah:x}.exe")); // Copy rustup (probably this process's exe) to the gc exe - utils::copy_file(&rustup_path, &gc_exe)?; + utils::copy_file_symlink_to_source(&rustup_path, &gc_exe)?; let gc_exe_win: Vec<_> = gc_exe.as_os_str().encode_wide().chain(Some(0)).collect(); // Make the sub-process opened by gc exe inherit its attribute. diff --git a/src/dist/component/tests.rs b/src/dist/component/tests.rs index 3c1bfcfe33..d8a15fb40e 100644 --- a/src/dist/component/tests.rs +++ b/src/dist/component/tests.rs @@ -492,3 +492,208 @@ fn rollback_failure_keeps_going() { #[test] #[ignore] fn intermediate_dir_rollback() {} + +#[test] +#[cfg(unix)] +fn copy_dir_preserves_symlinks() { + // copy_dir must preserve symlinks, not follow them + use std::os::unix::fs::symlink; + + let cx = DistContext::new(None).unwrap(); + let mut tx = cx.transaction(); + + let src_dir = cx.pkg_dir.path(); + + let real_file = src_dir.join("real_file.txt"); + utils::write_file("", &real_file, "original content").unwrap(); + + let subdir = src_dir.join("subdir"); + fs::create_dir(&subdir).unwrap(); + + let file_symlink = subdir.join("link_to_file.txt"); + symlink("../real_file.txt", &file_symlink).unwrap(); + + let real_dir = src_dir.join("real_dir"); + fs::create_dir(&real_dir).unwrap(); + utils::write_file("", &real_dir.join("inner.txt"), "inner content").unwrap(); + let dir_symlink = subdir.join("link_to_dir"); + symlink("../real_dir", &dir_symlink).unwrap(); + + assert!( + fs::symlink_metadata(&file_symlink) + .unwrap() + .file_type() + .is_symlink(), + "Source file symlink should be a symlink" + ); + assert!( + fs::symlink_metadata(&dir_symlink) + .unwrap() + .file_type() + .is_symlink(), + "Source dir symlink should be a symlink" + ); + + tx.copy_dir("test-component", PathBuf::from("dest"), src_dir) + .unwrap(); + tx.commit(); + + let dest_file_symlink = cx.prefix.path().join("dest/subdir/link_to_file.txt"); + let dest_dir_symlink = cx.prefix.path().join("dest/subdir/link_to_dir"); + + assert!( + fs::symlink_metadata(&dest_file_symlink) + .unwrap() + .file_type() + .is_symlink(), + "Destination file symlink should be preserved as a symlink" + ); + assert!( + fs::symlink_metadata(&dest_dir_symlink) + .unwrap() + .file_type() + .is_symlink(), + "Destination dir symlink should be preserved as a symlink" + ); + + assert_eq!( + fs::read_link(&dest_file_symlink).unwrap().to_str().unwrap(), + "../real_file.txt", + "File symlink target should be preserved" + ); + assert_eq!( + fs::read_link(&dest_dir_symlink).unwrap().to_str().unwrap(), + "../real_dir", + "Dir symlink target should be preserved" + ); +} + +/// Test that utils::copy_file preserves symlink targets +#[test] +#[cfg(unix)] +fn copy_file_preserves_symlinks() { + use std::os::unix::fs::symlink; + + let tmp = tempfile::tempdir().unwrap(); + let src_dir = tmp.path().join("src"); + let dest_dir = tmp.path().join("dest"); + fs::create_dir_all(&src_dir).unwrap(); + fs::create_dir_all(&dest_dir).unwrap(); + + let real_file = src_dir.join("real_file.txt"); + utils::write_file("", &real_file, "content").unwrap(); + + let link_file = src_dir.join("link.txt"); + symlink("real_file.txt", &link_file).unwrap(); + + assert!( + fs::symlink_metadata(&link_file) + .unwrap() + .file_type() + .is_symlink() + ); + assert_eq!( + fs::read_link(&link_file).unwrap().to_str().unwrap(), + "real_file.txt" + ); + + // copy_file should preserve the symlink target + let dest_link = dest_dir.join("link.txt"); + utils::copy_file(&link_file, &dest_link).unwrap(); + + assert!( + fs::symlink_metadata(&dest_link) + .unwrap() + .file_type() + .is_symlink(), + "copy_file should preserve symlinks" + ); + assert_eq!( + fs::read_link(&dest_link).unwrap().to_str().unwrap(), + "real_file.txt", + "copy_file should preserve the original symlink target" + ); +} + +/// Test that utils::copy_file_symlink_to_source follows symlinks and copies content +#[test] +#[cfg(unix)] +fn copy_file_symlink_to_source_follows_symlinks() { + use std::os::unix::fs::symlink; + + let tmp = tempfile::tempdir().unwrap(); + let src_dir = tmp.path().join("src"); + let dest_dir = tmp.path().join("dest"); + fs::create_dir_all(&src_dir).unwrap(); + fs::create_dir_all(&dest_dir).unwrap(); + + let real_file = src_dir.join("real_file.txt"); + utils::write_file("", &real_file, "original content").unwrap(); + + let link_file = src_dir.join("link.txt"); + symlink("real_file.txt", &link_file).unwrap(); + + assert!( + fs::symlink_metadata(&link_file) + .unwrap() + .file_type() + .is_symlink() + ); + + // copy_file_symlink_to_source should follow the symlink and copy the content + let dest_file = dest_dir.join("copied.txt"); + utils::copy_file_symlink_to_source(&link_file, &dest_file).unwrap(); + + // Destination should be a regular file, NOT a symlink + assert!( + !fs::symlink_metadata(&dest_file) + .unwrap() + .file_type() + .is_symlink(), + "copy_file_symlink_to_source should follow symlinks and create regular files" + ); + assert_eq!( + fs::read_to_string(&dest_file).unwrap(), + "original content", + "copy_file_symlink_to_source should copy the target file's content" + ); +} + +/// Test that Transaction::copy_file (which uses utils::copy_file) preserves symlinks +#[test] +#[cfg(unix)] +fn transaction_copy_file_preserves_symlinks() { + use std::os::unix::fs::symlink; + + let cx = DistContext::new(None).unwrap(); + let mut tx = cx.transaction(); + + let src_dir = cx.pkg_dir.path(); + let real_file = src_dir.join("real_file.txt"); + utils::write_file("", &real_file, "content").unwrap(); + + let link_file = src_dir.join("link.txt"); + symlink("real_file.txt", &link_file).unwrap(); + + tx.copy_file( + "test-component", + PathBuf::from("copied_link.txt"), + &link_file, + ) + .unwrap(); + tx.commit(); + + let dest_link = cx.prefix.path().join("copied_link.txt"); + assert!( + fs::symlink_metadata(&dest_link) + .unwrap() + .file_type() + .is_symlink(), + "Transaction::copy_file should preserve symlinks" + ); + assert_eq!( + fs::read_link(&dest_link).unwrap().to_str().unwrap(), + "real_file.txt", + "Transaction::copy_file should preserve symlink target" + ); +} diff --git a/src/utils/mod.rs b/src/utils/mod.rs index b4ac6f6856..8241e56d7d 100644 --- a/src/utils/mod.rs +++ b/src/utils/mod.rs @@ -226,13 +226,29 @@ pub(crate) fn copy_dir(src: &Path, dest: &Path) -> Result<()> { }) } +/// Copy a file, preserving symlink targets instead of following them. +/// This is the default behavior for component installation. pub(crate) fn copy_file(src: &Path, dest: &Path) -> Result<()> { + copy_file_impl(src, dest, true) +} + +/// Copy a file, following symlinks and copying the target content. +/// Used for self-update where we want the actual file content, not symlink structure. +pub(crate) fn copy_file_symlink_to_source(src: &Path, dest: &Path) -> Result<()> { + copy_file_impl(src, dest, false) +} + +fn copy_file_impl(src: &Path, dest: &Path, preserve_symlink: bool) -> Result<()> { let metadata = fs::symlink_metadata(src).with_context(|| RustupError::ReadingFile { name: "metadata for", path: PathBuf::from(src), })?; - if metadata.file_type().is_symlink() { - symlink_file(src, dest).map(|_| ()) + if preserve_symlink && metadata.file_type().is_symlink() { + let target = fs::read_link(src).with_context(|| RustupError::ReadingFile { + name: "symlink target for", + path: PathBuf::from(src), + })?; + symlink_file(&target, dest).map(|_| ()) } else { fs::copy(src, dest) .with_context(|| {