Skip to content

Commit

Permalink
Use rootpath helper for ctx.expand_location
Browse files Browse the repository at this point in the history
BREAKING CHANGE:
This forces us to use workspace-relative paths since the rootpath helper returns this format.
Users are now exposed to the external path segment to reference other workspaces.
See discussion: bazel-contrib#84

Fixes bazel-contrib#32
  • Loading branch information
alexeagle committed May 14, 2018
1 parent 3a35786 commit 72c7b2d
Show file tree
Hide file tree
Showing 13 changed files with 65 additions and 25 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,8 @@ nodejs_binary(
"@//:node_modules",
"main.js",
],
entry_point = "workspace_name/main.js",
# the rootpath function looks up the runtime location of the script
entry_point = "$(rootpath main.js)",
args = ["--node_options=--expose-gc"],
)
```
Expand Down
32 changes: 27 additions & 5 deletions examples/program/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,18 @@
load("@build_bazel_rules_nodejs//:defs.bzl", "jasmine_node_test")
# Copyright 2017 The Bazel Authors. All rights reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

load("@build_bazel_rules_nodejs//:defs.bzl", "jasmine_node_test", "nodejs_binary")
load("@build_bazel_rules_nodejs//:internal/common/typescript_mock_lib.bzl", "mock_typescript_lib")

# Make the jasmine library available at runtime by exposing our node_modules
Expand All @@ -13,16 +27,24 @@ filegroup(
visibility = ["//visibility:public"],
)

# Mock out the ts_library rule from rules_typescript
# We want to test that TypeScript outputs can be used in nodejs rules, but
# don't want a cyclical dependency between this repository and rules_typescript.
mock_typescript_lib(
name = "mock_ts_lib",
srcs = ["decrement.js"],
)

# This is like a "js_library", but there are no actions to run on JS files so a
# filegroup is semantically equivalent.
filegroup(
# This binary wraps up our program in a way that can be `bazel run`, and also
# produces a .exe file on windows.
nodejs_binary(
name = "program",
srcs = ["index.js"],
# The script we'll ask node to run
# Note, the label given here must exactly match an entry in "data"
# so that the script is a runtime dependency.
entry_point = "$(rootpath index.js)",
# Runtime dependencies (in this case, the script doesn't require any other deps)
data = ["index.js"],
)

jasmine_node_test(
Expand Down
2 changes: 1 addition & 1 deletion examples/rollup/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ rollup(
"bar.js",
"foo.js",
],
entry_point = "examples/rollup/foo.js",
entry_point = "$(rootpath foo.js)",
)

jasmine_node_test(
Expand Down
2 changes: 1 addition & 1 deletion examples/rollup/rollup.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ other rules in this repo.
"""

def _rollup(ctx):
args = ["--input", ctx.attr.entry_point]
args = ["--input", ctx.expand_location(ctx.attr.entry_point)]
args += ["--output.file", ctx.outputs.bundle.path]
args += ["--output.format", "es"]

Expand Down
2 changes: 1 addition & 1 deletion internal/e2e/rollup/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ rollup_bundle(
"bar.js",
"foo.js",
],
entry_point = "internal/e2e/rollup/foo.js",
entry_point = "$(rootpath foo.js)",
globals = {"some_global_var": "runtime_name_of_global_var"},
license_banner = ":license.txt",
node_modules = ":node_modules",
Expand Down
11 changes: 6 additions & 5 deletions internal/jasmine_node_test/jasmine_node_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,16 @@ def jasmine_node_test(name, srcs = [], data = [], deps = [], **kwargs):
testonly = 1,
)

all_data = data + srcs + deps
all_data += [Label("//internal/jasmine_node_test:jasmine_runner.js")]
all_data += [":%s_devmode_srcs.MF" % name]
entry_point = "build_bazel_rules_nodejs/internal/jasmine_node_test/jasmine_runner.js"
runner_js = "@build_bazel_rules_nodejs//internal/jasmine_node_test:jasmine_runner.js"
all_data = data + srcs + deps + [
runner_js,
":%s_devmode_srcs.MF" % name,
]

nodejs_test(
name = name,
data = all_data,
entry_point = entry_point,
entry_point = "$(rootpath %s)" % runner_js,
templated_args = ["$(location :%s_devmode_srcs.MF)" % name],
testonly = 1,
**kwargs
Expand Down
14 changes: 12 additions & 2 deletions internal/node/node.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,17 @@ def _write_loader_script(ctx):
escaped = mn.replace("/", r"\/").replace(".", r"\.")
mapping = r"{module_name: /^%s\b/, module_root: '%s'}" % (escaped, mr)
module_mappings.append(mapping)
location_targets = [ctx.attr.node_modules] + (
ctx.attr.data if hasattr(ctx.attr, "data") else []
)
ctx.template_action(
template=ctx.file._loader_template,
output=ctx.outputs.loader,
substitutions={
"TEMPLATED_module_roots": "\n " + ",\n ".join(module_mappings),
"TEMPLATED_bootstrap": "\n " + ",\n ".join(
["\"" + d + "\"" for d in ctx.attr.bootstrap]),
"TEMPLATED_entry_point": ctx.attr.entry_point,
"TEMPLATED_entry_point": ctx.expand_location(ctx.attr.entry_point, location_targets),
"TEMPLATED_label_package": ctx.attr.node_modules.label.package,
# There are two workspaces in general:
# A) The user's workspace is the one where the bazel command is run
Expand Down Expand Up @@ -239,7 +242,7 @@ The runtime will pause before executing the program, allowing you to connect a
remote debugger.
"""

def nodejs_binary_macro(name, args=[], visibility=None, tags=[], **kwargs):
def nodejs_binary_macro(name, args=[], entry_point=None, data=[], entry_module=None, visibility=None, tags=[], **kwargs):
"""This macro exists only to wrap the nodejs_binary as an .exe for Windows.
This is exposed in the public API at `//:defs.bzl` as `nodejs_binary`, so most
Expand All @@ -252,8 +255,15 @@ def nodejs_binary_macro(name, args=[], visibility=None, tags=[], **kwargs):
tags: applied to the wrapper binary
**kwargs: passed to the nodejs_binary
"""
if entry_module:
entry_point = "$(rootpath %s)" % entry_module
all_data = data + [entry_module]
else:
all_data = data
nodejs_binary(
name = "%s_bin" % name,
entry_point = entry_point,
data = all_data,
**kwargs
)

Expand Down
7 changes: 6 additions & 1 deletion internal/node/node_loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,12 @@ module.constructor._resolveFilename =
var failedResolutions = [];
var resolveLocations = [
request,
resolveRunfiles(request),
// See https://github.com/bazelbuild/rules_nodejs/issues/84 for discussion of paths
// The request should have either external/other_wksp/path/to/thing
// or just path/to/thing with the implied local workspace
// This matches Bazel semantics for cwd of toolchain processes
// and helpers like ctx.expand_location("$(rootpath foo)")
resolveRunfiles('TEMPLATED_user_workspace_name', request),
resolveRunfiles(
'TEMPLATED_user_workspace_name', 'TEMPLATED_label_package', 'node_modules', request),
];
Expand Down
4 changes: 2 additions & 2 deletions internal/node/node_repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -396,8 +396,8 @@ def node_repositories(
files need to be included in your node_modules filegroup. This option is desirable as it gives a stronger
guarantee of hermiticity which is required for remote execution.
"""
# Windows users need sh_binary wrapped as an .exe
check_bazel_version("0.5.4")
# we rely on $(rootpath) expansion
check_bazel_version("0.8.0")

