From f5bdf1ba33bec69cb4bb4a794086ae0aed2cd042 Mon Sep 17 00:00:00 2001 From: Joseph Date: Thu, 20 Aug 2026 23:46:40 +0200 Subject: [PATCH 1/4] docs: document deploymentId build ID override and Pages Router skew in 16.2 (#97645) - Addressing: https://github.com/vercel/next.js/issues/97632 --- docs/01-app/02-guides/self-hosting.mdx | 2 ++ .../05-config/01-next-config-js/deploymentId.mdx | 11 +++++++---- .../05-config/01-next-config-js/generateBuildId.mdx | 2 ++ 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/docs/01-app/02-guides/self-hosting.mdx b/docs/01-app/02-guides/self-hosting.mdx index 2917c783472e..72e3db25a0ef 100644 --- a/docs/01-app/02-guides/self-hosting.mdx +++ b/docs/01-app/02-guides/self-hosting.mdx @@ -182,6 +182,8 @@ module.exports = { } ``` +> **Good to know:** When [`deploymentId`](/docs/app/api-reference/config/next-config-js/deploymentId) is set, Next.js uses a constant build ID and `generateBuildId` has no effect. [Version skew](#version-skew) is detected from the deployment ID instead. + ## Multi-Server Deployments When running Next.js across multiple server instances (for example, containers behind a load balancer), there are additional considerations to ensure consistent behavior. diff --git a/docs/01-app/03-api-reference/05-config/01-next-config-js/deploymentId.mdx b/docs/01-app/03-api-reference/05-config/01-next-config-js/deploymentId.mdx index 336065ec2657..70718b19911d 100644 --- a/docs/01-app/03-api-reference/05-config/01-next-config-js/deploymentId.mdx +++ b/docs/01-app/03-api-reference/05-config/01-next-config-js/deploymentId.mdx @@ -68,12 +68,15 @@ module.exports = { } ``` +A per-deployment value only avoids skew if requests are also routed by deployment. Next.js does not route on `?dpl=`, so that routing comes from your host or CDN. Without it, clients that reach an instance from another deployment during a rollout will reload rather than navigate. + ## Version History -| Version | Changes | -| ---------- | ----------------------------------------------------- | -| `v14.1.4` | `deploymentId` stabilized as top-level config option. | -| `v13.4.10` | `experimental.deploymentId` introduced. | +| Version | Changes | +| ---------- | --------------------------------------------------------------------------------------------------------------------------------------------- | +| `v16.2.0` | Pages Router detects version skew from the response header rather than the build ID, and the build ID is constant when `deploymentId` is set. | +| `v14.1.4` | `deploymentId` stabilized as top-level config option. | +| `v13.4.10` | `experimental.deploymentId` introduced. | ## Related diff --git a/docs/01-app/03-api-reference/05-config/01-next-config-js/generateBuildId.mdx b/docs/01-app/03-api-reference/05-config/01-next-config-js/generateBuildId.mdx index 88f47dbb21e4..226ff6005b44 100644 --- a/docs/01-app/03-api-reference/05-config/01-next-config-js/generateBuildId.mdx +++ b/docs/01-app/03-api-reference/05-config/01-next-config-js/generateBuildId.mdx @@ -17,3 +17,5 @@ module.exports = { }, } ``` + +> **Good to know:** When [`deploymentId`](/docs/app/api-reference/config/next-config-js/deploymentId) is set, Next.js uses a constant build ID and `generateBuildId` has no effect. [Version skew](/docs/app/guides/self-hosting#version-skew) is detected from the deployment ID instead. From 38c38991485c290a4dd79b233d523f3b256d2e3e Mon Sep 17 00:00:00 2001 From: "next-js-bot[bot]" <279046576+next-js-bot[bot]@users.noreply.github.com> Date: Thu, 20 Aug 2026 23:23:05 +0000 Subject: [PATCH 2/4] v16.3.1-canary.26 --- lerna.json | 2 +- packages/create-next-app/package.json | 2 +- packages/devlow-bench/package.json | 2 +- packages/eslint-config-next/package.json | 4 ++-- packages/eslint-plugin-internal/package.json | 2 +- packages/eslint-plugin-next/package.json | 2 +- packages/font/package.json | 2 +- packages/next-bundle-analyzer/package.json | 2 +- packages/next-codemod/package.json | 2 +- packages/next-env/package.json | 2 +- packages/next-mdx/package.json | 2 +- packages/next-playwright/package.json | 2 +- packages/next-plugin-storybook/package.json | 2 +- packages/next-polyfill-module/package.json | 2 +- packages/next-polyfill-nomodule/package.json | 2 +- packages/next-routing/package.json | 2 +- packages/next-rspack/package.json | 2 +- packages/next-swc/package.json | 2 +- packages/next/package.json | 14 +++++++------- packages/react-refresh-utils/package.json | 2 +- packages/third-parties/package.json | 4 ++-- pnpm-lock.yaml | 16 ++++++++-------- 22 files changed, 37 insertions(+), 37 deletions(-) diff --git a/lerna.json b/lerna.json index 0eb4a94fa3fb..11b86399f1b5 100644 --- a/lerna.json +++ b/lerna.json @@ -15,5 +15,5 @@ "registry": "https://registry.npmjs.org/" } }, - "version": "16.3.1-canary.25" + "version": "16.3.1-canary.26" } \ No newline at end of file diff --git a/packages/create-next-app/package.json b/packages/create-next-app/package.json index 211159a14744..869ce76e9229 100644 --- a/packages/create-next-app/package.json +++ b/packages/create-next-app/package.json @@ -1,6 +1,6 @@ { "name": "create-next-app", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "keywords": [ "react", "next", diff --git a/packages/devlow-bench/package.json b/packages/devlow-bench/package.json index b5f69ed0ec65..dd2d9805f14b 100644 --- a/packages/devlow-bench/package.json +++ b/packages/devlow-bench/package.json @@ -1,7 +1,7 @@ { "name": "@vercel/devlow-bench", "private": true, - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "description": "Benchmarking tool for the developer workflow", "repository": { "type": "git", diff --git a/packages/eslint-config-next/package.json b/packages/eslint-config-next/package.json index 6b2a3f048be1..6a7e604efc7a 100644 --- a/packages/eslint-config-next/package.json +++ b/packages/eslint-config-next/package.json @@ -1,6 +1,6 @@ { "name": "eslint-config-next", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "description": "ESLint configuration used by Next.js.", "license": "MIT", "repository": { @@ -12,7 +12,7 @@ "dist" ], "dependencies": { - "@next/eslint-plugin-next": "16.3.1-canary.25", + "@next/eslint-plugin-next": "16.3.1-canary.26", "eslint-import-resolver-node": "^0.3.6", "eslint-import-resolver-typescript": "^3.5.2", "eslint-plugin-import": "^2.32.0", diff --git a/packages/eslint-plugin-internal/package.json b/packages/eslint-plugin-internal/package.json index c12f6d6943b3..c15546e2da98 100644 --- a/packages/eslint-plugin-internal/package.json +++ b/packages/eslint-plugin-internal/package.json @@ -1,7 +1,7 @@ { "name": "@next/eslint-plugin-internal", "private": true, - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "description": "ESLint plugin for working on Next.js.", "exports": { ".": "./src/eslint-plugin-internal.js" diff --git a/packages/eslint-plugin-next/package.json b/packages/eslint-plugin-next/package.json index 89514a1db2be..0960d029f5bc 100644 --- a/packages/eslint-plugin-next/package.json +++ b/packages/eslint-plugin-next/package.json @@ -1,6 +1,6 @@ { "name": "@next/eslint-plugin-next", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "description": "ESLint plugin for Next.js.", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/packages/font/package.json b/packages/font/package.json index a44e1238775b..7abb3f473558 100644 --- a/packages/font/package.json +++ b/packages/font/package.json @@ -1,7 +1,7 @@ { "name": "@next/font", "private": true, - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "repository": { "url": "vercel/next.js", "directory": "packages/font" diff --git a/packages/next-bundle-analyzer/package.json b/packages/next-bundle-analyzer/package.json index cee12c930732..5dc4875b6f87 100644 --- a/packages/next-bundle-analyzer/package.json +++ b/packages/next-bundle-analyzer/package.json @@ -1,6 +1,6 @@ { "name": "@next/bundle-analyzer", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "main": "index.js", "types": "index.d.ts", "license": "MIT", diff --git a/packages/next-codemod/package.json b/packages/next-codemod/package.json index 288baf0741e2..4c97378be04f 100644 --- a/packages/next-codemod/package.json +++ b/packages/next-codemod/package.json @@ -1,6 +1,6 @@ { "name": "@next/codemod", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "license": "MIT", "repository": { "type": "git", diff --git a/packages/next-env/package.json b/packages/next-env/package.json index e2b0cc768186..3cfd563db0fc 100644 --- a/packages/next-env/package.json +++ b/packages/next-env/package.json @@ -1,6 +1,6 @@ { "name": "@next/env", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "keywords": [ "react", "next", diff --git a/packages/next-mdx/package.json b/packages/next-mdx/package.json index 7f65a3606c34..eac8d15d5070 100644 --- a/packages/next-mdx/package.json +++ b/packages/next-mdx/package.json @@ -1,6 +1,6 @@ { "name": "@next/mdx", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "main": "index.js", "license": "MIT", "repository": { diff --git a/packages/next-playwright/package.json b/packages/next-playwright/package.json index 3fc78617b328..57514beb1f05 100644 --- a/packages/next-playwright/package.json +++ b/packages/next-playwright/package.json @@ -1,6 +1,6 @@ { "name": "@next/playwright", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "repository": { "url": "vercel/next.js", "directory": "packages/next-playwright" diff --git a/packages/next-plugin-storybook/package.json b/packages/next-plugin-storybook/package.json index ff1d693bca1f..68ba66c5e341 100644 --- a/packages/next-plugin-storybook/package.json +++ b/packages/next-plugin-storybook/package.json @@ -1,6 +1,6 @@ { "name": "@next/plugin-storybook", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "repository": { "url": "vercel/next.js", "directory": "packages/next-plugin-storybook" diff --git a/packages/next-polyfill-module/package.json b/packages/next-polyfill-module/package.json index fa81f72ba73e..c09840f99d73 100644 --- a/packages/next-polyfill-module/package.json +++ b/packages/next-polyfill-module/package.json @@ -1,6 +1,6 @@ { "name": "@next/polyfill-module", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "description": "A standard library polyfill for ES Modules supporting browsers (Edge 16+, Firefox 60+, Chrome 61+, Safari 10.1+)", "main": "dist/polyfill-module.js", "license": "MIT", diff --git a/packages/next-polyfill-nomodule/package.json b/packages/next-polyfill-nomodule/package.json index ffb829c328a5..d7629d67a3d7 100644 --- a/packages/next-polyfill-nomodule/package.json +++ b/packages/next-polyfill-nomodule/package.json @@ -1,6 +1,6 @@ { "name": "@next/polyfill-nomodule", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "description": "A polyfill for non-dead, nomodule browsers.", "main": "dist/polyfill-nomodule.js", "license": "MIT", diff --git a/packages/next-routing/package.json b/packages/next-routing/package.json index 475d144aef75..d1dbef2e96b7 100644 --- a/packages/next-routing/package.json +++ b/packages/next-routing/package.json @@ -1,6 +1,6 @@ { "name": "@next/routing", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "keywords": [ "react", "next", diff --git a/packages/next-rspack/package.json b/packages/next-rspack/package.json index d2416ee24ae9..6b4cf0021831 100644 --- a/packages/next-rspack/package.json +++ b/packages/next-rspack/package.json @@ -1,6 +1,6 @@ { "name": "next-rspack", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "repository": { "url": "vercel/next.js", "directory": "packages/next-rspack" diff --git a/packages/next-swc/package.json b/packages/next-swc/package.json index 89af2c0fa961..4474bdc1285d 100644 --- a/packages/next-swc/package.json +++ b/packages/next-swc/package.json @@ -1,6 +1,6 @@ { "name": "@next/swc", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "private": true, "files": [ "native/" diff --git a/packages/next/package.json b/packages/next/package.json index bcefc20cfde7..02846e46e059 100644 --- a/packages/next/package.json +++ b/packages/next/package.json @@ -1,6 +1,6 @@ { "name": "next", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "description": "The React Framework", "main": "./dist/server/next.js", "license": "MIT", @@ -100,7 +100,7 @@ ] }, "dependencies": { - "@next/env": "16.3.1-canary.25", + "@next/env": "16.3.1-canary.26", "@swc/helpers": "0.5.23", "baseline-browser-mapping": "^2.9.19", "caniuse-lite": "^1.0.30001579", @@ -164,11 +164,11 @@ "@modelcontextprotocol/sdk": "1.18.1", "@mswjs/interceptors": "0.42.0", "@napi-rs/triples": "1.2.0", - "@next/font": "16.3.1-canary.25", - "@next/polyfill-module": "16.3.1-canary.25", - "@next/polyfill-nomodule": "16.3.1-canary.25", - "@next/react-refresh-utils": "16.3.1-canary.25", - "@next/swc": "16.3.1-canary.25", + "@next/font": "16.3.1-canary.26", + "@next/polyfill-module": "16.3.1-canary.26", + "@next/polyfill-nomodule": "16.3.1-canary.26", + "@next/react-refresh-utils": "16.3.1-canary.26", + "@next/swc": "16.3.1-canary.26", "@opentelemetry/api": "1.6.0", "@playwright/test": "1.61.0", "@rspack/core": "1.6.7", diff --git a/packages/react-refresh-utils/package.json b/packages/react-refresh-utils/package.json index ffebf603ac57..db5970d0938c 100644 --- a/packages/react-refresh-utils/package.json +++ b/packages/react-refresh-utils/package.json @@ -1,6 +1,6 @@ { "name": "@next/react-refresh-utils", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "description": "An experimental package providing utilities for React Refresh.", "repository": { "url": "vercel/next.js", diff --git a/packages/third-parties/package.json b/packages/third-parties/package.json index 004af18f1a01..a5f9b4ee844c 100644 --- a/packages/third-parties/package.json +++ b/packages/third-parties/package.json @@ -1,6 +1,6 @@ { "name": "@next/third-parties", - "version": "16.3.1-canary.25", + "version": "16.3.1-canary.26", "repository": { "url": "vercel/next.js", "directory": "packages/third-parties" @@ -26,7 +26,7 @@ "third-party-capital": "1.0.20" }, "devDependencies": { - "next": "16.3.1-canary.25", + "next": "16.3.1-canary.26", "outdent": "0.8.0", "prettier": "2.5.1", "typescript": "6.0.2" diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 94fae70e4d61..a483fe7d014f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1024,7 +1024,7 @@ importers: packages/eslint-config-next: dependencies: '@next/eslint-plugin-next': - specifier: 16.3.1-canary.25 + specifier: 16.3.1-canary.26 version: link:../eslint-plugin-next eslint: specifier: '>=9.0.0' @@ -1107,7 +1107,7 @@ importers: packages/next: dependencies: '@next/env': - specifier: 16.3.1-canary.25 + specifier: 16.3.1-canary.26 version: link:../next-env '@swc/helpers': specifier: 0.5.23 @@ -1228,19 +1228,19 @@ importers: specifier: 1.2.0 version: 1.2.0 '@next/font': - specifier: 16.3.1-canary.25 + specifier: 16.3.1-canary.26 version: link:../font '@next/polyfill-module': - specifier: 16.3.1-canary.25 + specifier: 16.3.1-canary.26 version: link:../next-polyfill-module '@next/polyfill-nomodule': - specifier: 16.3.1-canary.25 + specifier: 16.3.1-canary.26 version: link:../next-polyfill-nomodule '@next/react-refresh-utils': - specifier: 16.3.1-canary.25 + specifier: 16.3.1-canary.26 version: link:../react-refresh-utils '@next/swc': - specifier: 16.3.1-canary.25 + specifier: 16.3.1-canary.26 version: link:../next-swc '@opentelemetry/api': specifier: 1.6.0 @@ -1983,7 +1983,7 @@ importers: version: 1.0.20 devDependencies: next: - specifier: 16.3.1-canary.25 + specifier: 16.3.1-canary.26 version: link:../next outdent: specifier: 0.8.0 From ee6909213f50e37c9291cd70c17f3bb16c31fecc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjamin=20Woodruff=20=E2=80=AE?= Date: Thu, 20 Aug 2026 16:59:56 -0700 Subject: [PATCH 3/4] Turbopack: Show last modified file when waiting for the filesystem to settle (#97648) This is a follow-up to https://github.com/vercel/next.js/pull/96116 If we're waiting a long time for your filesystem to settle, we should show you the last modified path, so at least you can have an idea of why this is happening. --- .../fs-settling-event.test.ts | 1 + .../src/watcher/batch_schedule.rs | 26 ++++- .../crates/turbo-tasks-fs/src/watcher/mod.rs | 103 +++++++++++------- 3 files changed, 86 insertions(+), 44 deletions(-) diff --git a/test/development/fs-settling-event/fs-settling-event.test.ts b/test/development/fs-settling-event/fs-settling-event.test.ts index 1b9ade87ad46..96285c880f7f 100644 --- a/test/development/fs-settling-event/fs-settling-event.test.ts +++ b/test/development/fs-settling-event/fs-settling-event.test.ts @@ -33,6 +33,7 @@ import path from 'path' () => { const output = stripAnsi(next.cliOutput.slice(outputIndex)) expect(output).toContain('waiting for the filesystem to settle') + expect(output).toContain(pkgFile) }, // The event fires after ~5s; allow a generous window to avoid flakes. 15000, diff --git a/turbopack/crates/turbo-tasks-fs/src/watcher/batch_schedule.rs b/turbopack/crates/turbo-tasks-fs/src/watcher/batch_schedule.rs index e9edab618384..c80fa191feec 100644 --- a/turbopack/crates/turbo-tasks-fs/src/watcher/batch_schedule.rs +++ b/turbopack/crates/turbo-tasks-fs/src/watcher/batch_schedule.rs @@ -7,9 +7,13 @@ use std::{ }; use serde::Serialize; +use turbo_rcstr::RcStr; use turbo_tasks::message_queue::{CompilationEvent, Severity}; -use crate::{DiskWatcherConfig, watcher::fs_api::DiskFileSystemWatcherApi}; +use crate::{ + DiskWatcherConfig, + watcher::{BatchedInvalidations, fs_api::DiskFileSystemWatcherApi}, +}; /// Decides how long a batch of watcher events stays open, and emits a repeated /// [`FilesystemSettlingEvent`] for as long as it does. @@ -45,7 +49,9 @@ impl BatchSchedule { let now = Instant::now(); let deadline = now.checked_add(delay).unwrap_or_else(far_future); match &mut self.pending { - Some(pending) => pending.deadline = pending.deadline.max(deadline), + Some(pending) => { + pending.deadline = pending.deadline.max(deadline); + } None => { self.pending = Some(PendingBatch { started: now, @@ -68,6 +74,7 @@ impl BatchSchedule { &mut self, rx: &Receiver>, fs: &FsApi, + batch: &BatchedInvalidations, ) -> Result, RecvTimeoutError> { let max_event_delay = self.settling_event_max_delay; loop { @@ -78,7 +85,7 @@ impl BatchSchedule { let now = Instant::now(); if now >= pending.settling_event_next_at { - pending.emit_settling_event(fs, now, max_event_delay); + pending.emit_settling_event(fs, batch, now, max_event_delay); } let timeout = pending @@ -112,6 +119,7 @@ impl PendingBatch { fn emit_settling_event( &mut self, fs: &FsApi, + batch: &BatchedInvalidations, now: Instant, max_event_delay: Duration, ) { @@ -119,6 +127,9 @@ impl PendingBatch { if let Some(turbo_tasks) = fs.turbo_tasks() { turbo_tasks.send_compilation_event(Arc::new(FilesystemSettlingEvent { elapsed_secs: (now - self.started).as_secs_f64(), + last_modified_path: batch + .last_updated_path() + .map(|path| RcStr::from(format!("{path:?}"))), })); } self.event_interval = self.event_interval.saturating_mul(2).min(max_event_delay); @@ -137,6 +148,8 @@ impl PendingBatch { pub struct FilesystemSettlingEvent { /// How long the current batch has been held open, in seconds. pub elapsed_secs: f64, + /// The most recently changed path in the current batch. + pub last_modified_path: Option, } impl CompilationEvent for FilesystemSettlingEvent { @@ -149,9 +162,14 @@ impl CompilationEvent for FilesystemSettlingEvent { } fn message(&self) -> String { + let last_modified = self + .last_modified_path + .as_deref() + .map(|path| format!("; last modified: {path}")) + .unwrap_or_default(); format!( "Turbopack has seen frequent file updates and is waiting for the filesystem to settle \ - ({:.1}s elapsed so far).", + ({:.1}s elapsed so far{last_modified}).", self.elapsed_secs ) } diff --git a/turbopack/crates/turbo-tasks-fs/src/watcher/mod.rs b/turbopack/crates/turbo-tasks-fs/src/watcher/mod.rs index 192284d21fde..01a5a2ec2fc9 100644 --- a/turbopack/crates/turbo-tasks-fs/src/watcher/mod.rs +++ b/turbopack/crates/turbo-tasks-fs/src/watcher/mod.rs @@ -23,17 +23,18 @@ use bincode::{ error::{DecodeError, EncodeError}, }; use bitflags::bitflags; +use indexmap::map::Entry; use notify::{ Config, EventKind, PollWatcher, RecommendedWatcher, Watcher, event::{MetadataKind, ModifyKind, RenameMode}, }; -use rustc_hash::{FxHashMap, FxHashSet}; +use rustc_hash::FxHashSet; use tokio::sync::{RwLock, RwLockWriteGuard}; use tracing::instrument; use turbo_rcstr::RcStr; use turbo_tasks::{ - FxIndexSet, InvalidationReason, InvalidationReasonKind, Invalidator, ResolvedVc, TraitRef, - TurboTasksApi, spawn_thread, trace::TraceRawVcs, util::StaticOrArc, + FxIndexMap, FxIndexSet, InvalidationReason, InvalidationReasonKind, Invalidator, ResolvedVc, + TraitRef, TurboTasksApi, spawn_thread, trace::TraceRawVcs, util::StaticOrArc, }; use crate::{ @@ -607,7 +608,7 @@ impl DiskWatcher { 'outer: loop { loop { - match schedule.recv_event(&rx, &*fs) { + match schedule.recv_event(&rx, &*fs, &batch) { Ok(Ok(event)) => { // TODO: We might benefit from some user-facing diagnostics if it rescans // occur frequently (i.e. more than X times in Y minutes) @@ -675,15 +676,7 @@ impl DiskWatcher { Ok(Err(notify::Error { kind, paths })) => { println!("watch error ({paths:?}): {kind:?} "); - let flags = InvalidationFlags::PATH_AND_CHILDREN - | InvalidationFlags::PATH_AND_CHILDREN_DIR; - if paths.is_empty() { - batch.mark(Box::from(fs.root_path()), flags); - } else { - for path in paths { - batch.mark(path.into_boxed_path(), flags); - } - } + batch.add_error(paths, fs.root_path()); schedule.extend(config.batch_delay); } Err(RecvTimeoutError::Timeout) => { @@ -781,7 +774,9 @@ bitflags! { /// needs to happen for each, rather than in several separate sets. This avoids cloning each /// `PathBuf` into multiple collections. struct BatchedInvalidations { - paths: FxHashMap, InvalidationFlags>, + paths: FxIndexMap, InvalidationFlags>, + /// The most recently updated entry in [`Self::paths`]. + last_updated_index: Option, /// See [`Self::new_paths`]). Stored as [`None`] in non-recursive mode. new_paths: Option>>, /// Whether events are coming from [`PollWatcher`] instead of [`RecommendedWatcher`], which @@ -792,7 +787,8 @@ struct BatchedInvalidations { impl BatchedInvalidations { fn new(recursive_mode: DiskWatcherRecursiveMode, polling: bool) -> Self { Self { - paths: FxHashMap::default(), + paths: FxIndexMap::default(), + last_updated_index: None, new_paths: match recursive_mode { DiskWatcherRecursiveMode::NonRecursive => Some(FxHashSet::default()), DiskWatcherRecursiveMode::Recursive => None, @@ -830,6 +826,7 @@ impl BatchedInvalidations { fn clear(&mut self) { self.paths.clear(); + self.last_updated_index = None; if let Some(new_paths) = &mut self.new_paths { new_paths.clear(); } @@ -843,8 +840,25 @@ impl BatchedInvalidations { } } - fn mark(&mut self, path: Box, flags: InvalidationFlags) { - *self.paths.entry(path).or_insert(InvalidationFlags::empty()) |= flags; + /// Sets the `flags` for `path`. Returns the index that was modified. + fn mark(&mut self, path: Box, flags: InvalidationFlags) -> usize { + match self.paths.entry(path) { + Entry::Occupied(mut entry) => { + *entry.get_mut() |= flags; + entry.index() + } + Entry::Vacant(entry) => { + let index = entry.index(); + entry.insert(flags); + index + } + } + } + + fn last_updated_path(&self) -> Option<&Path> { + self.last_updated_index + .and_then(|index| self.paths.get_index(index)) + .map(|(path, _)| &**path) } fn mark_parent_dir(&mut self, path: &Path) { @@ -863,49 +877,45 @@ impl BatchedInvalidations { /// Updates the batch to contain updated paths from the given event. Does not perform any /// invalidations. /// - /// Returns `true` if the event contained relevant events, or `false` if it was filtered out. + /// Returns whether the event contained relevant events. #[must_use] fn add_event(&mut self, event: notify::Event) -> bool { let paths: Vec = event.paths; - if paths.is_empty() { - return false; - } + let mut last_updated_index = None; match event.kind { EventKind::Modify(ModifyKind::Data(_)) => { for path in paths { - self.mark(path.into_boxed_path(), InvalidationFlags::PATH); + last_updated_index = + Some(self.mark(path.into_boxed_path(), InvalidationFlags::PATH)); } - true } // Some backends (fsevents, polling) can report metadata events for file content changes EventKind::Modify(ModifyKind::Metadata(kind)) if self.is_content_change(kind) => { for path in paths { - self.mark(path.into_boxed_path(), InvalidationFlags::PATH); + last_updated_index = + Some(self.mark(path.into_boxed_path(), InvalidationFlags::PATH)); } - true } EventKind::Create(_) => { for path in paths { self.mark_parent_dir(&path); self.mark_new_path(&path); - self.mark( + last_updated_index = Some(self.mark( path.into_boxed_path(), InvalidationFlags::PATH_AND_CHILDREN | InvalidationFlags::PATH_AND_CHILDREN_DIR, - ); + )); } - true } EventKind::Remove(_) => { for path in paths { self.mark_parent_dir(&path); - self.mark( + last_updated_index = Some(self.mark( path.into_boxed_path(), InvalidationFlags::PATH_AND_CHILDREN | InvalidationFlags::PATH_AND_CHILDREN_DIR, - ); + )); } - true } // A single event emitted with both the `From` and `To` paths. EventKind::Modify(ModifyKind::Name(RenameMode::Both)) => { @@ -918,11 +928,10 @@ impl BatchedInvalidations { ); self.mark_parent_dir(&destination); self.mark_new_path(&destination); - self.mark( + last_updated_index = Some(self.mark( destination.into_boxed_path(), InvalidationFlags::PATH_AND_CHILDREN, - ); - true + )); } // We expect `RenameMode::Both` to cover most of the cases we need to invalidate, // but we also check other RenameModes to cover cases where notify couldn't match the @@ -930,19 +939,33 @@ impl BatchedInvalidations { EventKind::Any | EventKind::Modify(ModifyKind::Any | ModifyKind::Name(..)) => { for path in paths { self.mark_parent_dir(&path); - self.mark( + last_updated_index = Some(self.mark( path.into_boxed_path(), InvalidationFlags::PATH_AND_CHILDREN | InvalidationFlags::PATH_AND_CHILDREN_DIR, - ); + )); } - true } EventKind::Modify(ModifyKind::Metadata(..) | ModifyKind::Other) | EventKind::Access(_) - | EventKind::Other => { - // ignored - false + | EventKind::Other => {} + } + if let Some(index) = last_updated_index { + self.last_updated_index = Some(index); + true + } else { + false + } + } + + /// Updates the batch to invalidate paths associated with a watcher error. + fn add_error(&mut self, paths: Vec, root_path: &Path) { + let flags = InvalidationFlags::PATH_AND_CHILDREN | InvalidationFlags::PATH_AND_CHILDREN_DIR; + if paths.is_empty() { + self.last_updated_index = Some(self.mark(Box::from(root_path), flags)); + } else { + for path in paths { + self.last_updated_index = Some(self.mark(path.into_boxed_path(), flags)); } } } From b2e835ded64a70dce56fce3742c07f4bea616816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Benjamin=20Woodruff=20=E2=80=AE?= Date: Thu, 20 Aug 2026 17:26:38 -0700 Subject: [PATCH 4/4] 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. --- Cargo.lock | 1 + Cargo.toml | 1 + crates/next-core/src/emit.rs | 18 +- .../src/next_api/project.rs | 11 +- turbopack/crates/turbo-tasks-fs/Cargo.toml | 6 +- .../crates/turbo-tasks-fs/src/content.rs | 150 +++- turbopack/crates/turbo-tasks-fs/src/disk.rs | 735 ++++++++++++------ .../crates/turbo-tasks-fs/src/embed/fs.rs | 16 +- turbopack/crates/turbo-tasks-fs/src/lib.rs | 24 +- .../crates/turbo-tasks-fs/src/null_fs.rs | 18 +- turbopack/crates/turbo-tasks-fs/src/path.rs | 110 ++- .../crates/turbo-tasks-fs/src/read_glob.rs | 10 +- .../crates/turbo-tasks-fs/src/virtual_fs.rs | 16 +- .../crates/turbo-tasks-fs/src/windows.rs | 147 ++-- .../crates/turbo-tasks-fuzz/src/fs_watcher.rs | 26 +- .../turbo-tasks-fuzz/src/symlink_stress.rs | 56 +- .../src/derive/deterministic_hash_macro.rs | 23 +- turbopack/crates/turbo-unix-path/src/lib.rs | 12 +- turbopack/crates/turbopack-core/src/asset.rs | 38 +- .../crates/turbopack-core/src/file_source.rs | 33 +- .../turbopack-core/src/introspect/utils.rs | 4 +- .../turbopack-core/src/resolve/pattern.rs | 29 +- .../crates/turbopack-core/src/server_fs.rs | 12 +- .../crates/turbopack-core/src/version.rs | 2 +- turbopack/crates/turbopack-css/src/process.rs | 2 +- .../crates/turbopack-ecmascript/src/parse.rs | 2 +- .../src/references/external_module.rs | 13 +- .../turbopack-test-utils/src/snapshot.rs | 4 +- 28 files changed, 1035 insertions(+), 484 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1f5903fe2cec..ed64efe1791f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10260,6 +10260,7 @@ dependencies = [ "turbo-unix-path", "url", "urlencoding", + "windows-sys 0.60.2", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 43e52f1fbcb2..0b765ff566b8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -361,6 +361,7 @@ unsize = "1.1.0" unty = "0.0.4" url = "2.2.2" urlencoding = "2.1.2" +windows-sys = "0.60" uuid = "1.18.1" vergen = { version = "9.0.6", features = ["cargo"] } vergen-gitcl = { version = "1.0.8", features = ["cargo"] } diff --git a/crates/next-core/src/emit.rs b/crates/next-core/src/emit.rs index ba1a7326e678..bb894b4ff86f 100644 --- a/crates/next-core/src/emit.rs +++ b/crates/next-core/src/emit.rs @@ -285,22 +285,14 @@ async fn assets_diff( ), } } - ( - AssetContent::Redirect { - target: target1, - link_type: link_type1, - }, - AssetContent::Redirect { - target: target2, - link_type: link_type2, - }, - ) => { - if target1 == target2 && link_type1 == link_type2 { + (AssetContent::Redirect(content1), AssetContent::Redirect(content2)) => { + if content1.target == content2.target && content1.target_type == content2.target_type { None } else { Some(format!( - "assets at the same path are both redirects but point to different targets: \ - {target1} vs {target2}" + "assets at the same path are both redirects but disagree: {:?} ({:?}) vs {:?} \ + ({:?})", + content1.target, content1.target_type, content2.target, content2.target_type, )) } } diff --git a/crates/next-napi-bindings/src/next_api/project.rs b/crates/next-napi-bindings/src/next_api/project.rs index d23497f285a5..3b284bdd3b0c 100644 --- a/crates/next-napi-bindings/src/next_api/project.rs +++ b/crates/next-napi-bindings/src/next_api/project.rs @@ -58,9 +58,11 @@ use turbo_tasks::{ unmark_top_level_task_may_leak_eventually_consistent_state, }; use turbo_tasks_backend::db_invalidation::invalidation_reasons; +#[cfg(windows)] +use turbo_tasks_fs::windows::to_verbatim_with_case_folded_disk; use turbo_tasks_fs::{ DiskFileSystem, FileContent, FileSystem, FileSystemPath, canonicalize_to_rcstr, invalidation, - to_verbatim_with_case_folded_disk, util::uri_from_file, + util::uri_from_file, }; use turbo_unix_path::{get_relative_path_to, unix_to_sys}; use turbopack_core::{ @@ -2385,9 +2387,12 @@ fn parse_and_canonicalize_source_url(source_url: &str) -> Result<(RcStr, Option< Err(_) => { // The file may not exist (e.g. a stale stack frame). Fall back to a purely lexical // normalization that approximates the canonical format. - if cfg!(windows) { + #[cfg(windows)] + { to_verbatim_with_case_folded_disk(&path).unwrap_or(path) - } else { + } + #[cfg(not(windows))] + { path } } diff --git a/turbopack/crates/turbo-tasks-fs/Cargo.toml b/turbopack/crates/turbo-tasks-fs/Cargo.toml index cb9537eca30e..3e3448f5dec2 100644 --- a/turbopack/crates/turbo-tasks-fs/Cargo.toml +++ b/turbopack/crates/turbo-tasks-fs/Cargo.toml @@ -59,6 +59,11 @@ urlencoding = { workspace = true } [target.'cfg(windows)'.dependencies] omnipath = "0.1.6" +windows-sys = { workspace = true, features = [ + "Win32_Foundation", + "Win32_Storage_FileSystem", + "Win32_System_SystemServices", +] } [dev-dependencies] criterion = { workspace = true, features = ["async_tokio"] } @@ -68,4 +73,3 @@ sha2 = { workspace = true } tempfile = { workspace = true } turbo-tasks-testing = { workspace = true } turbo-tasks-backend = { workspace = true } - diff --git a/turbopack/crates/turbo-tasks-fs/src/content.rs b/turbopack/crates/turbo-tasks-fs/src/content.rs index 8b90fb54f9ec..92c3d752f1ae 100644 --- a/turbopack/crates/turbo-tasks-fs/src/content.rs +++ b/turbopack/crates/turbo-tasks-fs/src/content.rs @@ -10,7 +10,6 @@ use std::{ use anyhow::{Result, bail}; use bincode::{Decode, Encode}; -use bitflags::bitflags; use jsonc_parser::{ParseOptions, parse_to_serde_value}; use mime::Mime; use serde_json::Value; @@ -21,6 +20,7 @@ use turbo_tasks_hash::{ }; use crate::{ + FileSystemEntryType, FileSystemPath, json::UnparsableJson, retry::retry_blocking, rope::{Rope, RopeReader}, @@ -163,47 +163,69 @@ pub(crate) enum FileComparison { NotEqual, } -bitflags! { - #[derive( - Default, - TraceRawVcs, - NonLocalValue, - DeterministicHash, - Encode, - Decode, - )] - pub struct LinkType: u8 { - const DIRECTORY = 0b00000001; - const ABSOLUTE = 0b00000010; - } +/// The target of a symbolic link, as read from a filesystem. +/// +/// Every variant carries the `resolved` path the link points at, computed once by +/// [`crate::FileSystem::read_link`], which is also what guarantees the target stays inside the +/// filesystem root — a link whose target leaves the root is [`LinkContent::Invalid`] instead. +#[derive(Clone, Debug, Hash, PartialEq, Eq, TraceRawVcs, NonLocalValue, Encode, Decode)] +pub enum LinkTarget { + /// The link is an absolute path on disk. + Absolute { resolved: FileSystemPath }, + Relative { + /// The value read from the link. The path is lexically converted to a [unix-style + /// path][turbo_unix_path::sys_to_unix], but it may contain `..` relative to the *directory + /// containing the link*. + raw: RcStr, + /// The link target relative the a `DiskFileSystem` root. + resolved: FileSystemPath, + }, } -/// The contents of a symbolic link. On Windows, this may be a junction point. +impl LinkTarget { + /// The path this link points at. + pub fn file_system_path(&self) -> &FileSystemPath { + match self { + LinkTarget::Absolute { resolved } | LinkTarget::Relative { resolved, .. } => resolved, + } + } + + /// The type of the file this link points at. + /// + /// This only follows a single link: if the target is itself a symbolic link, this returns + /// [`FileSystemEntryType::Symlink`]. Use [`FileSystemPath::realpath`] to follow a chain of + /// links. + /// + /// A dangling link returns [`FileSystemEntryType::NotFound`]. + pub async fn target_type(&self) -> Result { + Ok(*self.file_system_path().get_type().await?) + } +} + +/// The contents of a symbolic link, as read from a filesystem. On Windows, this may be a junction +/// point. +/// +/// We treat symbolic links and junction points on Windows as equivalent when reading. /// -/// When reading, we treat symbolic links and junction points on Windows as equivalent. When -/// creating a new link, we always create junction points, because symlink creation may fail if -/// Windows "developer mode" is not enabled and we're running in an unprivileged environment. +/// This describes the link itself and never the type of the file it points at. Use +/// [`LinkTarget::target_type`] if you need the type of the target. #[turbo_tasks::value(shared)] -#[derive(Debug, DeterministicHash)] +#[derive(Debug)] pub enum LinkContent { - /// A valid symbolic link pointing to `target`, a unix-style path. - /// - /// If [`LinkType::ABSOLUTE`] is set, `target` is normalized and relative to the *filesystem - /// root* (so that absolute system paths never end up in the persistent cache). Otherwise, - /// `target` is the raw value read from the link — unnormalized, may contain `..` — and is - /// relative to the *directory containing the link*. + /// A valid symbolic link pointing to `target`. + Link { target: LinkTarget }, + /// The link itself does not exist at the path given to [`FileSystemPath::read_link`]. /// - /// A relative `target` must stay raw so that [`FileSystem::write_link`] round-trips it - /// exactly: the value is written verbatim and compared against [`std::fs::read_link`] to - /// skip unchanged links. - Link { - target: RcStr, - link_type: LinkType, - }, - // Invalid means the link is invalid it points out of the filesystem root - Invalid, - // The target was not found + /// This says nothing about whether the link's target exists: a dangling link is still + /// returned as [`LinkContent::Link`]. NotFound, + /// The link could not be read. + /// + /// This includes all I/O errors other than `NotFound`, denied paths, and targets that leave the + /// filesystem root. A relative target that steps out of the root and back into it is also + /// invalid, since resolving it would need the names of the root's own ancestors, which a + /// root-relative path doesn't carry. + Invalid { reason: RcStr }, } #[turbo_tasks::value_impl] @@ -213,14 +235,70 @@ impl LinkContent { /// re-create a symlink as a symlink instead of copying the resolved file. #[turbo_tasks::function] pub async fn hash(&self, salt: Vc, algorithm: HashAlgorithm) -> Result> { + #[derive(DeterministicHash)] + enum SimplifiedLinkContent<'a> { + Absolute(&'a RcStr), + Relative(&'a RcStr), + NotFound, + Invalid, // the actual error message doesn't matter for this API + } + let simplified = match self { + LinkContent::Link { target } => match target { + LinkTarget::Absolute { resolved } => { + SimplifiedLinkContent::Absolute(&resolved.path) + } + LinkTarget::Relative { raw, resolved: _ } => SimplifiedLinkContent::Relative(raw), + }, + LinkContent::NotFound => SimplifiedLinkContent::NotFound, + LinkContent::Invalid { reason: _ } => SimplifiedLinkContent::Invalid, + }; Ok(Vc::cell(RcStr::from(deterministic_hash( &salt.await?, - self, + simplified, algorithm, )))) } } +/// The target of a symbolic link to create, used by [`WriteLinkContent`]. +/// +/// Unlike [`LinkTarget`] this carries only the raw path: the write side never needs the target +/// resolved, and the link being created may not even exist yet. +#[derive( + Clone, Debug, Hash, PartialEq, Eq, TraceRawVcs, NonLocalValue, DeterministicHash, Encode, Decode, +)] +pub enum WriteLinkTarget { + /// Normalized and relative to the *filesystem root*. + Absolute(RcStr), + /// Written verbatim, relative to the *directory containing the link*. + Relative(RcStr), +} + +/// The file type of the target of a newly written link. This value is only used on Windows. +#[derive( + Clone, Debug, Hash, PartialEq, Eq, TraceRawVcs, NonLocalValue, DeterministicHash, Encode, Decode, +)] +pub enum WriteLinkTargetType { + /// Represents a link to a file or a symbolic link that is not a junction point. This is likely + /// to fail on Windows, where symbolic links are not enabled by default. + FileNonPortable, + /// Represents a link to a directory. On Windows, this may also be a link to a junction point. + DirectoryOrJunctionPoint, +} + +/// The symbolic link to create at a path, passed to [`FileSystemPath::write_link`]. +/// +/// This is separate from [`LinkContent`] because writing needs to know whether the target is a +/// directory, while reading a link does not: on Windows we always create junction points for +/// directories, because symlink creation may fail if "developer mode" is not enabled and we're +/// running in an unprivileged environment. +#[turbo_tasks::value(shared)] +#[derive(Clone, Debug, DeterministicHash)] +pub struct WriteLinkContent { + pub target: WriteLinkTarget, + pub target_type: WriteLinkTargetType, +} + #[turbo_tasks::value(shared)] #[derive(Clone, DeterministicHash, PartialOrd, Ord)] pub struct File { diff --git a/turbopack/crates/turbo-tasks-fs/src/disk.rs b/turbopack/crates/turbo-tasks-fs/src/disk.rs index 2133641cffee..3db12ac5546f 100644 --- a/turbopack/crates/turbo-tasks-fs/src/disk.rs +++ b/turbopack/crates/turbo-tasks-fs/src/disk.rs @@ -7,11 +7,11 @@ use std::{ future::Future, io::{self, ErrorKind, Write as _}, mem::take, - path::{MAIN_SEPARATOR, Path, PathBuf}, + path::{Component, MAIN_SEPARATOR, Path, PathBuf, Prefix}, sync::{Arc, LazyLock, Weak}, }; -use anyhow::{Context, Result, anyhow, bail}; +use anyhow::{Context, Result, anyhow}; use async_trait::async_trait; use bincode::{Decode, Encode}; #[cfg(windows)] @@ -23,7 +23,7 @@ use tokio::{ sync::{RwLock, RwLockReadGuard}, }; use tracing::Instrument; -use turbo_rcstr::RcStr; +use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ CapturedEffect, Effect, EffectExt, EffectStateStorage, InvalidationReason, NonLocalValue, ReadRef, ResolvedVc, TurboTasksApi, ValueToString, Vc, debug::ValueDebugFormat, parallel, @@ -32,10 +32,12 @@ use turbo_tasks::{ use turbo_tasks_hash::{hash_xxh3_hash64, hash_xxh3_hash128}; use turbo_unix_path::{normalize_path, sys_to_unix, unix_to_sys}; +#[cfg(windows)] +use crate::windows::{is_link_junction_point, to_verbatim_with_case_folded_disk}; use crate::{ - AnyhowWrapper, File, FileComparison, FileContent, FileMeta, FileSystem, FileSystemEntryType, - FileSystemPath, LinkContent, LinkType, PersistedFileContent, RawDirectoryContent, - RawDirectoryEntry, + AnyhowWrapper, File, FileComparison, FileContent, FileMeta, FileSystem, FileSystemPath, + LinkContent, LinkTarget, PersistedFileContent, RawDirectoryContent, RawDirectoryEntry, + WriteLinkContent, WriteLinkTarget, WriteLinkTargetType, invalidation::Write, invalidator_map::InvalidatorMap, mutex_map::MutexMap, @@ -537,8 +539,7 @@ impl DiskFileSystem { // Unlike `std::fs::canonicalize`, this is a purely lexical operation: it does not // resolve symlinks or 8.3 short name format. #[cfg(windows)] - let normalized_sys_path = - crate::windows::to_verbatim_with_case_folded_disk(sys_path).ok()?; + let normalized_sys_path = to_verbatim_with_case_folded_disk(sys_path).ok()?; normalized_sys_path .strip_prefix(self.inner.root_path()) @@ -899,68 +900,156 @@ impl FileSystem for DiskFileSystem { let this = self.await?; let inner = &this.inner; if inner.is_path_denied(&fs_path) { - return Ok(LinkContent::NotFound.cell()); + return Ok(LinkContent::Invalid { + reason: rcstr!("access to the symlink path is denied"), + } + .cell()); } let full_link_path = Arc::new(this.to_sys_path_raw(&fs_path)); inner.register_read_invalidator(&full_link_path).await?; let _lock = inner.lock_path(full_link_path.clone()).await; - let target_sys_path = match retry_blocking(|| std::fs::read_link(&**full_link_path)) + let mut target_sys_path = match retry_blocking(|| std::fs::read_link(&**full_link_path)) .instrument(tracing::info_span!("read symlink", name = ?full_link_path)) .concurrency_limited(&inner.read_semaphore) .await { Ok(res) => res, - Err(_) => return Ok(LinkContent::NotFound.cell()), + Err(err) if err.kind() == ErrorKind::NotFound => { + return Ok(LinkContent::NotFound.cell()); + } + Err(err) => { + return Ok(LinkContent::Invalid { + reason: RcStr::from(err.to_string()), + } + .cell()); + } }; - // A relative symlink target is resolved relative to the directory *containing* the link, - // not the link path itself, so pass the parent as `relative_to`. (For absolute targets - // `try_from_sys_path` ignores `relative_to`.) - let link_parent = fs_path.parent(); - // First try a cheap, purely lexical conversion of the raw target. - let mut target_fs_path = this.try_from_sys_path(self, &target_sys_path, Some(&link_parent)); - - // If the lexical try_from_sys_path failed for an absolute target, the target may just be - // spelled differently than our canonicalized filesystem root (e.g. case insensitive - // filesystem, a Windows 8.3 short name, or a symlink in the path). This performs - // session-dependent IO to resolve the fs root path. - if target_fs_path.is_none() && target_sys_path.is_absolute() { - target_fs_path = this - .resolve_link_target_ancestry_slow_path(self, &target_sys_path) - .await?; + if cfg!(windows) && target_sys_path.has_root() && !target_sys_path.is_absolute() { + // On windows, `\foo` has a root but no drive and is not absolute. Just convert it to + // absolute and treat it like it's absolute. + let mut absolute_target = inner.root_path().to_path_buf(); + absolute_target.push(target_sys_path); + target_sys_path = absolute_target; } - let Some(target_fs_path) = target_fs_path else { - // The target leaves the filesystem root (or is a dangling link whose parent directory - // couldn't be canonicalized). - return Ok(LinkContent::Invalid.cell()); - }; - - let mut link_type = LinkType::default(); - // TODO(bgw): Reading the type here is silly, the callers could do it. - // The reason `LinkContent` contains the type information is just for the `write_link` - // codepath, which needs to know if it can create a Windows junction point or not. - let file_type = target_fs_path.get_type().await?; - if matches!(&*file_type, FileSystemEntryType::Directory) { - link_type |= LinkType::DIRECTORY; - } + let target = if target_sys_path.is_absolute() { + // First try a cheap, purely lexical conversion of the raw target. `relative_to` is + // ignored for absolute targets. + let mut target_fs_path = this.try_from_sys_path(self, &target_sys_path, None); + + // If that failed, the target may just be spelled differently than our canonicalized + // filesystem root (e.g. case insensitive filesystem, a Windows 8.3 short name, or a + // symlink in the path). This performs session-dependent IO to resolve the fs root + // path. + if target_fs_path.is_none() { + target_fs_path = this + .resolve_link_target_ancestry_slow_path(self, &target_sys_path) + .await?; + } - let target; - if target_sys_path.is_absolute() { - // absolute path, rewrite from the sys root to the DiskFileSystem root - target = target_fs_path.path; - link_type |= LinkType::ABSOLUTE; + let Some(target_fs_path) = target_fs_path else { + // The target leaves the filesystem root (or is a dangling link whose parent + // directory couldn't be canonicalized). + return Ok(LinkContent::Invalid { + reason: rcstr!( + "the symlink target leaves the filesystem root or its parent directory \ + could not be resolved" + ), + } + .cell()); + }; + // Rewrite from the sys root to the DiskFileSystem root. + LinkTarget::Absolute { + resolved: target_fs_path, + } } else { - // link-relative, the raw value read from the link, converted to a unix-style format - let target_str = target_sys_path.to_str().with_context(|| { - format!("symlink target {target_sys_path:?} is not valid unicode") - })?; - target = RcStr::from(sys_to_unix(target_str)); + if cfg!(windows) + && let Some(Component::Prefix(target_prefix)) = target_sys_path.components().next() + { + // Edge case: Windows supports relative file paths with a prefixed disk, e.g. + // `C:foo`. These only make sense when the drive letter matches the + // DiskFileSystem root. If it matches, we can safely strip it. + let Prefix::Disk(target_drive) = target_prefix.kind() else { + unreachable!( + "path is relative, but contains a prefix that should form an absolute path" + ); + }; + let root_drive = match inner.root_path().components().next() { + Some(Component::Prefix(root_prefix)) => match root_prefix.kind() { + Prefix::Disk(drive) | Prefix::VerbatimDisk(drive) => Some(drive), + _ => None, + }, + _ => None, + }; + if root_drive + .is_none_or(|root_drive| !target_drive.eq_ignore_ascii_case(&root_drive)) + { + return Ok(LinkContent::Invalid { + reason: rcstr!( + "the symlink target uses a different drive than the filesystem root" + ), + } + .cell()); + } + + target_sys_path = target_sys_path.components().skip(1).collect(); + } + + // The raw value read from the link, converted to a unix-style format. A relative + // target is resolved against the directory *containing* the link, not the link itself. + let Some(target_str) = target_sys_path.to_str() else { + return Ok(LinkContent::Invalid { + reason: RcStr::from(format!( + "the symlink target {target_sys_path:?} is not valid unicode" + )), + } + .cell()); + }; + let raw = RcStr::from(sys_to_unix(target_str)); + + // Require the target to stay within the filesystem root at every step, not just at + // the end. A target like `../..//foo` steps out of the root and back + // in; resolving that needs the names of the root's own ancestors, which a + // root-relative `FileSystemPath` doesn't carry. Rejecting it here is what lets + // `LinkTarget` carry a resolved path at all. + let Some(resolved) = fs_path.parent().try_join(&raw) else { + return Ok(LinkContent::Invalid { + reason: rcstr!("the symlink target leaves the filesystem root"), + } + .cell()); + }; + LinkTarget::Relative { raw, resolved } }; - Ok(LinkContent::Link { target, link_type }.cell()) + Ok(LinkContent::Link { target }.cell()) + } + + #[turbo_tasks::function(fs, session_dependent)] + async fn is_junction_point(&self, fs_path: FileSystemPath) -> Result> { + #[cfg(windows)] + { + if self.inner.is_path_denied(&fs_path) { + return Ok(Vc::cell(false)); + } + let full_path = Arc::new(self.to_sys_path_raw(&fs_path)); + self.inner.register_read_invalidator(&full_path).await?; + + let _lock = self.inner.lock_path(full_path.clone()).await; + let is_junction_point = retry_blocking(|| is_link_junction_point(&full_path)) + .instrument(tracing::info_span!("read junction point", name = ?full_path)) + .concurrency_limited(&self.inner.read_semaphore) + .await + .with_context(|| format!("checking junction point {full_path:?}"))?; + Ok(Vc::cell(is_junction_point)) + } + #[cfg(not(windows))] + { + let _ = fs_path; + Ok(Vc::cell(false)) + } } #[turbo_tasks::function(fs)] @@ -1185,7 +1274,7 @@ impl FileSystem for DiskFileSystem { async fn write_link( self: ResolvedVc, fs_path: FileSystemPath, - target: ResolvedVc, + target: ResolvedVc, ) -> Result<()> { // You might be tempted to use `session_dependent` here, but we purely declare a side // effect and does not need to be re-executed in the next session. All side effects are @@ -1206,7 +1295,7 @@ impl FileSystem for DiskFileSystem { struct WriteLinkEffect { full_path: Arc, fs: ResolvedVc, - target: ResolvedVc, + target: ResolvedVc, content_hash: u128, } @@ -1242,7 +1331,7 @@ impl FileSystem for DiskFileSystem { struct CapturedWriteLinkEffect { full_path: Arc, inner: Arc, - content: Option>, + content: Option>, content_hash: u128, } @@ -1268,46 +1357,33 @@ impl FileSystem for DiskFileSystem { } impl CapturedWriteLinkEffect { - async fn apply_inner(&self, content: &ReadRef) -> anyhow::Result<()> { + async fn apply_inner(&self, content: &ReadRef) -> anyhow::Result<()> { let full_path = self.full_path.clone(); let _lock = self.inner.lock_path(full_path.clone()).await; - enum OsSpecificLinkContent { - Link { - #[cfg(windows)] - is_directory: bool, - target: PathBuf, - }, - NotFound, - Invalid, - } - - let os_specific_link_content = match &**content { - LinkContent::Link { target, link_type } => { - let is_directory = link_type.contains(LinkType::DIRECTORY); - let target_path = if link_type.contains(LinkType::ABSOLUTE) { - self.inner.root_path().join(unix_to_sys(target).as_ref()) + let WriteLinkContent { + target, + target_type, + } = &**content; + let is_directory = + matches!(target_type, WriteLinkTargetType::DirectoryOrJunctionPoint); + let target = match target { + WriteLinkTarget::Absolute(target) => { + self.inner.root_path().join(unix_to_sys(target).as_ref()) + } + WriteLinkTarget::Relative(target) => { + let relative_target = PathBuf::from(unix_to_sys(target).as_ref()); + if cfg!(windows) && is_directory { + // Windows junction points must always be stored as absolute + full_path + .parent() + .unwrap_or(&full_path) + .join(relative_target) } else { - let relative_target = PathBuf::from(unix_to_sys(target).as_ref()); - if cfg!(windows) && is_directory { - // Windows junction points must always be stored as absolute - full_path - .parent() - .unwrap_or(&full_path) - .join(relative_target) - } else { - relative_target - } - }; - OsSpecificLinkContent::Link { - #[cfg(windows)] - is_directory, - target: target_path, + relative_target } } - LinkContent::Invalid => OsSpecificLinkContent::Invalid, - LinkContent::NotFound => OsSpecificLinkContent::NotFound, }; let old_content = match retry_blocking(|| std::fs::read_link(&**full_path)) @@ -1318,134 +1394,119 @@ impl FileSystem for DiskFileSystem { Ok(res) => Some((res.is_absolute(), res)), Err(_) => None, }; - let is_equal = match (&os_specific_link_content, &old_content) { - ( - OsSpecificLinkContent::Link { target, .. }, - Some((old_is_absolute, old_target)), - ) => target == old_target && target.is_absolute() == *old_is_absolute, - (OsSpecificLinkContent::NotFound, None) => true, - _ => false, + #[cfg(not(windows))] + let is_equal = match &old_content { + Some((old_is_absolute, old_target)) => { + target == *old_target && target.is_absolute() == *old_is_absolute + } + None => false, + }; + #[cfg(windows)] + let is_equal = match &old_content { + Some((old_is_absolute, old_target)) => { + target == *old_target + && target.is_absolute() == *old_is_absolute + && is_link_junction_point(&full_path).ok() == Some(is_directory) + } + None => false, }; if is_equal { return Ok(()); } - match os_specific_link_content { - OsSpecificLinkContent::Link { - target, - #[cfg(windows)] - is_directory, - .. - } => { - #[derive(thiserror::Error, Debug)] - #[error("{msg}: {source}")] - struct SymlinkCreationError { - msg: &'static str, - #[source] - source: io::Error, - } + #[derive(thiserror::Error, Debug)] + #[error("{msg}: {source}")] + struct SymlinkCreationError { + msg: &'static str, + #[source] + source: io::Error, + } - let mut missing_parent_dir = false; - let mut has_old_content = old_content.is_some(); - let try_create_link = || { - if missing_parent_dir && let Some(parent) = full_path.parent() { - std::fs::create_dir_all(parent).map_err(|err| { - SymlinkCreationError { - msg: "failed to create directory", - source: err, - } - })?; - missing_parent_dir = false; + let mut missing_parent_dir = false; + let mut has_old_content = old_content.is_some(); + let try_create_link = || { + if missing_parent_dir && let Some(parent) = full_path.parent() { + std::fs::create_dir_all(parent).map_err(|err| SymlinkCreationError { + msg: "failed to create directory", + source: err, + })?; + missing_parent_dir = false; + } + if has_old_content { + // Remove existing symlink before creating a new one. On Unix, + // symlink(2) fails with EEXIST if the link already exists instead + // of overwriting it. Windows has similar behavior with junction + // points. + remove_symbolic_link_dir_helper(&full_path).map_err(|err| { + SymlinkCreationError { + msg: "removal of existing symbolic link or junction point failed", + source: err, } - if has_old_content { - // Remove existing symlink before creating a new one. On Unix, - // symlink(2) fails with EEXIST if the link already exists instead - // of overwriting it. Windows has similar behavior with junction - // points. - remove_symbolic_link_dir_helper(&full_path).map_err(|err| { - SymlinkCreationError { - msg: "removal of existing symbolic link or junction point \ - failed", - source: err, - } - })?; - has_old_content = false; + })?; + has_old_content = false; + } + #[cfg(not(windows))] + let io_result = std::os::unix::fs::symlink(&target, &**full_path); + #[cfg(windows)] + let io_result = if is_directory { + std::os::windows::fs::junction_point(&target, &**full_path) + } else { + std::os::windows::fs::symlink_file(&target, &**full_path) + }; + io_result.map_err(|err| { + match err.kind() { + ErrorKind::NotFound => { + // create the parent dirs in the next attempt + missing_parent_dir = true; } - #[cfg(not(windows))] - let io_result = std::os::unix::fs::symlink(&target, &**full_path); - #[cfg(windows)] - let io_result = if is_directory { - std::os::windows::fs::junction_point(&target, &**full_path) - } else { - std::os::windows::fs::symlink_file(&target, &**full_path) - }; - io_result.map_err(|err| { - match err.kind() { - ErrorKind::NotFound => { - // create the parent dirs in the next attempt - missing_parent_dir = true; - } - ErrorKind::AlreadyExists => { - // try to remove the symlink on the next attempt - has_old_content = true; - } - _ => {} - } - SymlinkCreationError { - msg: "creation of a new symbolic link or junction point failed", - source: err, - } - }) - }; - fn can_retry_link(err: &SymlinkCreationError) -> bool { - matches!( - err.source.kind(), - ErrorKind::NotFound | ErrorKind::AlreadyExists - ) || can_retry(&err.source) + ErrorKind::AlreadyExists => { + // try to remove the symlink on the next attempt + has_old_content = true; + } + _ => {} } - let err_context = || { - #[cfg(not(windows))] - let message = format!( - "failed to create symlink at {full_path:?} pointing to {target:?}" - ); - #[cfg(windows)] - let message = if is_directory { - format!( - "failed to create junction point at {full_path:?} pointing to \ - {target:?}" - ) - } else { - format!( - "failed to create symlink at {full_path:?} pointing to \ - {target:?}\n\ - (Note: creating file symlinks on Windows require developer \ - mode or admin permissions: \ - https://learn.microsoft.com/en-us/windows/advanced-settings/developer-mode)", - ) - }; - message - }; - retry_blocking_custom(try_create_link, can_retry_link) - .instrument(tracing::info_span!( - "write symlink", - name = ?full_path, - target = ?target, - )) - .concurrency_limited(&self.inner.write_semaphore) - .await - .with_context(err_context)?; - } - OsSpecificLinkContent::Invalid => { - bail!("invalid symlink target: {full_path:?}"); - } - OsSpecificLinkContent::NotFound => { - retry_blocking(|| remove_symbolic_link_dir_helper(&full_path)) - .instrument(tracing::info_span!("remove symlink", name = ?full_path)) - .concurrency_limited(&self.inner.write_semaphore) - .await - .with_context(|| format!("removing {full_path:?} failed"))?; - } + SymlinkCreationError { + msg: "creation of a new symbolic link or junction point failed", + source: err, + } + }) + }; + fn can_retry_link(err: &SymlinkCreationError) -> bool { + matches!( + err.source.kind(), + ErrorKind::NotFound | ErrorKind::AlreadyExists + ) || can_retry(&err.source) } + let err_context = || { + #[cfg(not(windows))] + let message = + format!("failed to create symlink at {full_path:?} pointing to {target:?}"); + #[cfg(windows)] + let message = if is_directory { + format!( + "failed to create junction point at {full_path:?} pointing to \ + {target:?}" + ) + } else { + format!( + "failed to create symlink at {full_path:?} pointing to \ + {target:?}\n\ + (Note: creating file symlinks on Windows require developer \ + mode or admin permissions: \ + https://learn.microsoft.com/en-us/windows/advanced-settings/developer-mode)", + ) + }; + message + }; + retry_blocking_custom(try_create_link, can_retry_link) + .instrument(tracing::info_span!( + "write symlink", + name = ?full_path, + target = ?target, + )) + .concurrency_limited(&self.inner.write_semaphore) + .await + .with_context(err_context)?; // Invalidate any read tasks tracking this path so they re-read the new content self.inner.invalidate_from_write(&self.full_path); @@ -1657,8 +1718,9 @@ mod tests { use super::extract_effects_operation; use crate::{ - DiskFileSystem, FileSystem, FileSystemPath, LinkContent, LinkType, - canonicalize_to_rcstr, + DiskFileSystem, FileSystem, FileSystemEntryType, FileSystemPath, LinkContent, + LinkTarget, RealPathResultError, WriteLinkContent, WriteLinkTarget, + WriteLinkTargetType, canonicalize_to_rcstr, }; #[turbo_tasks::function(operation, root)] @@ -1670,9 +1732,9 @@ mod tests { let write_file = |f| { fs.write_link( f, - LinkContent::Link { - target: format!("{target}/data.txt").into(), - link_type: LinkType::empty(), + WriteLinkContent { + target: WriteLinkTarget::Relative(format!("{target}/data.txt").into()), + target_type: WriteLinkTargetType::FileNonPortable, } .cell(), ) @@ -1684,9 +1746,9 @@ mod tests { let write_dir = |f| { fs.write_link( f, - LinkContent::Link { - target: target.clone(), - link_type: LinkType::DIRECTORY, + WriteLinkContent { + target: WriteLinkTarget::Relative(target.clone()), + target_type: WriteLinkTargetType::DirectoryOrJunctionPoint, } .cell(), ) @@ -1779,8 +1841,10 @@ mod tests { assert_eq!( *sibling, LinkContent::Link { - target: rcstr!("foo.txt"), - link_type: LinkType::empty(), + target: LinkTarget::Relative { + raw: rcstr!("foo.txt"), + resolved: root_path.join("sub/foo.txt")?, + }, } ); @@ -1789,14 +1853,234 @@ mod tests { assert_eq!( *parent, LinkContent::Link { - target: rcstr!("../root.txt"), - link_type: LinkType::empty(), + target: LinkTarget::Relative { + raw: rcstr!("../root.txt"), + resolved: root_path.join("root.txt")?, + }, } ); Ok(()) } + /// `read_link` never looks at the target, so a dangling link still reads back as a valid + /// [`LinkContent::Link`]. Resolving it is what discovers the target is missing. + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_dangling_symlink() { + use std::os::unix::fs::symlink; + + let scratch = tempfile::tempdir().unwrap(); + let path = scratch.path().to_owned(); + create_dir_all(path.join("sub")).unwrap(); + symlink("missing.txt", path.join("sub/link-dangling")).unwrap(); + symlink("link-dangling", path.join("sub/link-chain")).unwrap(); + + let root = canonicalize_to_rcstr(&path).unwrap(); + + #[turbo_tasks::function(operation, root)] + async fn assert_operation( + fs: ResolvedVc, + root_path: FileSystemPath, + ) -> anyhow::Result<()> { + let link_path = root_path.join("sub/link-dangling")?; + + // The link itself is perfectly valid; only its target is missing. + let link = fs.read_link(link_path.clone()).await?; + let LinkContent::Link { target } = &*link else { + anyhow::bail!("expected a valid link, got {link:?}"); + }; + assert_eq!( + *target, + LinkTarget::Relative { + raw: rcstr!("missing.txt"), + resolved: root_path.join("sub/missing.txt")?, + } + ); + assert_eq!(target.target_type().await?, FileSystemEntryType::NotFound,); + + // `realpath` follows the link, so it must report the missing target rather than + // succeeding with a path that doesn't exist. + let result = link_path.realpath_with_links().await?; + assert!( + matches!( + &result.path_result, + Err(RealPathResultError::Invalid { reason }) + if reason == "a symlink target does not exist" + ), + "realpath must report the missing target as an invalid chain: {:?}", + result.path_result + ); + let error = result.path_result.as_ref().unwrap_err(); + let message = error.as_error_message(&link_path, &result).await?; + assert!( + message.contains("could not be resolved: a symlink target does not exist"), + "unexpected error message: {message}" + ); + + // The same missing target after another link is still an invalid chain, never a + // missing initial link. + let chain_path = root_path.join("sub/link-chain")?; + let result = chain_path.realpath_with_links().await?; + assert!( + matches!( + &result.path_result, + Err(RealPathResultError::Invalid { reason }) + if reason == "a symlink target does not exist" + ), + "realpath must report a missing target later in a chain as invalid: {:?}", + result.path_result + ); + + // Resolving a path that simply doesn't exist is not an error, though: there is no + // link involved, so it resolves to itself. + let missing = root_path.join("sub/missing.txt")?; + let result = missing.realpath_with_links().await?; + assert_eq!(result.path_result, Ok(missing)); + + Ok(()) + } + + let tt = turbo_tasks::TurboTasks::new(TurboTasksBackend::new( + BackendOptions::default(), + noop_backing_storage(), + )); + + tt.run_once(async move { + let fs = disk_file_system_operation(root) + .resolve() + .strongly_consistent() + .await?; + + assert_operation(fs, disk_file_system_root(fs)) + .read_strongly_consistent() + .await?; + + anyhow::Ok(()) + }) + .await + .unwrap(); + + tt.stop_and_wait().await; + } + + /// A relative target must stay inside the filesystem root at every step, not just at the + /// end. Both of these step above the root; one comes back into it and one doesn't, but + /// neither can be resolved against a root-relative [`FileSystemPath`], so `read_link` + /// rejects both and every [`LinkContent::Link`] stays resolvable by construction. + #[cfg(unix)] + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_read_escaping_relative_symlink() { + use std::os::unix::fs::symlink; + + let scratch = tempfile::tempdir().unwrap(); + // The fs root is a subdirectory, so that `../..` can step above it and back in. + let path = scratch.path().join("the-root"); + create_dir_all(path.join("sub")).unwrap(); + File::create_new(path.join("root.txt")) + .unwrap() + .write_all(b"root") + .unwrap(); + // Steps above the root and back down into it. + symlink("../../the-root/root.txt", path.join("sub/link-reentrant")).unwrap(); + // Steps above the root and back down into a sibling of the root, so it escapes. + create_dir_all(scratch.path().join("sibling")).unwrap(); + File::create_new(scratch.path().join("sibling/root.txt")) + .unwrap() + .write_all(b"sibling") + .unwrap(); + symlink("../../sibling/root.txt", path.join("sub/link-sideways")).unwrap(); + // Stays inside the root the whole way. + symlink("../root.txt", path.join("sub/link-inside")).unwrap(); + // A target naming a file that literally contains a backslash. + File::create_new(path.join("sub/a\\b.txt")) + .unwrap() + .write_all(b"backslash") + .unwrap(); + symlink("a\\b.txt", path.join("sub/link-backslash")).unwrap(); + + let root = canonicalize_to_rcstr(&path).unwrap(); + + #[turbo_tasks::function(operation, root)] + async fn assert_operation( + fs: ResolvedVc, + root_path: FileSystemPath, + ) -> anyhow::Result<()> { + // sub/link-reentrant -> ../..//root.txt, which steps above the root + // and back down into it. Resolving this would need the names of the root's own + // ancestors, which a root-relative path doesn't carry. + let reentrant = fs.read_link(root_path.join("sub/link-reentrant")?).await?; + assert!(matches!( + &*reentrant, + LinkContent::Invalid { reason } + if reason == "the symlink target leaves the filesystem root" + )); + + // sub/link-sideways -> ../../sibling/root.txt, which steps above the root and down + // into a sibling, so it genuinely ends outside. + let sideways = fs.read_link(root_path.join("sub/link-sideways")?).await?; + assert!(matches!( + &*sideways, + LinkContent::Invalid{reason} + if reason == "the symlink target leaves the filesystem root" + )); + + // `\` is a legal filename character on unix, so a raw target may contain one. It + // must not be treated as a separator, and must not trip + // `join_path`'s debug assertion. + let backslash_path = root_path.join("sub/link-backslash")?; + let backslash = fs.read_link(backslash_path.clone()).await?; + assert_eq!( + *backslash, + LinkContent::Link { + target: LinkTarget::Relative { + raw: rcstr!("a\\b.txt"), + resolved: root_path.join("sub/a\\b.txt")?, + }, + } + ); + + // A relative target that stays within the root throughout is still fine. + let inside_path = root_path.join("sub/link-inside")?; + let inside = fs.read_link(inside_path.clone()).await?; + let LinkContent::Link { target } = &*inside else { + anyhow::bail!("expected a valid link, got {inside:?}"); + }; + assert_eq!( + *target, + LinkTarget::Relative { + raw: rcstr!("../root.txt"), + resolved: root_path.join("root.txt")?, + } + ); + assert_eq!(target.target_type().await?, FileSystemEntryType::File,); + + Ok(()) + } + + let tt = turbo_tasks::TurboTasks::new(TurboTasksBackend::new( + BackendOptions::default(), + noop_backing_storage(), + )); + tt.run_once(async move { + let fs = disk_file_system_operation(root) + .resolve() + .strongly_consistent() + .await?; + let root_path = disk_file_system_root(fs); + + assert_operation(fs, root_path) + .read_strongly_consistent() + .await?; + + anyhow::Ok(()) + }) + .await + .unwrap(); + + tt.stop_and_wait().await; + } + #[cfg(unix)] #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_read_relative_symlink() { @@ -1896,14 +2180,19 @@ mod tests { assert_eq!( *via_alias, LinkContent::Link { - target: rcstr!("foo.txt"), - link_type: LinkType::ABSOLUTE, + target: LinkTarget::Absolute { + resolved: root_path.join("foo.txt")?, + }, } ); // link-outside -> /outside.txt (outside of the fs root) let outside = fs.read_link(root_path.join("link-outside")?).await?; - assert_eq!(*outside, LinkContent::Invalid); + assert!(matches!( + &*outside, + LinkContent::Invalid { reason} + if reason.contains("leaves the filesystem root") + )); Ok(()) } @@ -1958,9 +2247,9 @@ mod tests { async move { fs.write_link( symlink_path, - LinkContent::Link { - target, - link_type: LinkType::DIRECTORY, + WriteLinkContent { + target: WriteLinkTarget::Relative(target), + target_type: WriteLinkTargetType::DirectoryOrJunctionPoint, } .cell(), ) diff --git a/turbopack/crates/turbo-tasks-fs/src/embed/fs.rs b/turbopack/crates/turbo-tasks-fs/src/embed/fs.rs index 12136726ab6f..bfec3e4e6886 100644 --- a/turbopack/crates/turbo-tasks-fs/src/embed/fs.rs +++ b/turbopack/crates/turbo-tasks-fs/src/embed/fs.rs @@ -1,12 +1,12 @@ use anyhow::{Result, bail}; use auto_hash_map::AutoMap; use include_dir::{Dir, DirEntry}; -use turbo_rcstr::RcStr; +use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ValueToString, Vc}; use crate::{ File, FileContent, FileMeta, FileSystem, FileSystemPath, LinkContent, RawDirectoryContent, - RawDirectoryEntry, + RawDirectoryEntry, WriteLinkContent, }; #[derive(ValueToString)] @@ -38,7 +38,15 @@ impl FileSystem for EmbeddedFileSystem { #[turbo_tasks::function] fn read_link(&self, _path: FileSystemPath) -> Vc { - LinkContent::NotFound.cell() + LinkContent::Invalid { + reason: rcstr!("the filesystem does not support symbolic links"), + } + .cell() + } + + #[turbo_tasks::function] + fn is_junction_point(&self, _path: FileSystemPath) -> Vc { + Vc::cell(false) } #[turbo_tasks::function] @@ -77,7 +85,7 @@ impl FileSystem for EmbeddedFileSystem { } #[turbo_tasks::function] - fn write_link(&self, _path: FileSystemPath, _target: Vc) -> Result> { + fn write_link(&self, _path: FileSystemPath, _target: Vc) -> Result> { bail!("Writing is not possible to the embedded filesystem") } diff --git a/turbopack/crates/turbo-tasks-fs/src/lib.rs b/turbopack/crates/turbo-tasks-fs/src/lib.rs index 93e748ee4502..a844fe78e0f1 100644 --- a/turbopack/crates/turbo-tasks-fs/src/lib.rs +++ b/turbopack/crates/turbo-tasks-fs/src/lib.rs @@ -33,7 +33,8 @@ pub mod source_context; pub mod util; pub(crate) mod virtual_fs; mod watcher; -mod windows; +#[cfg(windows)] +pub mod windows; use std::{fmt::Debug, fs::FileType, path::PathBuf}; @@ -53,7 +54,8 @@ pub(crate) use crate::{ pub use crate::{ content::{ File, FileContent, FileJsonContent, FileLine, FileLinesContent, FileMeta, LinkContent, - LinkType, Permissions, PersistedFileContent, + LinkTarget, Permissions, PersistedFileContent, WriteLinkContent, WriteLinkTarget, + WriteLinkTargetType, }, disk::{DiskFileSystem, canonicalize_to_rcstr, validate_path_length}, null_fs::NullFileSystem, @@ -61,7 +63,6 @@ pub use crate::{ read_glob::ReadGlobResult, virtual_fs::VirtualFileSystem, watcher::{DiskWatcherConfig, DiskWatcherPathMatcher, DiskWatcherRecursiveMode}, - windows::to_verbatim_with_case_folded_disk, }; #[turbo_tasks::value_trait] @@ -73,23 +74,20 @@ pub trait FileSystem: ValueToString { } #[turbo_tasks::function] fn read(self: Vc, fs_path: FileSystemPath) -> Vc; - /// Reads the target of a symbolic link (or of a junction point on Windows). - /// - /// The base of the returned [`LinkContent::Link`] `target` depends on the link's - /// [`LinkType`]: root-relative and normalized for [`LinkType::ABSOLUTE`] links, or the raw - /// link-relative on-disk value otherwise. - /// - /// Returns [`LinkContent::Invalid`] if the target points outside of the filesystem root, and - /// [`LinkContent::NotFound`] if `fs_path` doesn't exist or isn't a link. + /// Reads the target of a symbolic link (or junction point on Windows). See [`LinkContent`]. #[turbo_tasks::function] fn read_link(self: Vc, fs_path: FileSystemPath) -> Vc; + /// Returns whether a symbolic link is a junction point on Windows. Always `false` on all other + /// platforms. + #[turbo_tasks::function] + fn is_junction_point(self: Vc, fs_path: FileSystemPath) -> Vc; #[turbo_tasks::function] fn raw_read_dir(self: Vc, fs_path: FileSystemPath) -> Vc; #[turbo_tasks::function] fn write(self: Vc, fs_path: FileSystemPath, content: Vc) -> Vc<()>; - /// See [`FileSystemPath::write_symbolic_link_dir`]. + /// Creates a symbolic link. See [`WriteLinkContent`] and [`FileSystemPath::write_link`]. #[turbo_tasks::function] - fn write_link(self: Vc, fs_path: FileSystemPath, target: Vc) -> Vc<()>; + fn write_link(self: Vc, fs_path: FileSystemPath, target: Vc) -> Vc<()>; #[turbo_tasks::function] fn metadata(self: Vc, fs_path: FileSystemPath) -> Vc; } diff --git a/turbopack/crates/turbo-tasks-fs/src/null_fs.rs b/turbopack/crates/turbo-tasks-fs/src/null_fs.rs index ab0266fcd34c..345193ffd08a 100644 --- a/turbopack/crates/turbo-tasks-fs/src/null_fs.rs +++ b/turbopack/crates/turbo-tasks-fs/src/null_fs.rs @@ -1,8 +1,12 @@ //! [`NullFileSystem`], a filesystem where every path is empty/not-found. +use turbo_rcstr::rcstr; use turbo_tasks::{ValueToString, Vc}; -use crate::{FileContent, FileMeta, FileSystem, FileSystemPath, LinkContent, RawDirectoryContent}; +use crate::{ + FileContent, FileMeta, FileSystem, FileSystemPath, LinkContent, RawDirectoryContent, + WriteLinkContent, +}; #[derive(ValueToString)] #[value_to_string("null")] @@ -18,7 +22,15 @@ impl FileSystem for NullFileSystem { #[turbo_tasks::function] fn read_link(&self, _fs_path: FileSystemPath) -> Vc { - LinkContent::NotFound.cell() + LinkContent::Invalid { + reason: rcstr!("the filesystem does not support symbolic links"), + } + .cell() + } + + #[turbo_tasks::function] + fn is_junction_point(&self, _fs_path: FileSystemPath) -> Vc { + Vc::cell(false) } #[turbo_tasks::function] @@ -30,7 +42,7 @@ impl FileSystem for NullFileSystem { fn write(&self, _fs_path: FileSystemPath, _content: Vc) {} #[turbo_tasks::function] - fn write_link(&self, _fs_path: FileSystemPath, _target: Vc) {} + fn write_link(&self, _fs_path: FileSystemPath, _target: Vc) {} #[turbo_tasks::function] fn metadata(&self, _fs_path: FileSystemPath) -> Vc { diff --git a/turbopack/crates/turbo-tasks-fs/src/path.rs b/turbopack/crates/turbo-tasks-fs/src/path.rs index cf9a88819072..1774296e7802 100644 --- a/turbopack/crates/turbo-tasks-fs/src/path.rs +++ b/turbopack/crates/turbo-tasks-fs/src/path.rs @@ -6,7 +6,7 @@ use anyhow::{Result, bail}; use auto_hash_map::{AutoMap, AutoSet}; use bincode::{Decode, Encode}; use indexmap::IndexSet; -use turbo_rcstr::RcStr; +use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ Completion, NonLocalValue, ResolvedVc, ValueToString, ValueToStringRef, Vc, trace::TraceRawVcs, turbobail, turbofmt, @@ -16,8 +16,8 @@ use turbo_unix_path::{get_parent_path, get_relative_path_to, join_path, normaliz use crate::{ DirectoryContent, DirectoryEntry, FileContent, FileJsonContent, FileMeta, FileSystem, - FileSystemEntryType, LinkContent, LinkType, RawDirectoryContent, RawDirectoryEntry, - ReadGlobResult, + FileSystemEntryType, LinkContent, RawDirectoryContent, RawDirectoryEntry, ReadGlobResult, + WriteLinkContent, glob::Glob, read_glob::{read_glob, track_glob}, }; @@ -391,6 +391,10 @@ impl FileSystemPath { self.fs().read_link(self.clone()) } + pub fn is_junction_point(&self) -> Vc { + self.fs().is_junction_point(self.clone()) + } + pub fn read_json(&self) -> Vc { self.fs().read(self.clone()).parse_json() } @@ -420,24 +424,20 @@ impl FileSystemPath { self.fs().write(self.clone(), content) } - /// Creates a symbolic link to a directory on *nix platforms, or a directory junction point on - /// Windows. + /// Creates a symbolic link on *nix platforms. On Windows, directory links are created as + /// junction points. Links to files on Windows are attempted to be created as symbolic links. /// /// [Windows supports symbolic links][windows-symlink], but they [can require elevated /// privileges][windows-privileges] if "developer mode" is not enabled, so we can't safely use /// them. Using junction points [matches the behavior of pnpm][pnpm-windows]. /// - /// This only supports directories because Windows junction points are incompatible with files. - /// To ensure compatibility, this will return an error if the target is a file, even on - /// platforms with full symlink support. - /// - /// **We intentionally do not provide an API for symlinking a file**, as we cannot support that - /// on all Windows configurations. + /// It is not recommended to create non-directory links, as this is not portable and will likely + /// fail on Windows. /// /// [windows-symlink]: https://blogs.windows.com/windowsdeveloper/2016/12/02/symlinks-windows-10/ /// [windows-privileges]: https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-10/security/threat-protection/security-policy-settings/create-symbolic-links /// [pnpm-windows]: https://pnpm.io/faq#does-it-work-on-windows - pub fn write_symbolic_link_dir(&self, target: Vc) -> Vc<()> { + pub fn write_link(&self, target: Vc) -> Vc<()> { self.fs().write_link(self.clone(), target) } @@ -511,8 +511,12 @@ pub struct RealPathResult { pub enum RealPathResultError { TooManySymlinks, CycleDetected, - Invalid, + /// The first symlink that resolution attempted to read no longer exists. NotFound, + /// Resolution failed after finding a symlink, or the symlink was invalid to begin with. + Invalid { + reason: RcStr, + }, } impl RealPathResultError { @@ -536,12 +540,14 @@ impl RealPathResultError { ); turbofmt!("Symlink {orig} is in a symlink loop: {symlinks_dbg}").await? } - RealPathResultError::Invalid => { - turbofmt!("Symlink {orig} is invalid, it points out of the filesystem root").await? + RealPathResultError::Invalid { reason } => { + turbofmt!("Symlink {orig} could not be resolved: {reason}").await? } RealPathResultError::NotFound => { - turbofmt!("Symlink {orig} is invalid, it points at a file that doesn't exist") - .await? + turbofmt!( + "Symlink {orig} could not be read because the symlink itself no longer exists" + ) + .await? } }) } @@ -605,6 +611,9 @@ async fn realpath_with_links(path: FileSystemPath) -> Result> let mut symlinks: IndexSet = IndexSet::new(); let mut visited: AutoSet = AutoSet::new(); let mut error = RealPathResultError::TooManySymlinks; + // Whether a previous iteration resolved a symbolic link, so `current_path` is now a link + // target rather than the path we were asked about. + let mut followed_link = false; // Pick some arbitrary symlink depth limit... similar to the ELOOP logic for realpath(3). // SYMLOOP_MAX is 40 for Linux: https://unix.stackexchange.com/q/721724 for _i in 0..40 { @@ -630,25 +639,44 @@ async fn realpath_with_links(path: FileSystemPath) -> Result> .rsplit_once('/') .map_or(current_path.path.as_str(), |(_, name)| name); symlinks.extend(parent_result.symlinks); - let parent_path = match parent_result.path_result { + match parent_result.path_result { Ok(path) => { if path != parent { current_path = path.join(basename)?; } - path } Err(parent_error) => { - error = parent_error; + error = match parent_error { + RealPathResultError::NotFound if !symlinks.is_empty() => { + RealPathResultError::Invalid { + reason: rcstr!( + "a symlink encountered while resolving the path no longer exists" + ), + } + } + error => error, + }; break; } - }; + } // use `get_type` before trying `read_link`, as there's a good chance of a cache hit on // `get_type`, and `read_link` isn't the common codepath. - if !matches!( - *current_path.get_type().await?, - FileSystemEntryType::Symlink - ) { + let entry_type = *current_path.get_type().await?; + if !matches!(entry_type, FileSystemEntryType::Symlink) { + // A link we followed points at something that doesn't exist. `read_link` can't detect + // this, as it never looks at the target, so a dangling link reads back as valid. + // + // Only report this for a link we actually followed: resolving a path that simply + // doesn't exist is not an error, it just resolves to itself. + if (followed_link || !symlinks.is_empty()) + && matches!(entry_type, FileSystemEntryType::NotFound) + { + error = RealPathResultError::Invalid { + reason: rcstr!("a symlink target does not exist"), + }; + break; + } return Ok(RealPathResult { path_result: Ok(current_path), symlinks: symlinks.into_iter().collect(), // convert set to vec @@ -656,22 +684,30 @@ async fn realpath_with_links(path: FileSystemPath) -> Result> .cell()); } - match &*current_path.read_link().await? { - LinkContent::Link { target, link_type } => { - symlinks.insert(current_path.clone()); - current_path = if link_type.contains(LinkType::ABSOLUTE) { - current_path.root().owned().await? - } else { - parent_path - } - .join(target)?; + let link_content = current_path.read_link().await?; + match &*link_content { + LinkContent::Link { target } => { + let target_path = target.file_system_path().clone(); + symlinks.insert(current_path); + current_path = target_path; + followed_link = true; } LinkContent::NotFound => { - error = RealPathResultError::NotFound; + error = if symlinks.is_empty() { + RealPathResultError::NotFound + } else { + RealPathResultError::Invalid { + reason: rcstr!( + "a symlink encountered while resolving the path no longer exists" + ), + } + }; break; } - LinkContent::Invalid => { - error = RealPathResultError::Invalid; + LinkContent::Invalid { reason } => { + error = RealPathResultError::Invalid { + reason: reason.clone(), + }; break; } } diff --git a/turbopack/crates/turbo-tasks-fs/src/read_glob.rs b/turbopack/crates/turbo-tasks-fs/src/read_glob.rs index c7a5bd459fc8..0e3e6ca7504c 100644 --- a/turbopack/crates/turbo-tasks-fs/src/read_glob.rs +++ b/turbopack/crates/turbo-tasks-fs/src/read_glob.rs @@ -5,7 +5,8 @@ use turbo_rcstr::RcStr; use turbo_tasks::{Completion, ResolvedVc, TryJoinIterExt, Vc, turbobail}; use crate::{ - DirectoryContent, DirectoryEntry, FileSystem, FileSystemPath, LinkContent, LinkType, glob::Glob, + DirectoryContent, DirectoryEntry, FileSystem, FileSystemEntryType, FileSystemPath, LinkContent, + glob::Glob, }; #[turbo_tasks::value] @@ -85,8 +86,11 @@ async fn read_glob_internal( handle_dir(&mut result, entry_path, segment, path).await?; } DirectoryEntry::Symlink(path) => { - if let LinkContent::Link { link_type, .. } = &*path.read_link().await? { - if link_type.contains(LinkType::DIRECTORY) { + // Skip links that leave the filesystem root. + let link_content = path.read_link().await?; + if let LinkContent::Link { target } = &*link_content { + if matches!(target.target_type().await?, FileSystemEntryType::Directory) + { // Ensure that there are no infinite link loops, but don't resolve resolve_symlink_safely(entry.clone()).await?; diff --git a/turbopack/crates/turbo-tasks-fs/src/virtual_fs.rs b/turbopack/crates/turbo-tasks-fs/src/virtual_fs.rs index 32a608d772df..8eaa4bb85180 100644 --- a/turbopack/crates/turbo-tasks-fs/src/virtual_fs.rs +++ b/turbopack/crates/turbo-tasks-fs/src/virtual_fs.rs @@ -2,7 +2,10 @@ use anyhow::{Result, bail}; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ValueToString, Vc}; -use crate::{FileContent, FileMeta, FileSystem, FileSystemPath, LinkContent, RawDirectoryContent}; +use crate::{ + FileContent, FileMeta, FileSystem, FileSystemPath, LinkContent, RawDirectoryContent, + WriteLinkContent, +}; #[derive(ValueToString)] #[value_to_string(self.name)] @@ -51,6 +54,11 @@ impl FileSystem for VirtualFileSystem { bail!("Reading is not possible on the virtual file system") } + #[turbo_tasks::function] + fn is_junction_point(&self, _fs_path: FileSystemPath) -> Result> { + bail!("Reading is not possible on the virtual file system") + } + #[turbo_tasks::function] fn raw_read_dir(&self, _fs_path: FileSystemPath) -> Result> { bail!("Reading is not possible on the virtual file system") @@ -62,7 +70,11 @@ impl FileSystem for VirtualFileSystem { } #[turbo_tasks::function] - fn write_link(&self, _fs_path: FileSystemPath, _target: Vc) -> Result> { + fn write_link( + &self, + _fs_path: FileSystemPath, + _target: Vc, + ) -> Result> { bail!("Writing is not possible on the virtual file system") } diff --git a/turbopack/crates/turbo-tasks-fs/src/windows.rs b/turbopack/crates/turbo-tasks-fs/src/windows.rs index a67a0af87518..e331ea27d814 100644 --- a/turbopack/crates/turbo-tasks-fs/src/windows.rs +++ b/turbopack/crates/turbo-tasks-fs/src/windows.rs @@ -1,4 +1,61 @@ -use std::path::{Path, PathBuf}; +use std::{ + ffi::OsString, + io, mem, + os::windows::ffi::{OsStrExt, OsStringExt}, + path::{Component, Path, PathBuf, Prefix}, + ptr::{null, null_mut}, +}; + +use omnipath::WinPathExt; +use windows_sys::Win32::{ + Foundation::{CloseHandle, INVALID_HANDLE_VALUE}, + Storage::FileSystem::{ + CreateFileW, FILE_ATTRIBUTE_TAG_INFO, FILE_FLAG_BACKUP_SEMANTICS, + FILE_FLAG_OPEN_REPARSE_POINT, FILE_SHARE_DELETE, FILE_SHARE_READ, FILE_SHARE_WRITE, + FileAttributeTagInfo, GetFileInformationByHandleEx, OPEN_EXISTING, + }, + System::SystemServices::IO_REPARSE_TAG_MOUNT_POINT, +}; + +pub(crate) fn is_link_junction_point(path: &Path) -> io::Result { + let path: Vec = path.as_os_str().encode_wide().chain(Some(0)).collect(); + let handle = unsafe { + CreateFileW( + path.as_ptr(), + 0, + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + null(), + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT, + null_mut(), + ) + }; + if handle == INVALID_HANDLE_VALUE { + return Err(io::Error::last_os_error()); + } + + let mut tag_info = FILE_ATTRIBUTE_TAG_INFO { + FileAttributes: 0, + ReparseTag: 0, + }; + let result = unsafe { + GetFileInformationByHandleEx( + handle, + FileAttributeTagInfo, + (&mut tag_info as *mut FILE_ATTRIBUTE_TAG_INFO).cast(), + mem::size_of::() as u32, + ) + }; + let error = if result == 0 { + Some(io::Error::last_os_error()) + } else { + None + }; + unsafe { + CloseHandle(handle); + } + error.map_or(Ok(tag_info.ReparseTag == IO_REPARSE_TAG_MOUNT_POINT), Err) +} /// Converts `path` into the verbatim, drive-letter-case-folded representation used internally by /// [`crate::DiskFileSystem`] on Windows. @@ -8,57 +65,41 @@ use std::path::{Path, PathBuf}; /// `canonicalize`, does not touch the disk — it resolves neither symlinks nor 8.3 short names. The /// drive letter is then upper-cased to match the form `GetFinalPathNameByHandle` (and thus /// `canonicalize`) produces. -/// -/// No-op on non-Windows platforms (returns `path` unchanged). -pub fn to_verbatim_with_case_folded_disk(path: &Path) -> std::io::Result { - #[cfg(windows)] - { - use std::{ - ffi::OsString, - os::windows::ffi::{OsStrExt, OsStringExt}, - path::{Component, Prefix}, - }; - - use omnipath::WinPathExt; +pub fn to_verbatim_with_case_folded_disk(path: &Path) -> io::Result { + // `to_verbatim` guarantees an absolute, verbatim path from here on, so there's no + // non-verbatim case to guard against below. + let path = path.to_verbatim()?; - // `to_verbatim` guarantees an absolute, verbatim path from here on, so there's no - // non-verbatim case to guard against below. - let path = path.to_verbatim()?; - - // Only `\\?\C:\...` (`VerbatimDisk`) paths carry a drive letter; verbatim UNC - // (`\\?\UNC\...`) and other verbatim device paths (`\\?\prefix`) don't, so there's nothing - // to case-fold for those. - // - // We can't read the letter from `VerbatimDisk(disk)` because that value is already - // normalized to uppercase, so it wouldn't tell us whether the underlying path needs - // rewriting. - let is_verbatim_disk = matches!( - path.components().next(), - Some(Component::Prefix(prefix)) if matches!(prefix.kind(), Prefix::VerbatimDisk(_)) - ); - if !is_verbatim_disk { - return Ok(path); - } - - // The layout is `\\?\C:\...`, so the drive letter is the 5th UTF-16 code unit (index 4) - // and is guaranteed to be a-z or A-Z. - if let Some(disk) = path.as_os_str().encode_wide().nth(4).map(|disk| disk as u8) - && disk.is_ascii_lowercase() - { - // we must encode/decode because OsString's internal encoding is opaque/unstable - let mut wide: Vec = path.as_os_str().encode_wide().collect(); - wide[4] = u16::from(disk.to_ascii_uppercase()); - return Ok(PathBuf::from(OsString::from_wide(&wide))); - } + // Only `\\?\C:\...` (`VerbatimDisk`) paths carry a drive letter; verbatim UNC + // (`\\?\UNC\...`) and other verbatim device paths (`\\?\prefix`) don't, so there's nothing + // to case-fold for those. + // + // We can't read the letter from `VerbatimDisk(disk)` because that value is already + // normalized to uppercase, so it wouldn't tell us whether the underlying path needs + // rewriting. + let is_verbatim_disk = matches!( + path.components().next(), + Some(Component::Prefix(prefix)) if matches!(prefix.kind(), Prefix::VerbatimDisk(_)) + ); + if !is_verbatim_disk { + return Ok(path); + } - Ok(path) + // The layout is `\\?\C:\...`, so the drive letter is the 5th UTF-16 code unit (index 4) + // and is guaranteed to be a-z or A-Z. + if let Some(disk) = path.as_os_str().encode_wide().nth(4).map(|disk| disk as u8) + && disk.is_ascii_lowercase() + { + // we must encode/decode because OsString's internal encoding is opaque/unstable + let mut wide: Vec = path.as_os_str().encode_wide().collect(); + wide[4] = u16::from(disk.to_ascii_uppercase()); + return Ok(PathBuf::from(OsString::from_wide(&wide))); } - #[cfg(not(windows))] - Ok(path.to_path_buf()) + Ok(path) } -#[cfg(all(test, windows))] +#[cfg(test)] mod tests { use std::path::Path; @@ -87,4 +128,18 @@ mod tests { r"\\?\UNC\server\share\foo" ); } + + #[test] + fn identifies_junction_points() { + let scratch = tempfile::tempdir().unwrap(); + let target = scratch.path().join("target"); + let link = scratch.path().join("link"); + let file = scratch.path().join("file"); + std::fs::create_dir(&target).unwrap(); + std::fs::File::create(&file).unwrap(); + std::os::windows::fs::junction_point(&target, &link).unwrap(); + + assert!(is_link_junction_point(&link).unwrap()); + assert!(!is_link_junction_point(&file).unwrap()); + } } diff --git a/turbopack/crates/turbo-tasks-fuzz/src/fs_watcher.rs b/turbopack/crates/turbo-tasks-fuzz/src/fs_watcher.rs index 212efda935a6..341212b11f0c 100644 --- a/turbopack/crates/turbo-tasks-fuzz/src/fs_watcher.rs +++ b/turbopack/crates/turbo-tasks-fuzz/src/fs_watcher.rs @@ -20,7 +20,8 @@ use turbo_tasks::{ }; use turbo_tasks_backend::{BackendOptions, TurboTasksBackend, noop_backing_storage}; use turbo_tasks_fs::{ - DiskFileSystem, File, FileContent, FileSystem, FileSystemPath, LinkContent, LinkType, + DiskFileSystem, File, FileContent, FileSystem, FileSystemPath, WriteLinkContent, + WriteLinkTarget, WriteLinkTargetType, }; // `read_or_write_all_paths_operation` always writes the sentinel values to files/symlinks. We can @@ -76,12 +77,12 @@ enum SymlinkMode { } impl SymlinkMode { - fn to_link_type(self) -> LinkType { + fn is_directory(self) -> bool { match self { - SymlinkMode::File => LinkType::empty(), - SymlinkMode::Directory => LinkType::DIRECTORY, + SymlinkMode::File => false, + SymlinkMode::Directory => true, #[cfg(windows)] - SymlinkMode::Junction => LinkType::DIRECTORY, + SymlinkMode::Junction => true, } } } @@ -139,8 +140,7 @@ pub async fn run(args: FsWatcher) -> anyhow::Result<()> { }; let track_writes = args.track_writes; let symlink_mode = args.symlinks; - let symlink_is_directory = - symlink_mode.map(|m| m.to_link_type().contains(LinkType::DIRECTORY)); + let symlink_is_directory = symlink_mode.map(SymlinkMode::is_directory); let effects_op = extract_effects_operation(read_or_write_all_paths_operation( invalidations.clone(), @@ -348,12 +348,14 @@ async fn write_link( ) -> anyhow::Result<()> { let path_str = path.path.clone(); invalidations.0.lock().unwrap().insert(path_str); - let link_type = if is_directory { - LinkType::DIRECTORY - } else { - LinkType::empty() + let link_content = WriteLinkContent { + target: WriteLinkTarget::Relative(target), + target_type: if is_directory { + WriteLinkTargetType::DirectoryOrJunctionPoint + } else { + WriteLinkTargetType::FileNonPortable + }, }; - let link_content = LinkContent::Link { target, link_type }; let _ = path .fs() .write_link(path.clone(), link_content.cell()) diff --git a/turbopack/crates/turbo-tasks-fuzz/src/symlink_stress.rs b/turbopack/crates/turbo-tasks-fuzz/src/symlink_stress.rs index 721fb225a9bf..fe82fd985653 100644 --- a/turbopack/crates/turbo-tasks-fuzz/src/symlink_stress.rs +++ b/turbopack/crates/turbo-tasks-fuzz/src/symlink_stress.rs @@ -13,7 +13,10 @@ use turbo_tasks::{ read_strongly_consistent_and_apply_effects, take_effects, }; use turbo_tasks_backend::{BackendOptions, TurboTasksBackend, noop_backing_storage}; -use turbo_tasks_fs::{DiskFileSystem, FileSystem, FileSystemPath, LinkContent, LinkType}; +use turbo_tasks_fs::{ + DiskFileSystem, FileSystem, FileSystemPath, WriteLinkContent, WriteLinkTarget, + WriteLinkTargetType, +}; #[derive(Args)] pub struct SymlinkStress { @@ -40,6 +43,15 @@ async fn extract_effects_operation(op: OperationVc<()>) -> anyhow::Result anyhow::Result<()> { + // Each batch writes `parallelism` distinct symlinks, so there must be enough to go around. + if args.parallelism > args.symlink_count { + anyhow::bail!( + "--parallelism ({}) must not exceed --symlink-count ({}), since a batch cannot write \ + the same symlink twice", + args.parallelism, + args.symlink_count, + ); + } std::fs::create_dir(&args.fs_root)?; let fs_root = args.fs_root.canonicalize()?; let _guard = FsCleanup { @@ -58,7 +70,12 @@ pub async fn run(args: SymlinkStress) -> anyhow::Result<()> { std::fs::create_dir(&symlinks_dir)?; let tt = turbo_tasks::TurboTasks::new(TurboTasksBackend::new( - BackendOptions::default(), + BackendOptions { + // `noop_backing_storage` is read-only, so asking to persist on shutdown (the default) + // just fails with "Cannot perform write operations on a read-only database". + storage_mode: None, + ..Default::default() + }, noop_backing_storage(), )); @@ -102,6 +119,8 @@ pub async fn run(args: SymlinkStress) -> anyhow::Result<()> { ); let mut rng = rand::rngs::SmallRng::from_rng(&mut rand::rng()); + // Reused across batches; each batch partially shuffles it to sample distinct symlinks. + let mut symlink_indices: Vec = (0..symlink_count).collect(); let mut total_writes: u64 = 0; let mut last_progress_writes: u64 = 0; let start_time = Instant::now(); @@ -113,13 +132,18 @@ pub async fn run(args: SymlinkStress) -> anyhow::Result<()> { break; } - // Generate random symlink updates for this batch - let updates: Vec<(usize, usize)> = (0..parallelism) - .map(|_| { - let symlink_idx = rng.random_range(0..symlink_count); - let target_idx = rng.random_range(0..target_count); - (symlink_idx, target_idx) - }) + // Pick `parallelism` *distinct* symlinks for this batch, via a partial Fisher-Yates + // shuffle. They must be distinct because writing two different targets to one path + // within a single operation is a conflicting effect: a usage error, not something to + // stress test. Sampling without replacement (rather than deduplicating) keeps the + // batch size exactly `parallelism`, so the reported concurrency is honest. + for i in 0..parallelism { + let j = rng.random_range(i..symlink_count); + symlink_indices.swap(i, j); + } + let updates: Vec<(usize, usize)> = symlink_indices[..parallelism] + .iter() + .map(|&symlink_idx| (symlink_idx, rng.random_range(0..target_count))) .collect(); // Execute writes in parallel via turbo-tasks @@ -169,17 +193,17 @@ pub async fn run(args: SymlinkStress) -> anyhow::Result<()> { Ok(()) } -#[turbo_tasks::function(operation)] +#[turbo_tasks::function(operation, root)] fn disk_file_system_operation(fs_root: RcStr) -> Vc { DiskFileSystem::new(rcstr!("project"), Vc::cell(fs_root)) } -#[turbo_tasks::function(operation)] +#[turbo_tasks::function(operation, root)] fn disk_file_system_root_operation(fs: ResolvedVc) -> Vc { fs.root() } -#[turbo_tasks::function(operation)] +#[turbo_tasks::function(operation, root)] async fn create_initial_symlinks_operation( symlinks_dir: FileSystemPath, count: usize, @@ -192,7 +216,7 @@ async fn create_initial_symlinks_operation( Ok(()) } -#[turbo_tasks::function(operation)] +#[turbo_tasks::function(operation, root)] async fn write_symlinks_batch_operation( symlinks_dir: FileSystemPath, updates: Vec<(usize, usize)>, @@ -215,9 +239,9 @@ async fn write_symlink( target: RcStr, ) -> anyhow::Result<()> { let symlink_path = symlinks_dir.join(&symlink_idx.to_string())?; - let link_content = LinkContent::Link { - target, - link_type: LinkType::DIRECTORY, + let link_content = WriteLinkContent { + target: WriteLinkTarget::Relative(target), + target_type: WriteLinkTargetType::DirectoryOrJunctionPoint, }; symlink_path .fs() diff --git a/turbopack/crates/turbo-tasks-macros/src/derive/deterministic_hash_macro.rs b/turbopack/crates/turbo-tasks-macros/src/derive/deterministic_hash_macro.rs index 47811277aa07..3221ce225c4d 100644 --- a/turbopack/crates/turbo-tasks-macros/src/derive/deterministic_hash_macro.rs +++ b/turbopack/crates/turbo-tasks-macros/src/derive/deterministic_hash_macro.rs @@ -10,28 +10,27 @@ use crate::expand::{generate_exhaustive_destructuring, match_expansion}; /// /// This requires that every contained value also implement `DeterministicHash`. pub fn derive_deterministic_hash(input: TokenStream) -> TokenStream { - let derive_input = parse_macro_input!(input as DeriveInput); - - let ident = &derive_input.ident; - let match_hash = match_expansion( + let DeriveInput { ident, - &derive_input.data, - &hash_named, - &hash_unnamed, - &hash_unit, - ); - let discriminant = match derive_input.data { + generics, + data, + .. + } = parse_macro_input!(input as DeriveInput); + + let match_hash = match_expansion(&ident, &data, &hash_named, &hash_unnamed, &hash_unit); + let discriminant = match data { Data::Enum(_) => { quote! { - turbo_tasks_hash::DeterministicHash::deterministic_hash(&std::mem::discriminant(self), __state__); + ::turbo_tasks_hash::DeterministicHash::deterministic_hash(&std::mem::discriminant(self), __state__); } } _ => quote! {}, }; + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); quote! { #[automatically_derived] - impl turbo_tasks_hash::DeterministicHash for #ident { + impl #impl_generics ::turbo_tasks_hash::DeterministicHash for #ident #ty_generics #where_clause { fn deterministic_hash(&self, __state__: &mut H) { #discriminant #match_hash diff --git a/turbopack/crates/turbo-unix-path/src/lib.rs b/turbopack/crates/turbo-unix-path/src/lib.rs index 64dfac9a1c72..889f0009e739 100644 --- a/turbopack/crates/turbo-unix-path/src/lib.rs +++ b/turbopack/crates/turbo-unix-path/src/lib.rs @@ -6,11 +6,11 @@ use std::borrow::Cow; /// directory separators with forward slashes on Windows. #[inline] pub fn sys_to_unix(path: &str) -> Cow<'_, str> { - #[cfg(not(target_family = "windows"))] + #[cfg(not(windows))] { Cow::from(path) } - #[cfg(target_family = "windows")] + #[cfg(windows)] { Cow::Owned(path.replace(std::path::MAIN_SEPARATOR_STR, "/")) } @@ -20,11 +20,11 @@ pub fn sys_to_unix(path: &str) -> Cow<'_, str> { /// slash directory separators with backslashes on Windows. #[inline] pub fn unix_to_sys(path: &str) -> Cow<'_, str> { - #[cfg(not(target_family = "windows"))] + #[cfg(not(windows))] { Cow::from(path) } - #[cfg(target_family = "windows")] + #[cfg(windows)] { Cow::Owned(path.replace('/', std::path::MAIN_SEPARATOR_STR)) } @@ -36,10 +36,8 @@ pub fn unix_to_sys(path: &str) -> Cow<'_, str> { /// see also [normalize_path] for normalization. /// Returns `None` if the joined path would leave the filesystem root. pub fn join_path(fs_path: &str, join: &str) -> Option { - // Paths that we join are written as source code (eg, `join_path(fs_path, "foo/bar.js")`) and - // it's expected that they will never contain a backslash. debug_assert!( - !join.contains('\\'), + !cfg!(windows) || !join.contains('\\'), "joined path {join} must not contain a Windows directory '\\', it must be normalized to \ Unix '/'" ); diff --git a/turbopack/crates/turbopack-core/src/asset.rs b/turbopack/crates/turbopack-core/src/asset.rs index 860dd6bf169a..bb9801924f0f 100644 --- a/turbopack/crates/turbopack-core/src/asset.rs +++ b/turbopack/crates/turbopack-core/src/asset.rs @@ -2,7 +2,7 @@ use anyhow::Result; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, Vc}; use turbo_tasks_fs::{ - FileContent, FileJsonContent, FileLinesContent, FileSystemPath, LinkContent, LinkType, + FileContent, FileJsonContent, FileLinesContent, FileSystemPath, WriteLinkContent, }; use turbo_tasks_hash::{HashAlgorithm, deterministic_hash}; @@ -52,10 +52,8 @@ pub trait Asset { #[derive(Clone)] pub enum AssetContent { File(ResolvedVc), - // for the relative link, the target is raw value read from the link - // for the absolute link, the target is stripped of the root path while reading - // See [LinkContent::Link] for more details. - Redirect { target: RcStr, link_type: LinkType }, + /// A symbolic link. See [`WriteLinkContent`] for how it is written. + Redirect(WriteLinkContent), } #[turbo_tasks::value_impl] @@ -69,7 +67,7 @@ impl AssetContent { pub fn parse_json(&self) -> Vc { match self { AssetContent::File(content) => content.parse_json(), - AssetContent::Redirect { .. } => { + AssetContent::Redirect(..) => { FileJsonContent::unparsable(rcstr!("a redirect can't be parsed as json")).cell() } } @@ -79,7 +77,7 @@ impl AssetContent { pub fn file_content(&self) -> Vc { match self { AssetContent::File(content) => **content, - AssetContent::Redirect { .. } => FileContent::NotFound.cell(), + AssetContent::Redirect(..) => FileContent::NotFound.cell(), } } @@ -87,7 +85,7 @@ impl AssetContent { pub fn lines(&self) -> Vc { match self { AssetContent::File(content) => content.lines(), - AssetContent::Redirect { .. } => FileLinesContent::Unparsable.cell(), + AssetContent::Redirect(..) => FileLinesContent::Unparsable.cell(), } } @@ -95,7 +93,7 @@ impl AssetContent { pub fn len(&self) -> Vc> { match self { AssetContent::File(content) => content.len(), - AssetContent::Redirect { .. } => Vc::cell(None), + AssetContent::Redirect(..) => Vc::cell(None), } } @@ -103,7 +101,7 @@ impl AssetContent { pub fn parse_json_with_comments(&self) -> Vc { match self { AssetContent::File(content) => content.parse_json_with_comments(), - AssetContent::Redirect { .. } => { + AssetContent::Redirect(..) => { FileJsonContent::unparsable(rcstr!("a redirect can't be parsed as json")).cell() } } @@ -115,16 +113,10 @@ impl AssetContent { AssetContent::File(file) => { path.write(**file).as_side_effect().await?; } - AssetContent::Redirect { target, link_type } => { - path.write_symbolic_link_dir( - LinkContent::Link { - target: target.clone(), - link_type: *link_type, - } - .cell(), - ) - .as_side_effect() - .await?; + AssetContent::Redirect(content) => { + path.write_link(content.clone().cell()) + .as_side_effect() + .await?; } } Ok(()) @@ -134,9 +126,9 @@ impl AssetContent { pub async fn hash(&self, salt: Vc, algorithm: HashAlgorithm) -> Result> { Ok(match self { AssetContent::File(content) => content.hash(salt, algorithm), - AssetContent::Redirect { target, link_type } => Vc::cell(RcStr::from( + AssetContent::Redirect(content) => Vc::cell(RcStr::from( // no_hash_salt - deterministic_hash(&salt.await?, (target, link_type), algorithm), + deterministic_hash(&salt.await?, content, algorithm), )), }) } @@ -154,7 +146,7 @@ impl AssetContent { ) -> Result>> { match self { AssetContent::File(content) => Ok(content.content_hash(salt, algorithm)), - AssetContent::Redirect { .. } => Ok(Vc::cell(None)), + AssetContent::Redirect(..) => Ok(Vc::cell(None)), } } } diff --git a/turbopack/crates/turbopack-core/src/file_source.rs b/turbopack/crates/turbopack-core/src/file_source.rs index 7d6af4ebbdb8..4e027051d859 100644 --- a/turbopack/crates/turbopack-core/src/file_source.rs +++ b/turbopack/crates/turbopack-core/src/file_source.rs @@ -1,7 +1,10 @@ use anyhow::{Result, bail}; use turbo_rcstr::RcStr; use turbo_tasks::Vc; -use turbo_tasks_fs::{FileContent, FileSystemEntryType, FileSystemPath, LinkContent}; +use turbo_tasks_fs::{ + FileContent, FileSystemEntryType, FileSystemPath, LinkContent, LinkTarget, WriteLinkContent, + WriteLinkTarget, WriteLinkTargetType, +}; use crate::{ asset::{Asset, AssetContent}, @@ -66,11 +69,31 @@ impl Asset for FileSource { let file_type = &*self.path.get_type().await?; match file_type { FileSystemEntryType::Symlink => match &*self.path.read_link().await? { - LinkContent::Link { target, link_type } => Ok(AssetContent::Redirect { - target: target.clone(), - link_type: *link_type, + LinkContent::Link { target } => { + let write_target = match target { + LinkTarget::Absolute { resolved } => { + WriteLinkTarget::Absolute(resolved.path.clone()) + } + LinkTarget::Relative { raw, .. } => WriteLinkTarget::Relative(raw.clone()), + }; + let target_fs_path = target.file_system_path(); + let write_target_type = match *target_fs_path.get_type().await? { + FileSystemEntryType::Directory => { + WriteLinkTargetType::DirectoryOrJunctionPoint + } + FileSystemEntryType::Symlink + if *target_fs_path.is_junction_point().await? => + { + WriteLinkTargetType::DirectoryOrJunctionPoint + } + _ => WriteLinkTargetType::FileNonPortable, + }; + Ok(AssetContent::Redirect(WriteLinkContent { + target: write_target, + target_type: write_target_type, + }) + .cell()) } - .cell()), _ => bail!("Invalid symlink"), }, FileSystemEntryType::File => { diff --git a/turbopack/crates/turbopack-core/src/introspect/utils.rs b/turbopack/crates/turbopack-core/src/introspect/utils.rs index f20ffaaf88a0..30fed8d7bc2f 100644 --- a/turbopack/crates/turbopack-core/src/introspect/utils.rs +++ b/turbopack/crates/turbopack-core/src/introspect/utils.rs @@ -54,9 +54,7 @@ pub async fn content_to_details(content: Vc) -> Result> } FileContent::NotFound => Vc::cell(rcstr!("not found")), }, - AssetContent::Redirect { target, link_type } => { - Vc::cell(format!("redirect to {target} with type {link_type:?}").into()) - } + AssetContent::Redirect(content) => Vc::cell(format!("redirect to {content:?}").into()), }) } diff --git a/turbopack/crates/turbopack-core/src/resolve/pattern.rs b/turbopack/crates/turbopack-core/src/resolve/pattern.rs index 8c73dd246369..5f73e5e41ebe 100644 --- a/turbopack/crates/turbopack-core/src/resolve/pattern.rs +++ b/turbopack/crates/turbopack-core/src/resolve/pattern.rs @@ -14,7 +14,7 @@ use turbo_tasks::{ NonLocalValue, TaskInput, ValueToString, Vc, debug::ValueDebugFormat, trace::TraceRawVcs, }; use turbo_tasks_fs::{ - FileSystemPath, LinkContent, LinkType, RawDirectoryContent, RawDirectoryEntry, + FileSystemEntryType, FileSystemPath, LinkContent, RawDirectoryContent, RawDirectoryEntry, }; use turbo_unix_path::normalize_path; @@ -1606,12 +1606,12 @@ pub async fn read_matches( )), RawDirectoryEntry::Symlink => { let fs_path = parent_fs_path.join(last_segment)?; - let LinkContent::Link { link_type, .. } = &*fs_path.read_link().await? - else { + let LinkContent::Link { target } = &*fs_path.read_link().await? else { continue; }; let path = concat(&prefix, str).into(); - if link_type.contains(LinkType::DIRECTORY) { + if matches!(target.target_type().await?, FileSystemEntryType::Directory) + { results.push((index, PatternMatch::Directory(path, fs_path))); } else { results.push((index, PatternMatch::File(path, fs_path))) @@ -1795,10 +1795,13 @@ pub async fn read_matches( } if let Some(pos) = pat.match_position(&prefix) { let fs_path = lookup_dir.join(key)?; - if let LinkContent::Link { link_type, .. } = + if let LinkContent::Link { target } = &*fs_path.read_link().await? { - if link_type.contains(LinkType::DIRECTORY) { + if matches!( + target.target_type().await?, + FileSystemEntryType::Directory + ) { results.push(( pos, PatternMatch::Directory( @@ -1817,9 +1820,12 @@ pub async fn read_matches( prefix.push('/'); if let Some(pos) = pat.match_position(&prefix) { let fs_path = lookup_dir.join(key)?; - if let LinkContent::Link { link_type, .. } = + if let LinkContent::Link { target } = &*fs_path.read_link().await? - && link_type.contains(LinkType::DIRECTORY) + && matches!( + target.target_type().await?, + FileSystemEntryType::Directory + ) { results.push(( pos, @@ -1829,9 +1835,12 @@ pub async fn read_matches( } if let Some(pos) = pat.could_match_position(&prefix) { let fs_path = lookup_dir.join(key)?; - if let LinkContent::Link { link_type, .. } = + if let LinkContent::Link { target } = &*fs_path.read_link().await? - && link_type.contains(LinkType::DIRECTORY) + && matches!( + target.target_type().await?, + FileSystemEntryType::Directory + ) { results.push(( pos, diff --git a/turbopack/crates/turbopack-core/src/server_fs.rs b/turbopack/crates/turbopack-core/src/server_fs.rs index d632525c4c56..bce2eb4d6aa6 100644 --- a/turbopack/crates/turbopack-core/src/server_fs.rs +++ b/turbopack/crates/turbopack-core/src/server_fs.rs @@ -2,6 +2,7 @@ use anyhow::{Result, bail}; use turbo_tasks::{ValueToString, Vc}; use turbo_tasks_fs::{ FileContent, FileMeta, FileSystem, FileSystemPath, LinkContent, RawDirectoryContent, + WriteLinkContent, }; #[turbo_tasks::value] @@ -29,6 +30,11 @@ impl FileSystem for ServerFileSystem { bail!("Reading is not possible from the marker filesystem for the server") } + #[turbo_tasks::function] + fn is_junction_point(&self, _fs_path: FileSystemPath) -> Result> { + bail!("Reading is not possible from the marker filesystem for the server") + } + #[turbo_tasks::function] fn raw_read_dir(&self, _fs_path: FileSystemPath) -> Result> { bail!("Reading is not possible from the marker filesystem for the server") @@ -40,7 +46,11 @@ impl FileSystem for ServerFileSystem { } #[turbo_tasks::function] - fn write_link(&self, _fs_path: FileSystemPath, _target: Vc) -> Result> { + fn write_link( + &self, + _fs_path: FileSystemPath, + _target: Vc, + ) -> Result> { bail!("Writing is not possible to the marker filesystem for the server") } diff --git a/turbopack/crates/turbopack-core/src/version.rs b/turbopack/crates/turbopack-core/src/version.rs index 89744f6f80ea..fc3fb8b8e66f 100644 --- a/turbopack/crates/turbopack-core/src/version.rs +++ b/turbopack/crates/turbopack-core/src/version.rs @@ -234,7 +234,7 @@ impl FileHashVersion { .context("file not found")?; Ok(Self::cell(FileHashVersion { hash })) } - AssetContent::Redirect { .. } => bail!("not a file"), + AssetContent::Redirect(..) => bail!("not a file"), } } } diff --git a/turbopack/crates/turbopack-css/src/process.rs b/turbopack/crates/turbopack-css/src/process.rs index 56bb638811b2..f581126ac764 100644 --- a/turbopack/crates/turbopack-css/src/process.rs +++ b/turbopack/crates/turbopack-css/src/process.rs @@ -378,7 +378,7 @@ pub async fn parse_css( let content = source.content(); let ident_str = &*source.ident().to_string().await?; Ok(match &*content.await? { - AssetContent::Redirect { .. } => ParseCssResult::Unparsable.cell(), + AssetContent::Redirect(..) => ParseCssResult::Unparsable.cell(), AssetContent::File(file_content) => match &*file_content.await? { FileContent::NotFound => ParseCssResult::NotFound.cell(), FileContent::Content(file) => match file.content().to_str() { diff --git a/turbopack/crates/turbopack-ecmascript/src/parse.rs b/turbopack/crates/turbopack-ecmascript/src/parse.rs index 7340c1098884..5895ecc9d0a6 100644 --- a/turbopack/crates/turbopack-ecmascript/src/parse.rs +++ b/turbopack/crates/turbopack-ecmascript/src/parse.rs @@ -372,7 +372,7 @@ async fn parse_internal( } } }, - AssetContent::Redirect { .. } => ParseResult::Unparsable { messages: None }.cell(), + AssetContent::Redirect(..) => ParseResult::Unparsable { messages: None }.cell(), }) } diff --git a/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs b/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs index 1e332aa4df4f..704b2d483ad7 100644 --- a/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs +++ b/turbopack/crates/turbopack-ecmascript/src/references/external_module.rs @@ -4,7 +4,10 @@ use anyhow::{Context, Result}; use bincode::{Decode, Encode}; use turbo_rcstr::{RcStr, rcstr}; use turbo_tasks::{ResolvedVc, TryJoinIterExt, ValueToStringRef, Vc, trace::TraceRawVcs}; -use turbo_tasks_fs::{FileSystem, FileSystemPath, LinkType, VirtualFileSystem, rope::RopeBuilder}; +use turbo_tasks_fs::{ + FileSystem, FileSystemPath, VirtualFileSystem, WriteLinkContent, WriteLinkTarget, + WriteLinkTargetType, rope::RopeBuilder, +}; use turbo_tasks_hash::{encode_hex, hash_xxh3_hash64}; use turbopack_core::{ asset::{Asset, AssetContent}, @@ -508,10 +511,10 @@ impl Asset for ExternalsSymlinkAsset { ) .into(); - Ok(AssetContent::Redirect { - target, - link_type: LinkType::DIRECTORY, - } + Ok(AssetContent::Redirect(WriteLinkContent { + target: WriteLinkTarget::Relative(target), + target_type: WriteLinkTargetType::DirectoryOrJunctionPoint, + }) .cell()) } } diff --git a/turbopack/crates/turbopack-test-utils/src/snapshot.rs b/turbopack/crates/turbopack-test-utils/src/snapshot.rs index 4d013fc7dff8..77926a6a23ac 100644 --- a/turbopack/crates/turbopack-test-utils/src/snapshot.rs +++ b/turbopack/crates/turbopack-test-utils/src/snapshot.rs @@ -169,9 +169,7 @@ async fn get_contents(file: Vc) -> Result> { } } }, - AssetContent::Redirect { target, link_type } => Some(format!( - "Redirect {{ target: {target}, link_type: {link_type:?} }}" - )), + AssetContent::Redirect(content) => Some(format!("Redirect {content:?}")), }) }