From 18504e7f84c43191ea1f17968f65420a5b8a9212 Mon Sep 17 00:00:00 2001 From: Jason Kuhrt Date: Sat, 24 Oct 2026 19:22:53 -0400 Subject: [PATCH 1/2] fix(cache): restore symlinks in task outputs as links Output archives followed symbolic links and kept regular files only, and output globs listed regular files only, so a cache hit dropped every link a task's output held. Archive links as links, let output globs list them, and restore directory links as directory links on Windows. Bump the cache schema so entries archived without their links are not replayed. --- CHANGELOG.md | 1 + crates/vt/src/session/cache/archive.rs | 128 ++++++++++++++++++++++++- crates/vt/src/session/cache/mod.rs | 2 +- crates/vt/src/session/execute/glob.rs | 65 +++++++++++-- 4 files changed, 183 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26e2ea8f4..662a0954c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +- **Fixed** A cache hit restores the symbolic links a task's `output` globs match as links, instead of dropping them, so a replayed output that links a directory, such as a bundler's `node_modules/` link, is complete ([#???](https://github.com/voidzero-dev/vite-task/pull/???)). - **Perf** When tasks miss the cache, `vp run` no longer waits for the disk after saving each task's result, so the run finishes sooner. If the computer loses power or the operating system crashes during `vp run`, the cache entries that the run saved can be lost, and those tasks run again next time instead of hitting the cache ([#871](https://github.com/voidzero-dev/vite-task/pull/871)). - **Fixed** Running `vp run` in a package that sits inside a workspace folder but isn't listed in its `pnpm-workspace.yaml` `packages` or `package.json` `workspaces` now runs that package's own tasks, instead of treating the enclosing workspace as its root. When both configurations exist, `pnpm-workspace.yaml` takes precedence ([#859](https://github.com/voidzero-dev/vite-task/pull/859)). - **Fixed** A mistyped or misplaced setting in a task now gets an error that names it. For example, `cache: { outputs: [...] }` now fails with an `unknown field` error that names `outputs` and lists the accepted settings, instead of `data did not match any variant of untagged enum UserTaskDefinition` ([vite-plus#2635](https://github.com/voidzero-dev/vite-plus/issues/2635), [#815](https://github.com/voidzero-dev/vite-task/pull/815)). diff --git a/crates/vt/src/session/cache/archive.rs b/crates/vt/src/session/cache/archive.rs index dc110e29c..47512af8b 100644 --- a/crates/vt/src/session/cache/archive.rs +++ b/crates/vt/src/session/cache/archive.rs @@ -7,10 +7,13 @@ use std::{ use vt_path::{AbsolutePath, RelativePathBuf}; -/// Create a tar.zst archive from workspace-relative output file paths. +/// Create a tar.zst archive from workspace-relative output paths. /// -/// Files that no longer exist are silently skipped (the task may delete -/// temporary files during execution). +/// A regular file is stored with its contents. A symbolic link is stored as a +/// link with its target, never followed, so a link to a directory or a +/// dangling link round-trips as the task left it. Paths that no longer exist +/// are silently skipped (the task may delete temporary files during +/// execution). /// /// # Errors /// @@ -28,7 +31,7 @@ pub fn create_output_archive( let abs_path = workspace_root.join(rel_path); // Skip files that no longer exist (task may delete temp files between // glob walk and archiving). Any other error is propagated. - let metadata = match std::fs::metadata(abs_path.as_path()) { + let metadata = match std::fs::symlink_metadata(abs_path.as_path()) { Ok(m) => m, Err(err) if err.kind() == io::ErrorKind::NotFound => continue, Err(err) => return Err(err.into()), @@ -39,6 +42,13 @@ pub fn create_output_archive( header.set_metadata(&metadata); header.set_cksum(); builder.append_data(&mut header, rel_path.as_str(), &mut file)?; + } else if metadata.file_type().is_symlink() { + let target = std::fs::read_link(abs_path.as_path())?; + let mut header = tar::Header::new_gnu(); + header.set_metadata(&metadata); + header.set_entry_type(tar::EntryType::Symlink); + header.set_size(0); + builder.append_link(&mut header, rel_path.as_str(), target)?; } } @@ -46,7 +56,8 @@ pub fn create_output_archive( Ok(()) } -/// Extract a tar.zst archive, restoring files relative to workspace root. +/// Extract a tar.zst archive, restoring files and symbolic links relative to +/// workspace root. /// /// Parent directories are created automatically. Existing files are overwritten. /// @@ -61,7 +72,43 @@ pub fn extract_output_archive( let decoder = zstd::Decoder::new(file)?; let mut archive = tar::Archive::new(decoder); + #[cfg(not(windows))] archive.unpack(workspace_root.as_path())?; + #[cfg(windows)] + unpack_with_directory_links(&mut archive, workspace_root)?; + Ok(()) +} + +/// [`tar::Archive::unpack`], except that a restored link whose target is a +/// directory becomes a directory link: `tar` creates every link with +/// `symlink_file` on Windows, and Windows doesn't follow a file link to a +/// directory. Links are fixed up once every entry is unpacked, so their targets +/// exist when they are checked. +#[cfg(windows)] +fn unpack_with_directory_links( + archive: &mut tar::Archive, + workspace_root: &AbsolutePath, +) -> anyhow::Result<()> { + // As `tar::Archive::unpack` does: the canonical root is a verbatim (`\\?\`) + // path, which lifts Windows' path length limit. + let root = std::fs::canonicalize(workspace_root.as_path()) + .unwrap_or_else(|_| workspace_root.as_path().to_path_buf()); + let mut links = Vec::new(); + for entry in archive.entries()? { + let mut entry = entry?; + let is_link = entry.header().entry_type().is_symlink(); + let link = root.join(entry.path()?); + if entry.unpack_in(&root)? && is_link { + links.push(link); + } + } + for link in links { + let target = std::fs::read_link(&link)?; + if link.parent().is_some_and(|parent| parent.join(&target).is_dir()) { + std::fs::remove_file(&link)?; + std::os::windows::fs::symlink_dir(&target, &link)?; + } + } Ok(()) } @@ -84,6 +131,8 @@ pub fn check_output_archive(reader: impl Read) -> io::Result<()> { #[cfg(test)] mod tests { + use std::fs; + use vt_path::AbsolutePathBuf; use super::*; @@ -103,4 +152,73 @@ mod tests { assert!(check_output_archive(corrupt).is_err()); } } + + fn symlink(target: &str, link: &AbsolutePath, directory: bool) { + #[cfg(unix)] + { + let _ = directory; + std::os::unix::fs::symlink(target, link.as_path()).unwrap(); + } + #[cfg(windows)] + if directory { + std::os::windows::fs::symlink_dir(target, link.as_path()).unwrap(); + } else { + std::os::windows::fs::symlink_file(target, link.as_path()).unwrap(); + } + } + + #[test] + fn symlinks_round_trip_as_links() { + let tmp = tempfile::TempDir::new().unwrap(); + let dir = AbsolutePathBuf::new(tmp.path().to_path_buf()).unwrap(); + fs::create_dir_all(dir.join("dist/pkg")).unwrap(); + fs::write(dir.join("dist/pkg/index.js"), "export default 42;").unwrap(); + let links = [ + ("file.js", "pkg/index.js", false), + ("dependency", "pkg", true), + ("dangling", "missing", false), + ]; + for (name, target, directory) in links { + symlink(target, &dir.join("dist").join(name), directory); + } + let files = ["dist/pkg/index.js", "dist/file.js", "dist/dependency", "dist/dangling"] + .map(|path| RelativePathBuf::new(path).unwrap()); + let archive_path = dir.join("output.tar.zst"); + create_output_archive(&dir, &files, &archive_path).unwrap(); + + fs::remove_dir_all(dir.join("dist")).unwrap(); + extract_output_archive(&dir, &archive_path).unwrap(); + for (name, target, _) in links { + let restored = fs::read_link(dir.join("dist").join(name)).unwrap(); + assert_eq!( + RelativePathBuf::new(restored).unwrap(), + RelativePathBuf::new(target).unwrap() + ); + } + assert_eq!(fs::read_to_string(dir.join("dist/file.js")).unwrap(), "export default 42;"); + assert_eq!( + fs::read_to_string(dir.join("dist/dependency/index.js")).unwrap(), + "export default 42;" + ); + assert!(!dir.join("dist/dangling").as_path().exists()); + } + + #[test] + fn restore_rejects_writing_through_a_link_outside_the_workspace() { + let tmp = tempfile::TempDir::new().unwrap(); + let dir = AbsolutePathBuf::new(tmp.path().to_path_buf()).unwrap(); + let outside = tempfile::TempDir::new().unwrap(); + let sentinel = outside.path().join("sentinel.txt"); + fs::write(&sentinel, "archived").unwrap(); + symlink(outside.path().to_str().unwrap(), &dir.join("link"), true); + let files = ["link", "link/sentinel.txt"].map(|path| RelativePathBuf::new(path).unwrap()); + let archive_path = dir.join("output.tar.zst"); + create_output_archive(&dir, &files, &archive_path).unwrap(); + fs::write(&sentinel, "must remain unchanged").unwrap(); + + let restored = dir.join("restored"); + fs::create_dir_all(&restored).unwrap(); + assert!(extract_output_archive(&restored, &archive_path).is_err()); + assert_eq!(fs::read_to_string(&sentinel).unwrap(), "must remain unchanged"); + } } diff --git a/crates/vt/src/session/cache/mod.rs b/crates/vt/src/session/cache/mod.rs index ab6c806f9..30536eb9d 100644 --- a/crates/vt/src/session/cache/mod.rs +++ b/crates/vt/src/session/cache/mod.rs @@ -276,7 +276,7 @@ pub fn split_path(path: &str) -> (Option<&str>, &str) { /// its own cache warm across branch switches, and a cache from a different /// version is simply ignored (it lives in a directory this build never looks /// at) rather than aborting the run. Bumping the version starts a fresh cache. -const CACHE_SCHEMA_VERSION: u32 = 18; +const CACHE_SCHEMA_VERSION: u32 = 19; /// Name of the per-version subdirectory (e.g. `v14`) under the task-cache /// directory that holds the database and output archives for the current diff --git a/crates/vt/src/session/execute/glob.rs b/crates/vt/src/session/execute/glob.rs index 23808b422..1afb803e2 100644 --- a/crates/vt/src/session/execute/glob.rs +++ b/crates/vt/src/session/execute/glob.rs @@ -15,7 +15,7 @@ use wax::{ }; /// Walk positive globs (rooted at `root`), filtering with negative globs, and -/// call `for_each_file` with the absolute path of each file entry. Stripping +/// call `for_each_file` with the absolute path and type of each file or symlink. Stripping /// the root prefix (if needed) is the caller's responsibility. /// /// Walk errors for non-existent directories are skipped gracefully. Returning @@ -25,7 +25,7 @@ fn walk_glob_files( root: &AbsolutePath, positive_globs: &std::collections::BTreeSet, negative_globs: &std::collections::BTreeSet, - mut for_each_file: impl FnMut(AbsolutePathBuf) -> anyhow::Result>, + mut for_each_file: impl FnMut(AbsolutePathBuf, std::fs::FileType) -> anyhow::Result>, ) -> anyhow::Result<()> { if positive_globs.is_empty() { return Ok(()); @@ -52,13 +52,14 @@ fn walk_glob_files( return Err(io_err.into()); } }; - if !entry.file_type().is_file() { + let file_type = entry.file_type(); + if !file_type.is_file() && !file_type.is_symlink() { continue; } let Some(absolute) = AbsolutePathBuf::new(entry.path().to_path_buf()) else { continue; // wax can hand back non-absolute paths in some cases }; - if for_each_file(absolute)?.is_break() { + if for_each_file(absolute, file_type)?.is_break() { return Ok(()); } } @@ -77,7 +78,10 @@ pub fn compute_globbed_inputs( negative_globs: &std::collections::BTreeSet, ) -> anyhow::Result> { let mut result = BTreeMap::new(); - walk_glob_files(root, positive_globs, negative_globs, |absolute| { + walk_glob_files(root, positive_globs, negative_globs, |absolute, file_type| { + if !file_type.is_file() { + return Ok(ControlFlow::Continue(())); + } let Some(relative) = strip_root(root, &absolute)? else { return Ok(ControlFlow::Continue(())); // outside root }; @@ -98,7 +102,7 @@ pub fn compute_globbed_inputs( Ok(result) } -/// Collect file paths matching positive globs, filtered by negative globs. +/// Collect file and symlink paths matching positive globs, filtered by negative globs. /// /// Like [`compute_globbed_inputs`] but only collects paths (no hashing). /// Used for determining which output files to archive. @@ -108,7 +112,7 @@ pub fn collect_glob_paths( negative_globs: &std::collections::BTreeSet, ) -> anyhow::Result> { let mut result = Vec::new(); - walk_glob_files(root, positive_globs, negative_globs, |absolute| { + walk_glob_files(root, positive_globs, negative_globs, |absolute, _| { if let Some(relative) = strip_root(root, &absolute)? { result.push(relative); } @@ -445,4 +449,51 @@ mod tests { // BTreeMap naturally deduplicates by key assert_eq!(result.len(), 3); } + + fn symlink(target: &str, link: &AbsolutePath, directory: bool) { + #[cfg(unix)] + { + let _ = directory; + std::os::unix::fs::symlink(target, link.as_path()).unwrap(); + } + #[cfg(windows)] + if directory { + std::os::windows::fs::symlink_dir(target, link.as_path()).unwrap(); + } else { + std::os::windows::fs::symlink_file(target, link.as_path()).unwrap(); + } + } + + #[test] + fn output_paths_include_symlinks_without_traversing_them() { + let temp_dir = TempDir::new().unwrap(); + let workspace = AbsolutePathBuf::new(temp_dir.path().to_path_buf()).unwrap(); + fs::create_dir_all(workspace.join("dist")).unwrap(); + fs::create_dir_all(workspace.join("outside")).unwrap(); + fs::write(workspace.join("outside/index.js"), "export default 42;").unwrap(); + fs::write(workspace.join("dist/regular.js"), "export default 7;").unwrap(); + for (name, target, directory) in [ + ("file.js", "../outside/index.js", false), + ("dependency", "../outside", true), + ("dangling", "missing", false), + ("excluded", "../outside", true), + ] { + symlink(target, &workspace.join("dist").join(name), directory); + } + let positive: std::collections::BTreeSet = std::iter::once("dist/**".into()).collect(); + let negative: std::collections::BTreeSet = + std::iter::once("dist/excluded".into()).collect(); + + let outputs = collect_glob_paths(&workspace, &positive, &negative).unwrap(); + assert_eq!( + outputs.iter().map(|path| path.as_str()).collect::>(), + ["dist/dangling", "dist/dependency", "dist/file.js", "dist/regular.js"] + ); + // Inputs are unchanged: a link is not a globbed input. + let inputs = compute_globbed_inputs(&workspace, &positive, &negative).unwrap(); + assert_eq!( + inputs.keys().map(|path| path.as_str()).collect::>(), + ["dist/regular.js"] + ); + } } From dfa2fea298d459ab8c73c3288efc392837a5864b Mon Sep 17 00:00:00 2001 From: Jason Kuhrt Date: Sat, 24 Oct 2026 19:23:42 -0400 Subject: [PATCH 2/2] docs(changelog): link #885 --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 662a0954c..f8db0940f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -- **Fixed** A cache hit restores the symbolic links a task's `output` globs match as links, instead of dropping them, so a replayed output that links a directory, such as a bundler's `node_modules/` link, is complete ([#???](https://github.com/voidzero-dev/vite-task/pull/???)). +- **Fixed** A cache hit restores the symbolic links a task's `output` globs match as links, instead of dropping them, so a replayed output that links a directory, such as a bundler's `node_modules/` link, is complete ([#885](https://github.com/voidzero-dev/vite-task/pull/885)). - **Perf** When tasks miss the cache, `vp run` no longer waits for the disk after saving each task's result, so the run finishes sooner. If the computer loses power or the operating system crashes during `vp run`, the cache entries that the run saved can be lost, and those tasks run again next time instead of hitting the cache ([#871](https://github.com/voidzero-dev/vite-task/pull/871)). - **Fixed** Running `vp run` in a package that sits inside a workspace folder but isn't listed in its `pnpm-workspace.yaml` `packages` or `package.json` `workspaces` now runs that package's own tasks, instead of treating the enclosing workspace as its root. When both configurations exist, `pnpm-workspace.yaml` takes precedence ([#859](https://github.com/voidzero-dev/vite-task/pull/859)). - **Fixed** A mistyped or misplaced setting in a task now gets an error that names it. For example, `cache: { outputs: [...] }` now fails with an `unknown field` error that names `outputs` and lists the accepted settings, instead of `data did not match any variant of untagged enum UserTaskDefinition` ([vite-plus#2635](https://github.com/voidzero-dev/vite-plus/issues/2635), [#815](https://github.com/voidzero-dev/vite-task/pull/815)).