From 992ad7d46276956882bcf8ec6e59a24f32f42054 Mon Sep 17 00:00:00 2001 From: Chris Olszewski Date: Mon, 23 Dec 2024 11:34:45 -0600 Subject: [PATCH 1/4] chore(cache): add failing test --- .../src/cache_archive/restore.rs | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/crates/turborepo-cache/src/cache_archive/restore.rs b/crates/turborepo-cache/src/cache_archive/restore.rs index 7dbf1ebb3e7a1..1ea474eb9b629 100644 --- a/crates/turborepo-cache/src/cache_archive/restore.rs +++ b/crates/turborepo-cache/src/cache_archive/restore.rs @@ -934,4 +934,48 @@ mod tests { Ok(()) } + + #[test] + fn test_gh_8476() { + let input_dir = tempdir().unwrap(); + let input_files = &[ + TarFile::Directory { + path: AnchoredSystemPathBuf::from_raw("dist").unwrap(), + }, + TarFile::File { + path: AnchoredSystemPathBuf::from_raw("dist/index.js").unwrap(), + body: b"console.log('hello')".to_vec(), + }, + ]; + let archive_path = generate_tar(&input_dir, input_files).unwrap(); + // create a symlink to something in the output + let output_dir = tempdir().unwrap(); + let output_path = AbsoluteSystemPath::from_std_path(output_dir.path()).unwrap(); + + // Create dist -> src symlink + let output_dist = output_path.join_component("dist"); + let output_src = output_path.join_component("src"); + output_src.create_dir_all().unwrap(); + output_dist.symlink_to_dir("src").unwrap(); + + let output_dir_path = output_dir.path().to_string_lossy(); + let anchor = AbsoluteSystemPath::new(&output_dir_path).unwrap(); + + assert!( + !output_src.join_component("index.js").try_exists().unwrap(), + "src is empty before restore" + ); + + let mut cache_reader = CacheReader::open(&archive_path).unwrap(); + + let actual = cache_reader.restore(anchor).unwrap(); + assert_eq!( + actual, + into_anchored_system_path_vec(vec!["dist", "dist/index.js",]) + ); + assert!( + !output_src.join_component("index.js").try_exists().unwrap(), + "src should remain empty after restore" + ); + } } From 6cb518d2133b2539441fdd080af006b728ddda56 Mon Sep 17 00:00:00 2001 From: Anthony Shew Date: Wed, 2 Jul 2025 19:12:26 -0600 Subject: [PATCH 2/4] fix --- .../src/cache_archive/restore_directory.rs | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/turborepo-cache/src/cache_archive/restore_directory.rs b/crates/turborepo-cache/src/cache_archive/restore_directory.rs index e684de3a9e77e..d019d3e232103 100644 --- a/crates/turborepo-cache/src/cache_archive/restore_directory.rs +++ b/crates/turborepo-cache/src/cache_archive/restore_directory.rs @@ -1,4 +1,4 @@ -use std::{backtrace::Backtrace, ffi::OsString, io}; +use std::{backtrace::Backtrace, ffi::OsString, fs, io}; use camino::Utf8Component; use tar::Entry; @@ -92,6 +92,32 @@ impl CachedDirTree { // // This could _still_ error, but we don't care. let resolved_name = anchor.resolve(processed_name); + + // Before attempting to create the directory, check if there's a symlink + // at this location that would cause cache restoration to the wrong location. + if let Ok(metadata) = resolved_name.symlink_metadata() { + if metadata.is_symlink() { + // Check if the symlink points to a sibling directory (like dist -> src) + if let Ok(link_target) = resolved_name.read_link() { + let is_relative = link_target.is_relative(); + let is_sibling = is_relative + && link_target.components().count() == 1 + && !link_target.as_str().starts_with('.'); + + if is_sibling { + debug!( + "Found sibling symlink at directory location {:?}, removing to ensure \ + correct cache restoration", + resolved_name + ); + if fs::remove_file(resolved_name.as_path()).is_ok() { + debug!("Successfully removed symlink at {:?}", resolved_name); + } + } + } + } + } + let directory_exists = resolved_name.try_exists(); if matches!(directory_exists, Ok(false)) { resolved_name.create_dir_all()?; From 4b9683b773cea168781280b1863e0004afc69edc Mon Sep 17 00:00:00 2001 From: Anthony Shew Date: Wed, 2 Jul 2025 19:14:36 -0600 Subject: [PATCH 3/4] WIP 5da6c --- crates/turborepo-cache/src/cache_archive/restore.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/turborepo-cache/src/cache_archive/restore.rs b/crates/turborepo-cache/src/cache_archive/restore.rs index 1ea474eb9b629..8580c8595d2b0 100644 --- a/crates/turborepo-cache/src/cache_archive/restore.rs +++ b/crates/turborepo-cache/src/cache_archive/restore.rs @@ -936,7 +936,7 @@ mod tests { } #[test] - fn test_gh_8476() { + fn test_restore_with_existing_symlink_does_not_overwrite_target() { let input_dir = tempdir().unwrap(); let input_files = &[ TarFile::Directory { From 575a5965f356a2628a433fa3e803678fe78ef6dc Mon Sep 17 00:00:00 2001 From: Anthony Shew Date: Wed, 2 Jul 2025 19:53:37 -0600 Subject: [PATCH 4/4] fix windows --- .../src/cache_archive/restore_directory.rs | 45 ++++++++++++++++++- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/crates/turborepo-cache/src/cache_archive/restore_directory.rs b/crates/turborepo-cache/src/cache_archive/restore_directory.rs index d019d3e232103..e62f8ca317628 100644 --- a/crates/turborepo-cache/src/cache_archive/restore_directory.rs +++ b/crates/turborepo-cache/src/cache_archive/restore_directory.rs @@ -97,23 +97,64 @@ impl CachedDirTree { // at this location that would cause cache restoration to the wrong location. if let Ok(metadata) = resolved_name.symlink_metadata() { if metadata.is_symlink() { + debug!( + "Found symlink at directory location {:?}, checking if it should be removed", + resolved_name + ); + // Check if the symlink points to a sibling directory (like dist -> src) if let Ok(link_target) = resolved_name.read_link() { + debug!( + "Symlink target: {:?}, is_relative: {}, components: {:?}", + link_target, + link_target.is_relative(), + link_target.components().collect::>() + ); + let is_relative = link_target.is_relative(); let is_sibling = is_relative && link_target.components().count() == 1 && !link_target.as_str().starts_with('.'); + debug!( + "Symlink analysis: is_relative={}, is_sibling={}, target='{}'", + is_relative, + is_sibling, + link_target.as_str() + ); + if is_sibling { debug!( "Found sibling symlink at directory location {:?}, removing to ensure \ correct cache restoration", resolved_name ); - if fs::remove_file(resolved_name.as_path()).is_ok() { - debug!("Successfully removed symlink at {:?}", resolved_name); + + // On Windows, directory symlinks need to be removed with remove_dir() + // On other platforms, use remove_file() for symlinks + #[cfg(windows)] + let removal_result = fs::remove_dir(resolved_name.as_path()); + #[cfg(not(windows))] + let removal_result = fs::remove_file(resolved_name.as_path()); + + match removal_result { + Ok(()) => { + debug!("Successfully removed symlink at {:?}", resolved_name); + } + Err(e) => { + debug!("Failed to remove symlink at {:?}: {}", resolved_name, e); + // Continue anyway - the directory creation + // might still work + } } + } else { + debug!( + "Symlink at {:?} is not a sibling symlink, leaving it intact", + resolved_name + ); } + } else { + debug!("Could not read symlink target at {:?}", resolved_name); } } }