Turbopack: Fix missing canonicalization of paths and always use verbatim paths internally for Windows#95668
Merged
Merged
Conversation
Contributor
Stats from current PRWarning No stats were collected for Webpack because its stats job did not complete (it failed, was cancelled, or timed out). The results below only cover the bundlers that finished. ✅ No significant changes detected📊 All Metrics📖 Metrics GlossaryDev Server Metrics:
Build Metrics:
Change Thresholds:
⚡ Dev Server
⚡ Production Builds
📦 Production Builds (Webpack) (Legacy)📦 Production Builds (Webpack)
📦 Bundle SizesBundle Sizes⚡ TurbopackClient Main Bundles
Server Middleware
Build DetailsBuild Manifests
🔄 Shared (bundler-independent)Runtimes
📎 Tarball URLCommit: 0f280ac |
Contributor
Tests PassedCommit: 0f280ac |
lukesandberg
approved these changes
Jul 11, 2026
bgw
force-pushed
the
bgw/canonicalize-root-path
branch
from
July 20, 2026 07:25
69305e4 to
082cad2
Compare
bgw
changed the title
bgw
force-pushed
the
bgw/fs-lock-arc-keys
branch
from
July 20, 2026 21:10
ad90db0 to
eeadc40
Compare
bgw
force-pushed
the
bgw/canonicalize-root-path
branch
from
July 20, 2026 21:10
082cad2 to
81793a4
Compare
bgw
force-pushed
the
bgw/fs-lock-arc-keys
branch
from
July 20, 2026 21:11
eeadc40 to
a756827
Compare
bgw
force-pushed
the
bgw/canonicalize-root-path
branch
2 times, most recently
from
July 20, 2026 22:15
a7ec075 to
58f0896
Compare
bgw
marked this pull request as ready for review
July 20, 2026 22:38
Member
Author
|
@lukesandberg this has turned into a very significant refactor of all of our windows path handling since your last review. |
lukesandberg
approved these changes
Jul 21, 2026
Contributor
|
unsubscribe |
bgw
force-pushed
the
bgw/canonicalize-root-path
branch
from
July 21, 2026 19:53
58f0896 to
0409c3a
Compare
…uctor call and always use verbatim paths internally for Windows
bgw
force-pushed
the
bgw/canonicalize-root-path
branch
from
July 21, 2026 20:21
0409c3a to
0f280ac
Compare
bgw
added a commit
that referenced
this pull request
Jul 21, 2026
…endencies (#95628) We also mitigate the security impact of e2e tests by running everything in VMs, and by not reading caches in release jobs, but we should still be using `minimumReleaseAge` in e2e tests. Ideally we'd also use lockfiles here too, and I'll work on that next, but that is likely a lot more complicated because we probably need some way to merge lockfiles, since we can't pin the next.js and react versions in every test, and we need a reasonable/efficient way to bulk-update all the lockfiles for tests that need it. Depends on #95668 for a bunch of windows path representation fixes that this ends up exposing in CI, because adding workspace files causes pnpm to change the absolute path representation used in the Windows junction points that it creates.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I was seeing failures on #95628 and went down this rabbit hole.
We were (incorrectly) not canonicalizing the
DiskFileSystemroot 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 addedpnpm-workspace.yamlfiles. This was just coincidence and indicative of much deeper problems.dunceisn'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.htmlThe 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_linkwas re-inventingtry_from_sys_pathbut badly. Just calltry_from_sys_path.turbopack/crates/turbo-tasks-fs/src/embed/file.rswas dead code.validate_path_length_innerwas incorrectly referring to verbatim paths as UNC paths. These are two different things.Canonicalize any absolute symlink targets outside of the
DiskFileSystemroot 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\blahpaths. 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 alongPathAwareapplication manifest. 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, howeverPath::joinwill 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 (used by 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, though the normal 260 character limit typically does not apply here.canonicalize: Corresponds to
std::fs::canonicalize. 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.