fix(@angular/build): rewrite paths from sandboxed execroots when running under Bazel - #33662
Conversation
…ing under Bazel rewriteForBazel only maps paths that fall inside BAZEL_BINDIR relative to JS_BINARY__EXECROOT. Locally sandboxed actions (darwin-sandbox, linux-sandbox) stage inputs as symlinks into a differently rooted copy of the execroot, so realpath resolves entry points to the unsandboxed execroot while the TypeScript program is keyed by sandbox paths. The compiler plugin then fails with "File 'main.ts' is missing from the TypeScript compilation". Derive the effective execroot from the working directory and re-anchor paths that resolve outside of it.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Bazel path rewriting logic to support sandboxed actions by deriving the effective execroot from the current working directory. Feedback was provided regarding potential cross-platform issues on Windows, where path separator mismatches (forward vs. backward slashes) between tools like Esbuild/TypeScript and Node's native path module could prevent correct path matching. A suggestion was made to normalize paths to the platform-native separator before processing.
| export function rewriteForBazel(path: string): string { | ||
| if (!bazelBinDirectory || !bazelExecRoot) { | ||
| return path; | ||
| } | ||
|
|
||
| const fromExecRoot = relative(bazelExecRoot, path); | ||
| const execRoot = effectiveExecRoot() ?? bazelExecRoot; | ||
|
|
||
| const fromExecRoot = relative(execRoot, path); | ||
| if (!fromExecRoot.startsWith('..')) { | ||
| return path; | ||
| } | ||
|
|
||
| const fromBinDirectory = relative(bazelBinDirectory, path); | ||
| if (fromBinDirectory.startsWith('..')) { | ||
| return path; | ||
| if (!fromBinDirectory.startsWith('..')) { | ||
| return join(execRoot, fromBinDirectory); | ||
| } | ||
|
|
||
| // A path that realpath resolved out of a symlink-staged sandbox into the | ||
| // unsandboxed execroot: re-anchor its execroot-relative tail. | ||
| const markerIndex = path.indexOf(execRootMarker); | ||
| if (markerIndex !== -1) { | ||
| const tail = path.slice(markerIndex + execRootMarker.length); | ||
| const workspaceEnd = tail.indexOf(sep); | ||
| if (workspaceEnd !== -1) { | ||
| return join(execRoot, tail.slice(workspaceEnd + 1)); | ||
| } | ||
| } | ||
|
|
||
| return join(bazelExecRoot, fromBinDirectory); | ||
| return path; | ||
| } |
There was a problem hiding this comment.
On Windows, tools like TypeScript and Esbuild often normalize internal paths to use forward slashes (/), whereas Node's process.cwd() and sep will use backward slashes (\\). This mismatch can cause path.indexOf(execRootMarker) and tail.indexOf(sep) to fail to find matches, preventing path rewriting on Windows.
To ensure robustness across platforms, we should normalize the input path, execRoot, and bazelBinDirectory to use the platform-native path separator (sep) before performing relative path calculations and string searches.
export function rewriteForBazel(path: string): string {
if (!bazelBinDirectory || !bazelExecRoot) {
return path;
}
const normalizedPath = sep === '\\' ? path.replace(/\//g, '\\') : path;
const rawExecRoot = effectiveExecRoot() ?? bazelExecRoot;
const execRoot = sep === '\\' ? rawExecRoot.replace(/\//g, '\\') : rawExecRoot;
const binDir = sep === '\\' ? bazelBinDirectory.replace(/\//g, '\\') : bazelBinDirectory;
const fromExecRoot = relative(execRoot, normalizedPath);
if (!fromExecRoot.startsWith('..')) {
return path;
}
const fromBinDirectory = relative(binDir, normalizedPath);
if (!fromBinDirectory.startsWith('..')) {
return join(execRoot, fromBinDirectory);
}
// A path that realpath resolved out of a symlink-staged sandbox into the
// unsandboxed execroot: re-anchor its execroot-relative tail.
const markerIndex = normalizedPath.indexOf(execRootMarker);
if (markerIndex !== -1) {
const tail = normalizedPath.slice(markerIndex + execRootMarker.length);
const workspaceEnd = tail.indexOf(sep);
if (workspaceEnd !== -1) {
return join(execRoot, tail.slice(workspaceEnd + 1));
}
}
return path;
}There was a problem hiding this comment.
The only call site wraps the argument in path.normalize() ( in compiler-plugin.ts), which on win32 converts forward slashes to backslashes, so the sep-based searches always see native separators. The new branch also only fires under Bazel's symlink-staging local sandboxes (darwin-sandbox/linux-sandbox), which don't exist on Windows.
If the marker ever missed anyway, the function falls through to the pre-existing behavior rather than regressing.
Let me know if this is insufficient and/or warrants more documentation in the code.
alan-agius4
added
action: review
alan-agius4
added
action: merge
2 participants
PR Type
Bugfix
What is the current behavior?
ng builddriven by rules_js (js_run_binary) fails under Bazel's local sandbox (macOS darwin-sandbox, also linux-sandbox) with:The sandbox stages inputs as symlinks into a copy of the execroot, so
realpathresolves the entry points to the unsandboxed execroot while the TypeScript program is keyed by sandbox paths.rewriteForBazel(261dbb3) only handles paths insideBAZEL_BINDIR/JS_BINARY__EXECROOT, which does not cover this case. Remote execution is unaffected because inputs are materialized as regular files.What is the new behavior?
The effective execroot is derived from the working directory, and paths that realpath resolved into the unsandboxed execroot are re-anchored onto it. Builds that previously required
--preserve-symlinksas a workaround now succeed sandboxed.Does this PR introduce a breaking change?
No