From f1503da3dd64a61f4b3fdb35e0afe23899de74f8 Mon Sep 17 00:00:00 2001 From: Nick Schaap Date: Tue, 24 Jun 2025 13:50:35 -0400 Subject: [PATCH 1/4] feat: lazy initialize runfiles --- packages/runfiles/runfiles.ts | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/packages/runfiles/runfiles.ts b/packages/runfiles/runfiles.ts index 9f716baa97..a4dcb0f451 100644 --- a/packages/runfiles/runfiles.ts +++ b/packages/runfiles/runfiles.ts @@ -23,20 +23,26 @@ export class Runfiles { */ repoMappings: RepoMappings|undefined; - constructor(private _env = process.env) { + private _initialized = false; + + constructor(private _env = process.env) {} + + private _initialize() { + if (this._initialized) return; + this._initialized = true; // If Bazel sets a variable pointing to a runfiles manifest, // we'll always use it. // Note that this has a slight performance implication on Mac/Linux // where we could use the runfiles tree already laid out on disk // but this just costs one file read for the external npm/node_modules // and one for each first-party module, not one per file. - if (!!_env['RUNFILES_MANIFEST_FILE']) { - this.manifest = this.loadRunfilesManifest(_env['RUNFILES_MANIFEST_FILE']!); - } else if (!!_env['RUNFILES_DIR']) { - this.runfilesDir = path.resolve(_env['RUNFILES_DIR']!); + if (!!this._env['RUNFILES_MANIFEST_FILE']) { + this.manifest = this.loadRunfilesManifest(this._env['RUNFILES_MANIFEST_FILE']!); + } else if (!!this._env['RUNFILES_DIR']) { + this.runfilesDir = path.resolve(this._env['RUNFILES_DIR']!); this.repoMappings = this.parseRepoMapping(this.runfilesDir); - } else if (!!_env['RUNFILES']) { - this.runfilesDir = path.resolve(_env['RUNFILES']!); + } else if (!!this._env['RUNFILES']) { + this.runfilesDir = path.resolve(this._env['RUNFILES']!); this.repoMappings = this.parseRepoMapping(this.runfilesDir); } else { throw new Error( @@ -46,7 +52,7 @@ export class Runfiles { // Bazel sets RUNFILES_MANIFEST_ONLY=1. // When this happens, we need to read the manifest file to locate // inputs - if (_env['RUNFILES_MANIFEST_ONLY'] === '1' && !_env['RUNFILES_MANIFEST_FILE']) { + if (this._env['RUNFILES_MANIFEST_ONLY'] === '1' && !this._env['RUNFILES_MANIFEST_FILE']) { console.warn(`Workaround https://github.com/bazelbuild/bazel/issues/7994 RUNFILES_MANIFEST_FILE should have been set but wasn't. falling back to using runfiles symlinks. @@ -54,11 +60,11 @@ export class Runfiles { --spawn_strategy=standalone to the command line.`); } // Bazel starts actions with pwd=execroot/my_wksp or pwd=runfiles/my_wksp - this.workspace = _env['BAZEL_WORKSPACE'] || _env['JS_BINARY__WORKSPACE'] || undefined; + this.workspace = this._env['BAZEL_WORKSPACE'] || this._env['JS_BINARY__WORKSPACE'] || undefined; // If target is from an external workspace such as @npm//rollup/bin:rollup // resolvePackageRelative is not supported since package is in an external // workspace. - let target = _env['BAZEL_TARGET'] || _env['JS_BINARY__TARGET']; + let target = this._env['BAZEL_TARGET'] || this._env['JS_BINARY__TARGET']; if (!!target && !target.startsWith('@')) { // //path/to:target -> path/to this.package = target.split(':')[0].replace(/^\/\//, ''); @@ -111,6 +117,7 @@ export class Runfiles { * See https://github.com/bazelbuild/bazel/issues/3726 */ loadRunfilesManifest(manifestPath: string) { + this._initialize(); const runfilesEntries = new Map(); const input = fs.readFileSync(manifestPath, {encoding: 'utf-8'}); @@ -124,6 +131,7 @@ export class Runfiles { } parseRepoMapping(runfilesDir: string): RepoMappings | undefined { + this._initialize(); const repoMappingPath = path.join(runfilesDir, REPO_MAPPING_RLOCATION); if (!fs.existsSync(repoMappingPath)) { @@ -152,6 +160,7 @@ export class Runfiles { /** Resolves the given module path. */ resolve(modulePath: string, sourceRepo?: string): string { + this._initialize(); // Normalize path by converting to forward slashes and removing all trailing // forward slashes modulePath = modulePath.replace(/\\/g, '/').replace(/\/+$/g, '') @@ -188,6 +197,7 @@ export class Runfiles { /** Resolves the given path relative to the current Bazel workspace. */ resolveWorkspaceRelative(modulePath: string) { + this._initialize(); // Normalize path by converting to forward slashes and removing all trailing // forward slashes modulePath = modulePath.replace(/\\/g, '/').replace(/\/+$/g, '') @@ -200,6 +210,7 @@ export class Runfiles { /** Resolves the given path relative to the current Bazel package. */ resolvePackageRelative(modulePath: string) { + this._initialize(); // Normalize path by converting to forward slashes and removing all trailing // forward slashes modulePath = modulePath.replace(/\\/g, '/').replace(/\/+$/g, '') @@ -220,6 +231,7 @@ export class Runfiles { * @deprecated Use the runfile helpers directly instead. **/ patchRequire() { + this._initialize(); const requirePatch = this._env['BAZEL_NODE_PATCH_REQUIRE']; if (!requirePatch) { throw new Error('require patch location could not be determined from the environment'); From 6618a12cf0a9fee5a881b048477c4be2431b844b Mon Sep 17 00:00:00 2001 From: Nick Schaap Date: Tue, 24 Jun 2025 14:52:16 -0400 Subject: [PATCH 2/4] fix: only resolve initialization if it doesn't throw --- packages/runfiles/runfiles.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/runfiles/runfiles.ts b/packages/runfiles/runfiles.ts index a4dcb0f451..e12406d182 100644 --- a/packages/runfiles/runfiles.ts +++ b/packages/runfiles/runfiles.ts @@ -29,7 +29,6 @@ export class Runfiles { private _initialize() { if (this._initialized) return; - this._initialized = true; // If Bazel sets a variable pointing to a runfiles manifest, // we'll always use it. // Note that this has a slight performance implication on Mac/Linux @@ -69,6 +68,7 @@ export class Runfiles { // //path/to:target -> path/to this.package = target.split(':')[0].replace(/^\/\//, ''); } + this._initialized = true; } /** Resolves the given path from the runfile manifest. */ From 5d733c86e657dc2df2af3459842814ad0eca10f7 Mon Sep 17 00:00:00 2001 From: Nick Schaap Date: Tue, 24 Jun 2025 16:04:00 -0400 Subject: [PATCH 3/4] updates --- packages/runfiles/runfiles.ts | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/packages/runfiles/runfiles.ts b/packages/runfiles/runfiles.ts index e12406d182..324255d495 100644 --- a/packages/runfiles/runfiles.ts +++ b/packages/runfiles/runfiles.ts @@ -23,12 +23,9 @@ export class Runfiles { */ repoMappings: RepoMappings|undefined; - private _initialized = false; + private _runfilesResolutionError = false; - constructor(private _env = process.env) {} - - private _initialize() { - if (this._initialized) return; + constructor(private _env = process.env) { // If Bazel sets a variable pointing to a runfiles manifest, // we'll always use it. // Note that this has a slight performance implication on Mac/Linux @@ -44,8 +41,7 @@ export class Runfiles { this.runfilesDir = path.resolve(this._env['RUNFILES']!); this.repoMappings = this.parseRepoMapping(this.runfilesDir); } else { - throw new Error( - 'Every node program run under Bazel must have a $RUNFILES_DIR, $RUNFILES or $RUNFILES_MANIFEST_FILE environment variable'); + this._runfilesResolutionError = true; } // Under --noenable_runfiles (in particular on Windows) // Bazel sets RUNFILES_MANIFEST_ONLY=1. @@ -68,7 +64,13 @@ export class Runfiles { // //path/to:target -> path/to this.package = target.split(':')[0].replace(/^\/\//, ''); } - this._initialized = true; + } + + private _assertRunfilesResolved() { + if (this._runfilesResolutionError) { + throw new Error( + 'Every node program run under Bazel must have a $RUNFILES_DIR, $RUNFILES or $RUNFILES_MANIFEST_FILE environment variable'); + } } /** Resolves the given path from the runfile manifest. */ @@ -117,7 +119,6 @@ export class Runfiles { * See https://github.com/bazelbuild/bazel/issues/3726 */ loadRunfilesManifest(manifestPath: string) { - this._initialize(); const runfilesEntries = new Map(); const input = fs.readFileSync(manifestPath, {encoding: 'utf-8'}); @@ -131,7 +132,6 @@ export class Runfiles { } parseRepoMapping(runfilesDir: string): RepoMappings | undefined { - this._initialize(); const repoMappingPath = path.join(runfilesDir, REPO_MAPPING_RLOCATION); if (!fs.existsSync(repoMappingPath)) { @@ -160,7 +160,8 @@ export class Runfiles { /** Resolves the given module path. */ resolve(modulePath: string, sourceRepo?: string): string { - this._initialize(); + this._assertRunfilesResolved(); + // Normalize path by converting to forward slashes and removing all trailing // forward slashes modulePath = modulePath.replace(/\\/g, '/').replace(/\/+$/g, '') @@ -197,7 +198,6 @@ export class Runfiles { /** Resolves the given path relative to the current Bazel workspace. */ resolveWorkspaceRelative(modulePath: string) { - this._initialize(); // Normalize path by converting to forward slashes and removing all trailing // forward slashes modulePath = modulePath.replace(/\\/g, '/').replace(/\/+$/g, '') @@ -210,7 +210,6 @@ export class Runfiles { /** Resolves the given path relative to the current Bazel package. */ resolvePackageRelative(modulePath: string) { - this._initialize(); // Normalize path by converting to forward slashes and removing all trailing // forward slashes modulePath = modulePath.replace(/\\/g, '/').replace(/\/+$/g, '') @@ -231,7 +230,6 @@ export class Runfiles { * @deprecated Use the runfile helpers directly instead. **/ patchRequire() { - this._initialize(); const requirePatch = this._env['BAZEL_NODE_PATCH_REQUIRE']; if (!requirePatch) { throw new Error('require patch location could not be determined from the environment'); From 857967ce0e007102cbc668ffcf7fea47ad8e6a9c Mon Sep 17 00:00:00 2001 From: Nick Schaap Date: Tue, 24 Jun 2025 16:04:53 -0400 Subject: [PATCH 4/4] chore: pr feedback --- packages/runfiles/runfiles.ts | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/runfiles/runfiles.ts b/packages/runfiles/runfiles.ts index 324255d495..1d3a98bcd6 100644 --- a/packages/runfiles/runfiles.ts +++ b/packages/runfiles/runfiles.ts @@ -32,13 +32,13 @@ export class Runfiles { // where we could use the runfiles tree already laid out on disk // but this just costs one file read for the external npm/node_modules // and one for each first-party module, not one per file. - if (!!this._env['RUNFILES_MANIFEST_FILE']) { - this.manifest = this.loadRunfilesManifest(this._env['RUNFILES_MANIFEST_FILE']!); - } else if (!!this._env['RUNFILES_DIR']) { - this.runfilesDir = path.resolve(this._env['RUNFILES_DIR']!); + if (!!_env['RUNFILES_MANIFEST_FILE']) { + this.manifest = this.loadRunfilesManifest(_env['RUNFILES_MANIFEST_FILE']!); + } else if (!!_env['RUNFILES_DIR']) { + this.runfilesDir = path.resolve(_env['RUNFILES_DIR']!); this.repoMappings = this.parseRepoMapping(this.runfilesDir); - } else if (!!this._env['RUNFILES']) { - this.runfilesDir = path.resolve(this._env['RUNFILES']!); + } else if (!!_env['RUNFILES']) { + this.runfilesDir = path.resolve(_env['RUNFILES']!); this.repoMappings = this.parseRepoMapping(this.runfilesDir); } else { this._runfilesResolutionError = true; @@ -47,7 +47,7 @@ export class Runfiles { // Bazel sets RUNFILES_MANIFEST_ONLY=1. // When this happens, we need to read the manifest file to locate // inputs - if (this._env['RUNFILES_MANIFEST_ONLY'] === '1' && !this._env['RUNFILES_MANIFEST_FILE']) { + if (_env['RUNFILES_MANIFEST_ONLY'] === '1' && !_env['RUNFILES_MANIFEST_FILE']) { console.warn(`Workaround https://github.com/bazelbuild/bazel/issues/7994 RUNFILES_MANIFEST_FILE should have been set but wasn't. falling back to using runfiles symlinks. @@ -55,11 +55,11 @@ export class Runfiles { --spawn_strategy=standalone to the command line.`); } // Bazel starts actions with pwd=execroot/my_wksp or pwd=runfiles/my_wksp - this.workspace = this._env['BAZEL_WORKSPACE'] || this._env['JS_BINARY__WORKSPACE'] || undefined; + this.workspace = _env['BAZEL_WORKSPACE'] || _env['JS_BINARY__WORKSPACE'] || undefined; // If target is from an external workspace such as @npm//rollup/bin:rollup // resolvePackageRelative is not supported since package is in an external // workspace. - let target = this._env['BAZEL_TARGET'] || this._env['JS_BINARY__TARGET']; + let target = _env['BAZEL_TARGET'] || _env['JS_BINARY__TARGET']; if (!!target && !target.startsWith('@')) { // //path/to:target -> path/to this.package = target.split(':')[0].replace(/^\/\//, ''); @@ -108,7 +108,6 @@ export class Runfiles { return result; } - /** * The runfiles manifest maps from short_path * https://docs.bazel.build/versions/main/skylark/lib/File.html#short_path