diff --git a/CHANGELOG.md b/CHANGELOG.md index 26e2ea8f4..f8db0940f 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 ([#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)). 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"] + ); + } }