Skip to content

Commit

Permalink
feat(builtin): wire linker/node-patches to npm-generated bin and test
Browse files Browse the repository at this point in the history
- nodejs_binary and nodejs_test will always produce a modules manifest needed by the linker
- if the program isn't run with a user-specific linker manifest, we use the static one as a default
- always pass the flag to opt-out of custom module resolver for generated rules
  • Loading branch information
alexeagle committed Dec 6, 2019
1 parent 68a8467 commit 395f63d
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 8 deletions.
4 changes: 4 additions & 0 deletions internal/node/node.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ load("//internal/common:expand_into_runfiles.bzl", "expand_location_into_runfile
load("//internal/common:module_mappings.bzl", "module_mappings_runtime_aspect")
load("//internal/common:path_utils.bzl", "strip_external")
load("//internal/common:windows_utils.bzl", "create_windows_native_launcher_script", "is_windows")
load("//internal/linker:link_node_modules.bzl", "write_node_modules_manifest")
load("//internal/node:node_repositories.bzl", "BUILT_IN_NODE_PLATFORMS")

def _trim_package_node_modules(package_name):
Expand Down Expand Up @@ -144,6 +145,7 @@ def _to_execroot_path(ctx, file):
return ("<ERROR> _to_execroot_path not yet implemented for " + file.path)

def _nodejs_binary_impl(ctx):
node_modules_manifest = write_node_modules_manifest(ctx)
node_modules = depset(ctx.files.node_modules)

# Also include files from npm fine grained deps as inputs.
Expand Down Expand Up @@ -198,6 +200,7 @@ def _nodejs_binary_impl(ctx):

node_tool_files.append(ctx.file._link_modules_script)
node_tool_files.append(ctx.file._bazel_require_script)
node_tool_files.append(node_modules_manifest)

if not ctx.outputs.templated_args_file:
templated_args = ctx.attr.templated_args
Expand Down Expand Up @@ -236,6 +239,7 @@ def _nodejs_binary_impl(ctx):
"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_modules_manifest": _to_manifest_path(ctx, node_modules_manifest),
"TEMPLATED_repository_args": _to_manifest_path(ctx, ctx.file._repository_args),
"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
4 changes: 4 additions & 0 deletions internal/node/node_launcher.sh
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@ fi

readonly repository_args=$(rlocation "TEMPLATED_repository_args")
MAIN=$(rlocation "TEMPLATED_loader_path")
# For programs which are called with bazel run or bazel test, there will be no additional runtime
# dependencies to link, so we use the default modules_manifest which has only the static dependencies
# of the binary itself
MODULES_MANIFEST=$(rlocation "TEMPLATED_modules_manifest")
readonly link_modules_script=$(rlocation "TEMPLATED_link_modules_script")
bazel_require_script=$(rlocation "TEMPLATED_bazel_require_script")

Expand Down
9 changes: 6 additions & 3 deletions internal/npm_install/generate_build_file.js
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,8 @@ nodejs_binary(
name = "${name}",
entry_point = "//:node_modules/${pkg._dir}/${path}",
install_source_map_support = False,
data = [${data.map(p => `"${p}"`).join(', ')}],${additionalAttributes(pkg, name)}
data = [${data.map(p => `"${p}"`).join(', ')}],
templated_args = ["--nobazel_patch_module_resolver"],${additionalAttributes(pkg, name)}
)
`;
}
Expand Down Expand Up @@ -982,7 +983,8 @@ def ${name.replace(/-/g, '_')}(**kwargs):
nodejs_binary(
entry_point = "@${WORKSPACE}//:node_modules/${pkg._dir}/${path}",
install_source_map_support = False,
data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),${additionalAttributes(pkg, name)}
data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),
templated_args = ["--nobazel_patch_module_resolver"],${additionalAttributes(pkg, name)}
**kwargs
)
Expand All @@ -991,7 +993,8 @@ def ${name.replace(/-/g, '_')}_test(**kwargs):
nodejs_test(
entry_point = "@${WORKSPACE}//:node_modules/${pkg._dir}/${path}",
install_source_map_support = False,
data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),${additionalAttributes(pkg, name)}
data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),
templated_args = ["--nobazel_patch_module_resolver"],${additionalAttributes(pkg, name)}
**kwargs
)
`;
Expand Down
11 changes: 6 additions & 5 deletions internal/npm_install/generate_build_file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1049,7 +1049,8 @@ nodejs_binary(
name = "${name}",
entry_point = "//:node_modules/${pkg._dir}/${path}",
install_source_map_support = False,
data = [${data.map(p => `"${p}"`).join(', ')}],${additionalAttributes(pkg, name)}
data = [${data.map(p => `"${p}"`).join(', ')}],
templated_args = ["--nobazel_patch_module_resolver"],${additionalAttributes(pkg, name)}
)
`;
}
Expand Down Expand Up @@ -1084,8 +1085,8 @@ def ${name.replace(/-/g, '_')}(**kwargs):
nodejs_binary(
entry_point = "@${WORKSPACE}//:node_modules/${pkg._dir}/${path}",
install_source_map_support = False,
data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),${
additionalAttributes(pkg, name)}
data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),
templated_args = ["--nobazel_patch_module_resolver"],${additionalAttributes(pkg, name)}
**kwargs
)
Expand All @@ -1094,8 +1095,8 @@ def ${name.replace(/-/g, '_')}_test(**kwargs):
nodejs_test(
entry_point = "@${WORKSPACE}//:node_modules/${pkg._dir}/${path}",
install_source_map_support = False,
data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),${
additionalAttributes(pkg, name)}
data = [${data.map(p => `"${p}"`).join(', ')}] + kwargs.pop("data", []),
templated_args = ["--nobazel_patch_module_resolver"],${additionalAttributes(pkg, name)}
**kwargs
)
`;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ nodejs_binary(
entry_point = "//:node_modules/@gregmagolan/test-a/@bin/test.js",
install_source_map_support = False,
data = ["//@gregmagolan/test-a:test-a"],
templated_args = ["--nobazel_patch_module_resolver"],
)
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ def test(**kwargs):
entry_point = "@fine_grained_goldens//:node_modules/@gregmagolan/test-a/@bin/test.js",
install_source_map_support = False,
data = ["@fine_grained_goldens//@gregmagolan/test-a:test-a"] + kwargs.pop("data", []),
templated_args = ["--nobazel_patch_module_resolver"],
**kwargs
)
def test_test(**kwargs):
nodejs_test(
entry_point = "@fine_grained_goldens//:node_modules/@gregmagolan/test-a/@bin/test.js",
install_source_map_support = False,
data = ["@fine_grained_goldens//@gregmagolan/test-a:test-a"] + kwargs.pop("data", []),
templated_args = ["--nobazel_patch_module_resolver"],
**kwargs
)
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ nodejs_binary(
entry_point = "//:node_modules/jasmine/bin/jasmine.js",
install_source_map_support = False,
data = ["//jasmine:jasmine"],
templated_args = ["--nobazel_patch_module_resolver"],
)
2 changes: 2 additions & 0 deletions internal/npm_install/test/golden/jasmine/index.bzl.golden
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,14 @@ def jasmine(**kwargs):
entry_point = "@fine_grained_goldens//:node_modules/jasmine/bin/jasmine.js",
install_source_map_support = False,
data = ["@fine_grained_goldens//jasmine:jasmine"] + kwargs.pop("data", []),
templated_args = ["--nobazel_patch_module_resolver"],
**kwargs
)
def jasmine_test(**kwargs):
nodejs_test(
entry_point = "@fine_grained_goldens//:node_modules/jasmine/bin/jasmine.js",
install_source_map_support = False,
data = ["@fine_grained_goldens//jasmine:jasmine"] + kwargs.pop("data", []),
templated_args = ["--nobazel_patch_module_resolver"],
**kwargs
)

0 comments on commit 395f63d

Please sign in to comment.