Skip to content

Commit

Permalink
fix(builtin): linker test should run program as an action (#1113)
Browse files Browse the repository at this point in the history
We were accidentally using runfiles resolution to read all user inputs, because we ran as an sh_test
  • Loading branch information
alexeagle authored Sep 10, 2019
1 parent 162e436 commit 7f0102e
Show file tree
Hide file tree
Showing 23 changed files with 760 additions and 295 deletions.
17 changes: 17 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,20 @@ import %workspace%/third_party/github.com/bazelbuild/bazel-toolchains/bazelrc/ba
# Remote instance, borrow the one used by Angular devs
build:remote --remote_instance_name=projects/internal-200822/instances/default_instance
build:remote --project_id=internal-200822

# To reproduce Windows issues where there is no runfiles symlink there
build:no-runfiles --noenable_runfiles
# workaround https://github.com/bazelbuild/bazel/issues/7994
build:no-runfiles --spawn_strategy=standalone
# This config is probably only used while debugging
build:no-runfiles --define=VERBOSE_LOG=1

# Docker Sandbox Mode
# Useful for troubleshooting Remote Build Execution problems
# See https://docs.bazel.build/versions/master/remote-execution-sandbox.html#prerequisites
build:docker-sandbox --spawn_strategy=docker --strategy=Javac=docker --genrule_strategy=docker
build:docker-sandbox --define=EXECUTOR=remote
build:docker-sandbox --experimental_docker_verbose
build:docker-sandbox --experimental_enable_docker_sandbox
# This is the same image used on BazelCI rbe_ubuntu1604 job
build:docker-sandbox --experimental_docker_image=gcr.io/cloud-marketplace/google/rbe-ubuntu16-04
5 changes: 4 additions & 1 deletion internal/js_library/js_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,10 @@ def write_amd_names_shim(actions, amd_names_shim, targets):

def _js_library(ctx):
return [
DefaultInfo(files = depset(ctx.files.srcs)),
DefaultInfo(
files = depset(ctx.files.srcs),
runfiles = ctx.runfiles(files = ctx.files.srcs),
),
AmdNamesInfo(names = ctx.attr.amd_names),
]

Expand Down
17 changes: 16 additions & 1 deletion internal/linker/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,19 @@
exports_files(["link_node_modules.js"])
# BEGIN-INTERNAL
load("@npm_bazel_typescript//:index.from_src.bzl", "checked_in_ts_library")

# We can't bootstrap the ts_library rule using the linker itself,
# because the implementation of ts_library depends on the linker so that would be a cycle.
# So we compile it to JS and check in the result as index.js
checked_in_ts_library(
name = "linker_lib",
srcs = ["link_node_modules.ts"],
checked_in_js = "index.js",
visibility = ["//internal/linker:__subpackages__"],
deps = ["@npm//@types/node"],
)

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

filegroup(
name = "package_contents",
Expand Down
5 changes: 5 additions & 0 deletions internal/linker/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ Under Bazel, we have exactly this monorepo feature. But, we want users to have a
To make this seamless, we run a linker as a separate program inside the Bazel action, right before node.
It does essentially the same job as Lerna: make sure there is a `$PWD/node_modules` tree and that all the semantics from Bazel (such as `module_name`/`module_root` attributes) are mapped to the node module resolution algorithm, so that the node runtime behaves the same way as if the packages had been installed from npm.

Note that the behavior of the linker depends on whether the package to link was declared as:

1. a runtime dependency of a binary run by Bazel, which we call "statically linked", and which is resolved from Bazel's Runfiles tree or manifest
1. a dependency declared by a user of that binary, which we call "dynamically linked", and which is resolved from the execution root

In the future the linker should also generate `package.json` files so that things like `main` and `typings` fields are present and reflect the Bazel semantics, so that we can entirely eliminate custom loading and pathmapping logic from binaries we execute.

[lerna]: https://github.com/lerna/lerna
206 changes: 206 additions & 0 deletions internal/linker/index.js

Large diffs are not rendered by default.

9 changes: 7 additions & 2 deletions internal/linker/link_node_modules.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def _debug(vars, *args):
_ASPECT_RESULT_NAME = "link_node_modules__aspect_result"

# Traverse 'srcs' in addition so that we can go across a genrule
_MODULE_MAPPINGS_DEPS_NAMES = ["deps", "srcs"]
_MODULE_MAPPINGS_DEPS_NAMES = ["data", "deps", "srcs"]

def register_node_modules_linker(ctx, args, inputs):
"""Helps an action to run node by setting up the node_modules linker as a pre-process
Expand Down Expand Up @@ -55,7 +55,12 @@ def register_node_modules_linker(ctx, args, inputs):
# Write the result to a file, and use the magic node option --bazel_node_modules_manifest
# The node_launcher.sh will peel off this argument and pass it to the linker rather than the program.
modules_manifest = ctx.actions.declare_file("_%s.module_mappings.json" % ctx.label.name)
ctx.actions.write(modules_manifest, str({"modules": mappings, "root": node_modules_root}))
content = {
"modules": mappings,
"root": node_modules_root,
"workspace": ctx.workspace_name,
}
ctx.actions.write(modules_manifest, str(content))
args.add("--bazel_node_modules_manifest=%s" % modules_manifest.path)
inputs.append(modules_manifest)

Expand Down
156 changes: 0 additions & 156 deletions internal/linker/link_node_modules.js

This file was deleted.

Loading

0 comments on commit 7f0102e

Please sign in to comment.