fix(cache): restore symlinks in task outputs as links

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/<package>` 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)).

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,14 +42,22 @@ 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)?;
         }
     }
 
     builder.finish()?;
     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<impl Read>,
+    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");
+    }
 }

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

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<Str>,
     negative_globs: &std::collections::BTreeSet<Str>,
-    mut for_each_file: impl FnMut(AbsolutePathBuf) -> anyhow::Result<ControlFlow<()>>,
+    mut for_each_file: impl FnMut(AbsolutePathBuf, std::fs::FileType) -> anyhow::Result<ControlFlow<()>>,
 ) -> 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<Str>,
 ) -> anyhow::Result<BTreeMap<RelativePathBuf, u64>> {
     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<Str>,
 ) -> anyhow::Result<Vec<RelativePathBuf>> {
     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<Str> = std::iter::once("dist/**".into()).collect();
+        let negative: std::collections::BTreeSet<Str> =
+            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::<Vec<_>>(),
+            ["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::<Vec<_>>(),
+            ["dist/regular.js"]
+        );
+    }
 }