diff --git a/e2e/jasmine/BUILD.bazel b/e2e/jasmine/BUILD.bazel index 4c9ae78cb7..420e8695f6 100644 --- a/e2e/jasmine/BUILD.bazel +++ b/e2e/jasmine/BUILD.bazel @@ -5,17 +5,11 @@ jasmine_node_test( srcs = ["test.spec.js"], ) -# This requires the linker as jasmine_shared_env_bootstrap.js requires @bazel/jasmine -# which only works with the linker as the --require script doesn't get the require.resolve -# patches. jasmine_node_test( name = "shared_env_test", srcs = ["jasmine_shared_env_test.spec.js"], data = ["jasmine_shared_env_bootstrap.js"], - # TODO(gregmagolan): fix Windows error: Error: Cannot find module 'e2e_jasmine/shared_env_test_devmode_srcs.MF' - tags = ["fix-windows"], templated_args = [ - "--nobazel_patch_module_resolver", "--node_options=--require=$(rlocation $(location :jasmine_shared_env_bootstrap.js))", ], deps = [ diff --git a/e2e/jasmine/jasmine_shared_env_bootstrap.js b/e2e/jasmine/jasmine_shared_env_bootstrap.js index 27c1361141..4170531c58 100644 --- a/e2e/jasmine/jasmine_shared_env_bootstrap.js +++ b/e2e/jasmine/jasmine_shared_env_bootstrap.js @@ -1,3 +1,9 @@ +// bootstrap the bazel require patch since this bootstrap script is loaded with +// `--node_options=--require=$(rlocation $(location :jasmine_shared_env_bootstrap.js))` +if (process.env['BAZEL_NODE_RUNFILES_HELPER']) { + require(process.env['BAZEL_NODE_RUNFILES_HELPER']).patchRequire(); +} + global.foobar = 1; require('zone.js/dist/zone-node.js'); diff --git a/internal/golden_file_test/bin.js b/internal/golden_file_test/bin.js index d0980e6d94..279723d10e 100644 --- a/internal/golden_file_test/bin.js +++ b/internal/golden_file_test/bin.js @@ -29,7 +29,7 @@ ${prettyDiff} Update the golden file: bazel run ${debugBuild ? '--compilation_mode=dbg ' : ''}${ - process.env['BAZEL_TARGET'].replace(/_bin$/, '')}.accept + process.env['TEST_TARGET'].replace(/_bin$/, '')}.accept `); } else { throw new Error('unknown mode', mode); diff --git a/internal/linker/BUILD.bazel b/internal/linker/BUILD.bazel index f79e7fd1ed..08def5b3d1 100644 --- a/internal/linker/BUILD.bazel +++ b/internal/linker/BUILD.bazel @@ -20,7 +20,10 @@ bzl_library( ) # END-INTERNAL -exports_files(["index.js"]) +exports_files([ + "index.js", + "runfiles_helper.js", +]) filegroup( name = "package_contents", diff --git a/internal/linker/index.js b/internal/linker/index.js index 3b39440a98..cdbdfa55f2 100644 --- a/internal/linker/index.js +++ b/internal/linker/index.js @@ -165,16 +165,21 @@ Include as much of the build output as you can without disclosing anything confi If you want to test runfiles manifest behavior, add --spawn_strategy=standalone to the command line.`); } - const wksp = env['TEST_WORKSPACE']; - const target = env['TEST_TARGET']; - if (!!wksp && !!target) { - // //path/to:target -> //path/to - const pkg = target.split(':')[0]; - this.packagePath = path.posix.join(wksp, pkg); - } - // If under runfiles (and not unit testing) then don't add a bin to taget for bin links - this.noBin = !!env['TEST_TARGET'] && wksp !== 'build_bazel_rules_nodejs' && - target !== '//internal/linker/test:unit_tests'; + const wksp = env['BAZEL_WORKSPACE']; + if (!!wksp) { + this.workspace = wksp; + } + // If target is from an external workspace such as @npm//rollup/bin:rollup + // resolvePackageRelative is not supported since package is in an external + // workspace. + const target = env['BAZEL_TARGET']; + if (!!target && !target.startsWith('@')) { + // //path/to:target -> path/to + this.package = target.split(':')[0].replace(/^\/\//, ''); + } + // We can derive if the process is being run in the execroot + // if there is a bazel-out folder at the cwd. + this.execroot = existsSync('bazel-out'); } lookupDirectory(dir) { if (!this.manifest) @@ -225,11 +230,27 @@ Include as much of the build output as you can without disclosing anything confi } throw new Error(`could not resolve modulePath ${modulePath}`); } + resolveWorkspaceRelative(modulePath) { + if (!this.workspace) { + throw new Error('workspace could not be determined from the environment'); + } + return this.resolve(path.posix.join(this.workspace, modulePath)); + } resolvePackageRelative(modulePath) { - if (!this.packagePath) { - throw new Error('packagePath could not be determined from the environment'); + if (!this.workspace) { + throw new Error('workspace could not be determined from the environment'); + } + if (!this.package) { + throw new Error('package could not be determined from the environment'); + } + return this.resolve(path.posix.join(this.workspace, this.package, modulePath)); + } + patchRequire() { + const requirePatch = process.env['BAZEL_NODE_PATCH_REQUIRE']; + if (!requirePatch) { + throw new Error('require patch location could not be determined from the environment'); } - return this.resolve(path.posix.join(this.packagePath, modulePath)); + require(requirePatch); } } exports.Runfiles = Runfiles; @@ -249,6 +270,18 @@ Include as much of the build output as you can without disclosing anything confi } }); } + function existsSync(p) { + try { + fs.statSync(p); + return true; + } + catch (e) { + if (e.code === 'ENOENT') { + return false; + } + throw e; + } + } /** * Given a set of module aliases returns an array of recursive `LinkerTreeElement`. * @@ -424,9 +457,11 @@ Include as much of the build output as you can without disclosing anything confi let target = ''; switch (root) { case 'bin': + // If we are in the execroot then add the bin path to the target; otherwise + // we are in runfiles and the bin path should be omitted. // FIXME(#1196) - target = runfiles.noBin ? path.join(workspaceAbs, toWorkspaceDir(modulePath)) : - path.join(workspaceAbs, bin, toWorkspaceDir(modulePath)); + target = runfiles.execroot ? path.join(workspaceAbs, bin, toWorkspaceDir(modulePath)) : + path.join(workspaceAbs, toWorkspaceDir(modulePath)); break; case 'src': target = path.join(workspaceAbs, toWorkspaceDir(modulePath)); diff --git a/internal/linker/link_node_modules.ts b/internal/linker/link_node_modules.ts index 94b51524fa..fd0f3da3df 100644 --- a/internal/linker/link_node_modules.ts +++ b/internal/linker/link_node_modules.ts @@ -126,12 +126,15 @@ async function resolveRoot(root: string|undefined, runfiles: Runfiles) { export class Runfiles { manifest: Map|undefined; dir: string|undefined; - noBin: boolean; + execroot: boolean; /** - * If the environment gives us enough hints, we can know the path to the package - * in the form workspace_name/path/to/package + * If the environment gives us enough hints, we can know the workspace name */ - packagePath: string|undefined; + workspace: string|undefined; + /** + * If the environment gives us enough hints, we can know the package path + */ + package: string|undefined; constructor(env: typeof process.env) { // If Bazel sets a variable pointing to a runfiles manifest, @@ -159,18 +162,21 @@ export class Runfiles { If you want to test runfiles manifest behavior, add --spawn_strategy=standalone to the command line.`); } - - const wksp = env['TEST_WORKSPACE']; - const target = env['TEST_TARGET']; - if (!!wksp && !!target) { - // //path/to:target -> //path/to - const pkg = target.split(':')[0]; - this.packagePath = path.posix.join(wksp, pkg); + const wksp = env['BAZEL_WORKSPACE']; + if (!!wksp) { + this.workspace = wksp; } - - // If under runfiles (and not unit testing) then don't add a bin to taget for bin links - this.noBin = !!env['TEST_TARGET'] && wksp !== 'build_bazel_rules_nodejs' && - target !== '//internal/linker/test:unit_tests'; + // If target is from an external workspace such as @npm//rollup/bin:rollup + // resolvePackageRelative is not supported since package is in an external + // workspace. + const target = env['BAZEL_TARGET']; + if (!!target && !target.startsWith('@')) { + // //path/to:target -> path/to + this.package = target.split(':')[0].replace(/^\/\//, ''); + } + // We can derive if the process is being run in the execroot + // if there is a bazel-out folder at the cwd. + this.execroot = existsSync('bazel-out'); } lookupDirectory(dir: string): string|undefined { @@ -228,11 +234,29 @@ export class Runfiles { throw new Error(`could not resolve modulePath ${modulePath}`); } + resolveWorkspaceRelative(modulePath: string) { + if (!this.workspace) { + throw new Error('workspace could not be determined from the environment'); + } + return this.resolve(path.posix.join(this.workspace, modulePath)); + } + resolvePackageRelative(modulePath: string) { - if (!this.packagePath) { - throw new Error('packagePath could not be determined from the environment'); + if (!this.workspace) { + throw new Error('workspace could not be determined from the environment'); + } + if (!this.package) { + throw new Error('package could not be determined from the environment'); + } + return this.resolve(path.posix.join(this.workspace, this.package, modulePath)); + } + + patchRequire() { + const requirePatch = process.env['BAZEL_NODE_PATCH_REQUIRE']; + if (!requirePatch) { + throw new Error('require patch location could not be determined from the environment'); } - return this.resolve(path.posix.join(this.packagePath, modulePath)); + require(requirePatch); } } @@ -257,6 +281,18 @@ async function exists(p: string) { } } +function existsSync(p: string) { + try { + fs.statSync(p) + return true; + } catch (e) { + if (e.code === 'ENOENT') { + return false; + } + throw e; + } +} + /** * Given a set of module aliases returns an array of recursive `LinkerTreeElement`. * @@ -491,9 +527,11 @@ export async function main(args: string[], runfiles: Runfiles) { let target: string = ''; switch (root) { case 'bin': + // If we are in the execroot then add the bin path to the target; otherwise + // we are in runfiles and the bin path should be omitted. // FIXME(#1196) - target = runfiles.noBin ? path.join(workspaceAbs, toWorkspaceDir(modulePath)) : - path.join(workspaceAbs, bin, toWorkspaceDir(modulePath)); + target = runfiles.execroot ? path.join(workspaceAbs, bin, toWorkspaceDir(modulePath)) : + path.join(workspaceAbs, toWorkspaceDir(modulePath)); break; case 'src': target = path.join(workspaceAbs, toWorkspaceDir(modulePath)); diff --git a/internal/linker/runfiles_helper.js b/internal/linker/runfiles_helper.js new file mode 100644 index 0000000000..0a73f0a1b5 --- /dev/null +++ b/internal/linker/runfiles_helper.js @@ -0,0 +1 @@ +module.exports = require('./index.js').runfiles; diff --git a/internal/node/node.bzl b/internal/node/node.bzl index a2450c3532..dde8b81cc7 100644 --- a/internal/node/node.bzl +++ b/internal/node/node.bzl @@ -167,9 +167,8 @@ def _nodejs_binary_impl(ctx): _write_loader_script(ctx) - script_path = _to_manifest_path(ctx, ctx.outputs.loader) - env_vars = "export BAZEL_TARGET=%s\n" % ctx.label + env_vars += "export BAZEL_WORKSPACE=%s\n" % ctx.workspace_name for k in ctx.attr.configuration_env_vars + ctx.attr.default_env_vars: if k in ctx.var.keys(): env_vars += "export %s=\"%s\"\n" % (k, ctx.var[k]) @@ -191,6 +190,7 @@ def _nodejs_binary_impl(ctx): node_tool_files.extend(ctx.toolchains["@build_bazel_rules_nodejs//toolchains/node:toolchain_type"].nodeinfo.tool_files) node_tool_files.append(ctx.file._link_modules_script) + node_tool_files.append(ctx.file._runfiles_helper_script) node_tool_files.append(ctx.file._bazel_require_script) node_tool_files.append(node_modules_manifest) @@ -205,9 +205,10 @@ def _nodejs_binary_impl(ctx): "TEMPLATED_env_vars": env_vars, "TEMPLATED_expected_exit_code": str(expected_exit_code), "TEMPLATED_link_modules_script": _to_manifest_path(ctx, ctx.file._link_modules_script), - "TEMPLATED_loader_path": script_path, + "TEMPLATED_loader_path": _to_manifest_path(ctx, ctx.outputs.loader), "TEMPLATED_modules_manifest": _to_manifest_path(ctx, node_modules_manifest), "TEMPLATED_repository_args": _to_manifest_path(ctx, ctx.file._repository_args), + "TEMPLATED_runfiles_helper_script": _to_manifest_path(ctx, ctx.file._runfiles_helper_script), "TEMPLATED_script_path": _to_execroot_path(ctx, ctx.file.entry_point), "TEMPLATED_vendored_node": "" if is_builtin else strip_external(ctx.file._node.path), } @@ -449,6 +450,10 @@ jasmine_node_test( default = Label("@nodejs//:bin/node_repo_args.sh"), allow_single_file = True, ), + "_runfiles_helper_script": attr.label( + default = Label("//internal/linker:runfiles_helper.js"), + allow_single_file = True, + ), "_source_map_support_files": attr.label_list( default = [ Label("//third_party/github.com/buffer-from:contents"), diff --git a/internal/node/node_launcher.sh b/internal/node/node_launcher.sh index 6b7705073c..2f2008b28a 100644 --- a/internal/node/node_launcher.sh +++ b/internal/node/node_launcher.sh @@ -158,6 +158,13 @@ else fi fi +# Export the location of the runfiles helpers script +export BAZEL_NODE_RUNFILES_HELPER=$(rlocation "TEMPLATED_runfiles_helper_script") + +# Export the location of the loader script as it can be used to boostrap +# node require patch if needed +export BAZEL_NODE_PATCH_REQUIRE=$(rlocation "TEMPLATED_loader_path") + readonly repository_args=$(rlocation "TEMPLATED_repository_args") MAIN=$(rlocation "TEMPLATED_loader_path") readonly link_modules_script=$(rlocation "TEMPLATED_link_modules_script") diff --git a/internal/npm_install/test/check.js b/internal/npm_install/test/check.js index d4f0ebdcf9..c33ae79b13 100644 --- a/internal/npm_install/test/check.js +++ b/internal/npm_install/test/check.js @@ -40,7 +40,7 @@ ${prettyDiff} Update the golden file: - bazel run ${process.env['BAZEL_TARGET']}.accept`); + bazel run ${process.env['TEST_TARGET']}.accept`); } } } diff --git a/packages/jasmine/src/jasmine_runner.js b/packages/jasmine/src/jasmine_runner.js index 49c40b454e..aca07ebe53 100644 --- a/packages/jasmine/src/jasmine_runner.js +++ b/packages/jasmine/src/jasmine_runner.js @@ -191,7 +191,7 @@ function main(args) { // Special case! // To allow us to test sharding, always run the specs in the order they are declared if (process.env['TEST_WORKSPACE'] === 'build_bazel_rules_nodejs' && - process.env['BAZEL_TARGET'] === '//packages/jasmine/test:sharding_test') { + process.env['TEST_TARGET'] === '//packages/jasmine/test:sharding_test') { jrunner.randomizeTests(false); } }