_nodejs_repo(
name = "nodejs",
Expand Down
2 changes: 1 addition & 1 deletion internal/npm_package/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ nodejs_binary(
"packager.js",
"@nodejs//:run_npm.sh.template",
],
entry_point = "build_bazel_rules_nodejs/internal/npm_package/packager.js",
entry_point = "$(rootpath packager.js)",
node_modules = "@build_bazel_rules_nodejs_npm_install_deps//:node_modules",
visibility = ["//visibility:public"],
)
2 changes: 1 addition & 1 deletion internal/npm_package/packager.js
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ function main(args) {
}

const npmTemplate =
fs.readFileSync(require.resolve('nodejs/run_npm.sh.template'), {encoding: 'utf-8'});
fs.readFileSync(require.resolve('external/nodejs/run_npm.sh.template'), {encoding: 'utf-8'});
fs.writeFileSync(packPath, npmTemplate.replace('TMPL_args', `pack ${outDir}`));
fs.writeFileSync(publishPath, npmTemplate.replace('TMPL_args', `publish ${outDir}`));
}
Expand Down
7 changes: 4 additions & 3 deletions internal/rollup/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,22 @@ exports_files([

nodejs_binary(
name = "rollup",
entry_point = "build_bazel_rules_nodejs_rollup_deps/node_modules/rollup/bin/rollup",
# should just be "rollup" see #205
entry_module = "@build_bazel_rules_nodejs_rollup_deps//:node_modules/rollup/bin/rollup",
node_modules = "@build_bazel_rules_nodejs_rollup_deps//:node_modules",
visibility = ["//visibility:public"],
)

nodejs_binary(
name = "tsc",
entry_point = "build_bazel_rules_nodejs_rollup_deps/node_modules/typescript/bin/tsc",
entry_module = "@build_bazel_rules_nodejs_rollup_deps//:node_modules/typescript/bin/tsc",
node_modules = "@build_bazel_rules_nodejs_rollup_deps//:node_modules",
visibility = ["//visibility:public"],
)

nodejs_binary(
name = "uglify",
entry_point = "build_bazel_rules_nodejs_rollup_deps/node_modules/uglify-es/bin/uglifyjs",
entry_module = "@build_bazel_rules_nodejs_rollup_deps//:node_modules/uglify-es/bin/uglifyjs",
node_modules = "@build_bazel_rules_nodejs_rollup_deps//:node_modules",
visibility = ["//visibility:public"],
)
Expand Down
2 changes: 1 addition & 1 deletion internal/rollup/rollup_bundle.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def run_rollup(ctx, sources, config, output):
args.add(["--config", config.path])
args.add(["--output.file", output.path])
args.add(["--output.sourcemap", "--output.sourcemapFile", map_output.path])
args.add(["--input", ctx.attr.entry_point])
args.add(["--input", ctx.expand_location(ctx.attr.entry_point)])
# We will produce errors as needed. Anything else is spammy: a well-behaved
# bazel rule prints nothing on success.
args.add("--silent")
Expand Down

0 comments on commit 72c7b2d

Please sign in to comment.