From 72c7b2df55f89a3d383ea56ac74b6d432f0da3a9 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Mon, 14 May 2018 10:49:19 -0700 Subject: [PATCH] Use rootpath helper for ctx.expand_location 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: https://github.com/bazelbuild/rules_nodejs/issues/84 Fixes #32 --- README.md | 3 +- examples/program/BUILD.bazel | 32 ++++++++++++++++--- examples/rollup/BUILD.bazel | 2 +- examples/rollup/rollup.bzl | 2 +- internal/e2e/rollup/BUILD.bazel | 2 +- .../jasmine_node_test/jasmine_node_test.bzl | 11 ++++--- internal/node/node.bzl | 14 ++++++-- internal/node/node_loader.js | 7 +++- internal/node/node_repositories.bzl | 4 +-- internal/npm_package/BUILD.bazel | 2 +- internal/npm_package/packager.js | 2 +- internal/rollup/BUILD.bazel | 7 ++-- internal/rollup/rollup_bundle.bzl | 2 +- 13 files changed, 65 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index 6d96fa9d9d..622c322607 100644 --- a/README.md +++ b/README.md @@ -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"], ) ``` diff --git a/examples/program/BUILD.bazel b/examples/program/BUILD.bazel index daca405c89..9e4913a82f 100644 --- a/examples/program/BUILD.bazel +++ b/examples/program/BUILD.bazel @@ -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 @@ -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( diff --git a/examples/rollup/BUILD.bazel b/examples/rollup/BUILD.bazel index 270bb01c5e..b4ac174faf 100644 --- a/examples/rollup/BUILD.bazel +++ b/examples/rollup/BUILD.bazel @@ -32,7 +32,7 @@ rollup( "bar.js", "foo.js", ], - entry_point = "examples/rollup/foo.js", + entry_point = "$(rootpath foo.js)", ) jasmine_node_test( diff --git a/examples/rollup/rollup.bzl b/examples/rollup/rollup.bzl index 48f250bb77..5f08ca8b8a 100644 --- a/examples/rollup/rollup.bzl +++ b/examples/rollup/rollup.bzl @@ -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"] diff --git a/internal/e2e/rollup/BUILD.bazel b/internal/e2e/rollup/BUILD.bazel index f12121ffaf..9b4c2af056 100644 --- a/internal/e2e/rollup/BUILD.bazel +++ b/internal/e2e/rollup/BUILD.bazel @@ -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", diff --git a/internal/jasmine_node_test/jasmine_node_test.bzl b/internal/jasmine_node_test/jasmine_node_test.bzl index b9f7f3c373..ae008f6f38 100644 --- a/internal/jasmine_node_test/jasmine_node_test.bzl +++ b/internal/jasmine_node_test/jasmine_node_test.bzl @@ -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 diff --git a/internal/node/node.bzl b/internal/node/node.bzl index 05b009e261..6759fb49d3 100644 --- a/internal/node/node.bzl +++ b/internal/node/node.bzl @@ -34,6 +34,9 @@ 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, @@ -41,7 +44,7 @@ def _write_loader_script(ctx): "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 @@ -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 @@ -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 ) diff --git a/internal/node/node_loader.js b/internal/node/node_loader.js index 2973ef81d0..5abd2a8b58 100644 --- a/internal/node/node_loader.js +++ b/internal/node/node_loader.js @@ -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), ]; diff --git a/internal/node/node_repositories.bzl b/internal/node/node_repositories.bzl index f7f9050857..1e0e58a015 100644 --- a/internal/node/node_repositories.bzl +++ b/internal/node/node_repositories.bzl @@ -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", diff --git a/internal/npm_package/BUILD.bazel b/internal/npm_package/BUILD.bazel index 27fbaa9fcb..060492c868 100644 --- a/internal/npm_package/BUILD.bazel +++ b/internal/npm_package/BUILD.bazel @@ -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"], ) diff --git a/internal/npm_package/packager.js b/internal/npm_package/packager.js index 9330e1837e..b7b3374d2f 100644 --- a/internal/npm_package/packager.js +++ b/internal/npm_package/packager.js @@ -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}`)); } diff --git a/internal/rollup/BUILD.bazel b/internal/rollup/BUILD.bazel index f5fee63673..c8aa5f8a8c 100644 --- a/internal/rollup/BUILD.bazel +++ b/internal/rollup/BUILD.bazel @@ -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"], ) diff --git a/internal/rollup/rollup_bundle.bzl b/internal/rollup/rollup_bundle.bzl index 42e1371d74..68749058b9 100644 --- a/internal/rollup/rollup_bundle.bzl +++ b/internal/rollup/rollup_bundle.bzl @@ -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")