Benjamin Woodruff ‮

Turbopack: Fix missing canonicalization of paths and always use verbatim paths internally for Windows (#95668) I was seeing failures on https://github.com/vercel/next.js/pull/95628 and went down this rabbit hole. - We were (incorrectly) not canonicalizing the `DiskFileSystem` root dir. This happened to work out for us because pnpm also wasn't canonicalizing junction point targets, and so the root dir prefix matched. However after `#95628`, pnpm started canonicalizing junction point targets because we added `pnpm-workspace.yaml` files. This was just coincidence and indicative of much deeper problems. - `dunce` isn't a good fit for us. It picks a win32 or verbatim representation based on path length, which would mean that long paths inside of a short root path would never work because we'd never be able to strip the prefix, since we'd be looking for a win32 path prefix. Omnipath is better for what we want to do: https://docs.rs/omnipath/latest/omnipath/windows/trait.WinPathExt.html - The windows verbatim path format works with long paths (>260 characters) and behaves a lot closer to cross-platform unix paths, which is what we want. E.g. if somebody tries to import a JS module from a Windows 8.3 short path, it should fail. This will fail with verbatim paths (good!). This PR *always* uses the verbatim path format internally within `DiskFileSystem`. - `read_link` was re-inventing `try_from_sys_path` but badly. Just call `try_from_sys_path`. - `turbopack/crates/turbo-tasks-fs/src/embed/file.rs` was dead code. - `validate_path_length_inner` was incorrectly referring to verbatim paths as UNC paths. These are two different things. - Canonicalize any absolute symlink targets outside of the `DiskFileSystem` root path, resolving any symlinks they may have, normalizing case-insensitive paths, and handling Windows 8.3 short name format. ## Some Context About Windows Path Formats: - win32 paths: These are your normal user-friendly `C:\foo\blah` paths. When calling most filesystem APIs, these are limited to 260 characters unless the user has enabled long paths on the system, and avoiding the limit with these paths also requires a [`longPathAware` application manifest](https://learn.microsoft.com/en-us/windows/win32/sbscs/application-manifests#longPathAware). You can mix `/` and `\` in win32 paths, and Windows will normalize the path separator for you. - verbatim paths: This is a rust-specific name (used by the stdlib) for the "extended length" prefixed paths. These are paths starting with `\\?\`. These paths can be up to ~32,767 characters long. They do not support Windows 8.3 shortnames. They do not normalize between `/` and `\`, and do not support `.` or `..` components, however [`Path::join`](https://doc.rust-lang.org/std/path/struct.Path.html#method.join) will do this normalization if given a verbatim path. - Windows 8.3 shortnames: A legacy format where paths can be represented by 8 characters, followed by a tilde and a digit (or hash in some extreme cases where there are many collisions), followed by a 3 character file extension. NTFS supports this, [ReFS](https://learn.microsoft.com/en-us/windows-server/storage/refs/refs-overview) (used by [Windows Dev Drive](https://learn.microsoft.com/en-us/windows/dev-drive/)) does not. - UNC ("Universal Naming Convention"): This is a specific representation of paths used for remote files on network servers. - `file://` URIs: These must be formed from [win32 style paths](https://docs.rs/url/latest/url/struct.Url.html#method.from_file_path), though the normal 260 character limit typically does not apply here. - canonicalize: Corresponds to [`std::fs::canonicalize`](https://doc.rust-lang.org/stable/std/fs/fn.canonicalize.html). This involves actual fs operations (that we can't easily track), and recursively resolves symlinks. It will fail if the file path does not exist. On windows, this always returns a verbatim path. - wide format: Windows paths are utf-16. Generally we don't support paths that can't be encoded to utf-8, but we should try to fail as obviously as we can in these cases. We don't have good filesystem issue handling yet though. - case sensitivity: Generally paths are not case-sensitive on Windows or macOS, but are case-sensitive on most Linux filesystems. The exact definition of how case sensitivity works with unicode paths is highly platform and filesystem dependent. - junction points: The windows equivalent of symlinks (symlinks are also supported, but require developer mode to be turned on). These can contain an arbitrary path reference in any format (win32, verbatim, etc) with any case and with windows 8.3 short names. Junction points are always target absolute paths to directories.

Turbopack: Split up turbo-tasks-fs/src/lib.rs into smaller modules (#96030) This file was way too huge and was wasting a ton of tokens when trying to use it with agents. `turbo-tasks-fs/src/disk.rs` is still a little bigger than I'd like, but I think this is an overall improvement.

Turbopack: Split the read and write codepath data structures for symlinks (#97395) Another attempt at cleanup, similar to https://github.com/vercel/next.js/pull/96955. We need different information to write symlinks or junction points (most notably, if it's a directory or not) than we get when reading symlinks. We were trying to use the same data structure for both, but this lead us to always call `get_type()` as part of reading a link, which was just silly. This also meant that `write_link` had to handle `NotFound` and `Invalid`. `NotFound` is somewhat reasonable (just delete the file), but `Invalid` is awkward.

Turbopack: Improve how DiskWatcher is configured and fix polling watcher bugs (#96440) - Adds a `DiskWatcherConfig` object to centralize the recursive mode, polling, and invalidation reason options. - **Polling Fix:** Use the non-recursive mode if the polling watcher is selected, ignoring the OS/platform. - **Polling Fix:** Treat mtime events as data update events if the polling watcher is selected (this is what https://github.com/vercel/next.js/pull/96288 was doing, but this PR only does it if polling is enabled) - Only treat the `MetadataKind::Any` event as a data change on macOS. Fixes #80665 Closes #96288

Turbopack: More aggressively debounce filesystem watch events if we detected changes to node_modules (#96116) Previously, we were debouncing update by sleeping 1ms at a time on macos and windows, and 10ms at a time on Linux. During a slow `pnpm install`, or a `git checkout`, this could cause us to do a bunch of extra throwaway work. Changes: - Increase the debounce interval to a consistent 10ms everywhere. This should still be small enough that it's not noticable on macos or windows. - If an event touches `node_modules`, there's a good chance that a package manager is running and many other files will be modified, so extend the batch deadline by 200ms instead of 10ms. - Because there's a chance that the batch deadline could get extended indefinitely (this was always possible, just more likely now) include a compilation event that gets logged after 5 seconds.

Turbopack: Allow DiskWatcher to use a mocked DiskFileSystem, add a small unit test (#96353) Attempting to clean up our testing story here so that we have a way to test the watcher in isolation, ahead of trying to tackle https://github.com/vercel/next.js/pull/96288

[ci] split up large cache-components-errors tests (#95623) See https://github.com/vercel/next.js/pull/95553 for an explanation of why we want to split these up. TL;DR it makes CI faster and saves us some money. In CI we have to run tests at the suite/file granularity, so if a test suite is too long it limits parallelism and if it has to be retried, we have to retry the full suite.

Turbopack: Improve file existence error handling in realpath_with_links and in module resolution (#97717) Fixes some regressions from https://github.com/vercel/next.js/pull/97395. Human summary of changes: - Plainly return `NotFound` for `realpath_with_link` whenever any step in the resolution algorithm fails, matching the behavior the libc uses. - When probing in the module resolution algorithm, explicitly check for `NotFound`. - Fully remove the extremely non-standard and confusing behavior of trying to return the last processed path from `realpath_with_links` if a failure occurs. - Modify `realpath` to return a `RealPathError` instead of just an `anyhow::Error`, so that callers that care about the actual error kind but not all of the intermediate links can still use it instead of `realpath_with_links`. - Rename `RealPathResultError` to `RealPathError`, and make it actually implement `Display` so that it's easier to coerce to an `anyhow::Error`. This requires including a tiny bit more data on this object, but in the hot path it's just one extra clone of a `FileSystemPath`. Human discussion: https://vercel.slack.com/archives/C09R44U5HQW/p1787322213695299 AI slop summary of why #97395 caused a regression: > The regression came from newly added missing-target detection in b2e835ded6. > > Before that commit, realpath_with_links: > > 1. Resolved directory symlinks. > 2. Continued resolving the remaining path beneath the resolved directory. > 3. If that remaining path did not exist, returned the resolved path successfully. > 4. Let the caller’s subsequent existence check report that the candidate was absent. > > The commit changed read_link so it no longer checked the symlink target’s type. To detect dangling symlinks, realpath_with_links added this condition: > > ``` > if (followed_link || !symlinks.is_empty()) > && entry_type == FileSystemEntryType::NotFound > { > // report a missing symlink target > } > ``` > > The problem is that !symlinks.is_empty() does not mean the missing path is a symlink target. It only means that some earlier path component was a symlink. > > For the reported path: > > `node_modules/@workflow/core/dist/serialization/package.json` > > resolution worked like this: > > 1. Resolve node_modules/@workflow/core, a valid directory symlink. > 2. Continue into dist/serialization, which exists. > 3. Probe the optional dist/serialization/package.json, which does not exist. > 4. Because the earlier @workflow/core component was a symlink, symlinks was nonempty. > 5. The new condition incorrectly classified the missing package.json as a dangling symlink target. > 6. Turbopack aborted instead of treating package.json as an absent resolution candidate and continuing to serialization.js. > > So the regression was not caused by resolving the directory symlink incorrectly. It was caused by using “we traversed any symlink” as evidence that a later missing descendant was the target of a dangling symlink.

[ci] Split up large instant-validation tests (#95627) See https://github.com/vercel/next.js/pull/95553 for an explanation of why we want to split these up. TL;DR it makes CI faster and saves us some money. In CI we have to run tests at the suite/file granularity, so if a test suite is too long it limits parallelism and if it has to be retried, we have to retry the full suite.

Turbopack: Only extend the watcher's batch window for unfiltered events (#96186) If we get metadata events, or if we (on Linux with inotify) get file access events, these aren't useful, and we filter them, but we were unintentionally extending the deadline for the batch because of them.