Skip to content

Commit

Permalink
feat(builtin): allow patching require in bootstrap scripts
Browse files Browse the repository at this point in the history
* export BAZEL_WORKSPACE, BAZEL_NODE_LINKER & BAZEL_NODE_PATCH_REQUIRE env variables from node launcher
* linker now users BAZEL_WORKSPACE & BAZEL_TARGET so that its runfiles resolve support works with non-test targets that don't have TEST_WORKSPACE & TEST_TARGET envs
* adds resolveWorkspaceRelative() & patchRequire() to linker runfiles resolve support. The former is useful if you want to resolve without specifying the workspace. The latter is useful for nodejs_binary bootstrap scripts that want to patch require via the linker (such as e2e/jasmine/jasmine_shared_env_bootstrap.js)
* tests targets can safely use TEST_TARGET & TEST_WORKSPACE

build: s
  • Loading branch information
gregmagolan committed Jan 3, 2020
1 parent 8a931d8 commit 842dfb4
Show file tree
Hide file tree
Showing 11 changed files with 137 additions and 48 deletions.
6 changes: 0 additions & 6 deletions e2e/jasmine/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down
6 changes: 6 additions & 0 deletions e2e/jasmine/jasmine_shared_env_bootstrap.js
Original file line number Diff line number Diff line change
@@ -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');
Expand Down
2 changes: 1 addition & 1 deletion internal/golden_file_test/bin.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion internal/linker/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ bzl_library(
)

# END-INTERNAL
exports_files(["index.js"])
exports_files([
"index.js",
"runfiles_helper.js",
])

filegroup(
name = "package_contents",
Expand Down
65 changes: 50 additions & 15 deletions internal/linker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand All @@ -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`.
*
Expand Down Expand Up @@ -424,9 +457,11 @@ Include as much of the build output as you can without disclosing anything confi
let target = '<package linking failed>';
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));
Expand Down
78 changes: 58 additions & 20 deletions internal/linker/link_node_modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,15 @@ async function resolveRoot(root: string|undefined, runfiles: Runfiles) {
export class Runfiles {
manifest: Map<string, string>|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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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`.
*
Expand Down Expand Up @@ -491,9 +527,11 @@ export async function main(args: string[], runfiles: Runfiles) {
let target: string = '<package linking failed>';
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));
Expand Down
1 change: 1 addition & 0 deletions internal/linker/runfiles_helper.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = require('./index.js').runfiles;
11 changes: 8 additions & 3 deletions internal/node/node.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand All @@ -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)

Expand All @@ -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),
}
Expand Down Expand Up @@ -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"),
Expand Down
7 changes: 7 additions & 0 deletions internal/node/node_launcher.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion internal/npm_install/test/check.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ ${prettyDiff}
Update the golden file:
bazel run ${process.env['BAZEL_TARGET']}.accept`);
bazel run ${process.env['TEST_TARGET']}.accept`);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/jasmine/src/jasmine_runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down

0 comments on commit 842dfb4

Please sign in to comment.