Skip to content

Commit

Permalink
fix(builtin): use correct genrule-style make vars
Browse files Browse the repository at this point in the history
https://docs.bazel.build/versions/master/be/make-variables.html documents that $@ is for the output location of a single out
and $(RULEDIR) is the preferred way to refer to the output directory of the package

BREAKING CHANGE:
Usage of the $@ shortcut in npm_package_bin-generated rules should now be $(RULEDIR)
and you can now use $@ to refer to the location of a single output
  • Loading branch information
alexeagle committed Nov 26, 2019
1 parent 8f44bea commit 77039b1
Show file tree
Hide file tree
Showing 12 changed files with 36 additions and 22 deletions.
2 changes: 1 addition & 1 deletion e2e/ts_devserver/subpackage/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ html_insert_assets(
"--html",
"$(location index.tmpl.html)",
"--out",
"$(location index.html)",
"$@",
"--assets",
"$(location //:red-body-style.css)",
],
Expand Down
6 changes: 3 additions & 3 deletions examples/angular/src/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ html_insert_assets(
"--html",
"$(location //src:example/index.html)",
"--out",
"$(location index.html)",
"$@",
"--assets",
] + ["$(location %s)" % s for s in _ASSETS] + [
# This file doesn't exist during the build, but will be served by ts_devserver
Expand Down Expand Up @@ -154,7 +154,7 @@ babel(
"--source-maps",
"--presets=@babel/preset-env",
"--out-dir",
"$@",
"$(RULEDIR)",
],
data = [
":bundle-es2015",
Expand Down Expand Up @@ -184,7 +184,7 @@ html_insert_assets(
"--html",
"$(location //src:example/index.prod.html)",
"--out",
"$(location _prodapp/src/example/index.html)",
"$@",
"--assets",
] + ["$(location %s)" % s for s in _ASSETS],
data = ["//src:example/index.prod.html"] + _ASSETS,
Expand Down
6 changes: 3 additions & 3 deletions examples/angular_view_engine/src/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ html_insert_assets(
"--html",
"$(location //src:example/index.html)",
"--out",
"$(location index.html)",
"$@",
"--assets",
] + ["$(location %s)" % s for s in _ASSETS] + [
# This file doesn't exist during the build, but will be served by ts_devserver
Expand Down Expand Up @@ -154,7 +154,7 @@ babel(
"--source-maps",
"--presets=@babel/preset-env",
"--out-dir",
"$@",
"$(RULEDIR)",
],
data = [
":bundle-es2015",
Expand Down Expand Up @@ -184,7 +184,7 @@ html_insert_assets(
"--html",
"$(location //src:example/index.prod.html)",
"--out",
"$(location _prodapp/src/example/index.html)",
"$@",
"--assets",
] + ["$(location %s)" % s for s in _ASSETS],
data = ["//src:example/index.prod.html"] + _ASSETS,
Expand Down
8 changes: 4 additions & 4 deletions examples/app/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ html_insert_assets(
outs = ["index.html"],
args = [
"--out",
"$(location index.html)",
"$@",
"--html",
"$(location :index.tmpl.html)",
"--roots",
"$@",
"$(RULEDIR)",
"--assets",
] + ["$(locations %s)" % a for a in _ASSETS],
data = [":index.tmpl.html"] + _ASSETS,
Expand Down Expand Up @@ -77,8 +77,8 @@ tsc(
"-p",
"$(location tsconfig-test.json)",
"--outDir",
# $@ is a shorthand for the dist/bin directory where Bazel requires we write outputs
"$@",
# $(RULEDIR) is a shorthand for the dist/bin directory where Bazel requires we write outputs
"$(RULEDIR)",
],
data = [
"app.e2e-spec.ts",
Expand Down
2 changes: 1 addition & 1 deletion examples/closure/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ google_closure_compiler(
# --platform native would be faster but is failing on Windows
"--platform=javascript",
"--js=$(location hello.js)",
"--js_output_file=$(location bundle.js)",
"--js_output_file=$@",
],
data = ["hello.js"],
)
Expand Down
5 changes: 5 additions & 0 deletions examples/protocol_buffers/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2419,6 +2419,11 @@ requirejs@2.3.5:
resolved "https://registry.yarnpkg.com/requirejs/-/requirejs-2.3.5.tgz#617b9acbbcb336540ef4914d790323a8d4b861b0"
integrity sha512-svnO+aNcR/an9Dpi44C7KSAy5fFGLtmPbaaCeQaklUz8BQhS64tWWIIlvEA5jrWICzlO/X9KSzSeXFnZdBu8nw==

requirejs@2.3.6:
version "2.3.6"
resolved "https://registry.yarnpkg.com/requirejs/-/requirejs-2.3.6.tgz#e5093d9601c2829251258c0b9445d4d19fa9e7c9"
integrity sha512-ipEzlWQe6RK3jkzikgCupiTbTvm4S0/CAU5GlgptkN5SO6F3u0UD0K18wy6ErDqiCyP4J4YYe1HuAShvsxePLg==

requires-port@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/requires-port/-/requires-port-1.0.0.tgz#925d2601d39ac485e091cf0da5c6e694dc3dcaff"
Expand Down
4 changes: 2 additions & 2 deletions examples/react_webpack/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ tsc(
"$(location index.tsx)",
"$(location types.d.ts)",
"--outDir",
"$@",
"$(RULEDIR)",
"--lib",
"es2015,dom",
"--jsx",
Expand All @@ -42,7 +42,7 @@ webpack(
"--config",
"$(location webpack.config.js)",
"-o",
"$(location app.bundle.js)",
"$@",
],
data = [
"index.js",
Expand Down
2 changes: 1 addition & 1 deletion examples/webapp/differential_loading.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def differential_loading(name, entry_point, srcs):
"--config-file",
"$(location es5.babelrc)",
"--out-dir",
"$@",
"$(RULEDIR)",
],
)

Expand Down
15 changes: 12 additions & 3 deletions internal/node/npm_package_bin.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,15 @@ def _expand_location(ctx, s):
# We'll write into a newly created directory named after the rule
outdir_segments.append(ctx.attr.name)

if not ctx.attr.output_dir:
if s.find("$@") != -1 and len(ctx.outputs.outs) > 1:
fail("""$@ substitution may only be used with a single out
Upgrading rules_nodejs? Maybe you need to switch from $@ to $(RULEDIR)
See https://github.com/bazelbuild/rules_nodejs/releases/tag/0.42.0""")
s = s.replace("$@", ctx.outputs.outs[0].path)

# The list comprehension removes empty segments like if we are in the root package
s = s.replace("$@", "/".join([o for o in outdir_segments if o]))
s = s.replace("$(RULEDIR)", "/".join([o for o in outdir_segments if o]))
return ctx.expand_location(s, targets = ctx.attr.data)

def _inputs(ctx):
Expand Down Expand Up @@ -95,8 +102,10 @@ def npm_package_bin(tool = None, package = None, package_bin = None, data = [],
Subject to 'Make variable' substitution.
Can use $(location) expansion. See https://docs.bazel.build/versions/master/be/make-variables.html
You may also refer to the location of the output_dir with the special `$@` replacement, like genrule.
If output_dir=False then $@ will refer to the output directory for this package.
Like genrule, you may also use some syntax sugar for locations:
- `$@`: if you have only one output file, the location of the output
- `$(RULEDIR)`: the output directory of the rule, corresponding with its package
(can be used with output_dir=True or False)
package: an npm package whose binary to run, like "terser". Assumes your node_modules are installed in a workspace called "npm"
package_bin: the "bin" entry from `package` that should be run. By default package_bin is the same string as `package`
Expand Down
4 changes: 2 additions & 2 deletions internal/node/test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ npm_package_bin(
args = [
"$(location terser_input.js)",
"--output",
"$(location minified.js)",
"$@",
],
data = ["terser_input.js"],
package = "terser",
Expand All @@ -166,7 +166,7 @@ npm_package_bin(
name = "dir_output",
args = [
"--output-dir",
"$@",
"$(RULEDIR)",
"$(location terser_input.js)",
],
data = ["terser_input.js"],
Expand Down
2 changes: 1 addition & 1 deletion packages/node-patches/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ tsc(
"-p",
"$(location tsconfig-bazel.json)",
"--outDir",
"$@",
"$(RULEDIR)",
],
data = sources + tests + [
"tsconfig.json",
Expand Down
2 changes: 1 addition & 1 deletion packages/typescript/docs/install.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ tsc(
outs = [s.replace(".ts", ext) for ext in [".js", ".d.ts"] for s in srcs],
args = [
"--outDir",
"$@",
"$(RULEDIR)",
"--lib",
"es2017,dom",
"--downlevelIteration",
Expand Down

0 comments on commit 77039b1

Please sign in to comment.