diff --git a/WORKSPACE b/WORKSPACE index e0d1d7e077..53230654f0 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -237,13 +237,13 @@ filegroup( "//@gregmagolan:BUILD.bazel", "//@gregmagolan/test-a/bin:BUILD.bazel", "//@gregmagolan/test-a:BUILD.bazel", - "//@gregmagolan/test-b/bin:BUILD.bazel", + "//@gregmagolan/test-a:index.bzl", "//@gregmagolan/test-b:BUILD.bazel", "//ajv:BUILD.bazel", "//jasmine/bin:BUILD.bazel", "//jasmine:BUILD.bazel", + "//jasmine:index.bzl", "//rxjs:BUILD.bazel", - "//unidiff/bin:BUILD.bazel", "//unidiff:BUILD.bazel", "//zone.js:BUILD.bazel", ], diff --git a/defs.bzl b/defs.bzl index 27002e974b..c9b4e4e50a 100644 --- a/defs.bzl +++ b/defs.bzl @@ -28,6 +28,7 @@ load( _nodejs_test = "nodejs_test", ) load("//internal/node:node_repositories.bzl", _node_repositories = "node_repositories") +load("//internal/node:npm_package_bin.bzl", _npm_bin = "npm_package_bin") load("//internal/npm_install:npm_install.bzl", _npm_install = "npm_install", _yarn_install = "yarn_install") load("//internal/npm_package:npm_package.bzl", _npm_package = "npm_package") load("//internal/rollup:rollup_bundle.bzl", _rollup_bundle = "rollup_bundle") @@ -41,6 +42,7 @@ rollup_bundle = _rollup_bundle npm_package = _npm_package history_server = _history_server http_server = _http_server +npm_package_bin = _npm_bin # ANY RULES ADDED HERE SHOULD BE DOCUMENTED, see index.for_docs.bzl # Allows us to avoid a transitive dependency on bazel_skylib from leaking to users diff --git a/e2e/less/BUILD.bazel b/e2e/less/BUILD.bazel index ed980dc4f3..77ce9f3c14 100644 --- a/e2e/less/BUILD.bazel +++ b/e2e/less/BUILD.bazel @@ -1,9 +1,20 @@ load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_test") -load("@npm_bazel_less//:index.bzl", "less_binary") +load("@npm//less:index.bzl", "lessc") -less_binary( +lessc( name = "styles", - src = "test.less", + srcs = ["test.less"], + outs = [ + "test.css", + "test.css.map", + ], + args = [ + "test.less", + # Output paths must use $(location) since Bazel puts them in a platform-dependent output directory + "$(location test.css)", + "--silent", + "--source-map", + ], ) nodejs_test( diff --git a/e2e/less/yarn.lock b/e2e/less/yarn.lock index 1c62eff8d6..394e6a924a 100644 --- a/e2e/less/yarn.lock +++ b/e2e/less/yarn.lock @@ -2,8 +2,10 @@ # yarn lockfile v1 -"@bazel/less@file:../../dist/bin/packages/less/npm_package": - version "1.2.3" +"@bazel/less@latest": + version "0.37.1" + resolved "https://registry.yarnpkg.com/@bazel/less/-/less-0.37.1.tgz#915bd08f3f164b106639482ce3feebac30f63773" + integrity sha512-Ch+wN17E4CF5Gk7eX+mJ/i42LHXwDvDe6WMFHxBD0zwY3Omi/DozGYcaM+byCKSmssiPKK4g//d4ZIljVCsdfQ== dependencies: less "^3.9.0" diff --git a/e2e/stylus/BUILD.bazel b/e2e/stylus/BUILD.bazel index 3177bccf93..aabd2a4716 100644 --- a/e2e/stylus/BUILD.bazel +++ b/e2e/stylus/BUILD.bazel @@ -1,9 +1,21 @@ load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_test") -load("@npm_bazel_stylus//:index.bzl", "stylus_binary") +load("@npm//stylus:index.bzl", "stylus") -stylus_binary( +stylus( name = "styles", - src = "test.styl", + srcs = ["test.styl"], + outs = [ + "test.css", + "test.css.map", + ], + args = [ + "test.styl", + "--out", + # Output paths must use $(location) since Bazel puts them in a platform-dependent output directory + "$(location test.css)", + "--compress", + "--sourcemap", + ], ) nodejs_test( diff --git a/internal/node/npm_package_bin.bzl b/internal/node/npm_package_bin.bzl new file mode 100644 index 0000000000..fa781c512b --- /dev/null +++ b/internal/node/npm_package_bin.bzl @@ -0,0 +1,68 @@ +"A generic rule to run a tool that appears in node_modules/.bin" + +load("@build_bazel_rules_nodejs//internal/linker:link_node_modules.bzl", "module_mappings_aspect", "register_node_modules_linker") + +_ATTRS = { + "srcs": attr.label_list(allow_files = True), + "outs": attr.output_list(), + "args": attr.string_list(mandatory = True), + "tool": attr.label( + executable = True, + cfg = "host", + mandatory = True, + ), + "deps": attr.label_list(aspects = [module_mappings_aspect]), +} + +def _impl(ctx): + args = ctx.actions.args() + inputs = ctx.files.srcs + ctx.files.deps + outputs = ctx.outputs.outs + register_node_modules_linker(ctx, args, inputs) + for a in ctx.attr.args: + args.add(ctx.expand_location(a)) + ctx.actions.run( + executable = ctx.executable.tool, + inputs = inputs, + outputs = outputs, + arguments = [args], + ) + +_npm_genrule = rule( + _impl, + attrs = _ATTRS, +) + +def npm_package_bin(tool = None, package = None, package_bin = None, **kwargs): + """Run an arbitrary npm package binary (anything under node_modules/.bin/*) under Bazel. + + This is like a genrule() except that it runs our launcher script that first + links the node_modules tree before running the program. + + It's probably easy to wrap this with macros, so something with no rule logic + like stylus_binary could probably just be a macro wrapping this. + + Args: + srcs: identical to [genrule.srcs](https://docs.bazel.build/versions/master/be/general.html#genrule.srcs) + deps: Targets that produce or reference npm packages which are needed by the tool + outs: identical to [genrule.outs](https://docs.bazel.build/versions/master/be/general.html#genrule.outs) + args: Command-line arguments to the tool. + + Subject to 'Make variable' substitution. + Can use $(location) expansion. See https://docs.bazel.build/versions/master/be/make-variables.html + + 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` + tool: a label for a binary to run, like `@npm//terser/bin:terser`. This is the longer form of package/package_bin + """ + if not tool: + if not package: + fail("You must supply either the tool or package attribute") + if not package_bin: + package_bin = package + tool = "@npm//%s/bin:%s" % (package, package_bin) + + _npm_genrule( + tool = tool, + **kwargs + ) diff --git a/internal/node/test/BUILD.bazel b/internal/node/test/BUILD.bazel index 704ed6c1e9..2569c79597 100644 --- a/internal/node/test/BUILD.bazel +++ b/internal/node/test/BUILD.bazel @@ -1,4 +1,4 @@ -load("//:defs.bzl", "nodejs_binary", "nodejs_test") +load("//:defs.bzl", "nodejs_binary", "nodejs_test", "npm_package_bin") load("//internal/js_library:js_library.bzl", "js_library") load("//third_party/github.com/bazelbuild/bazel-skylib:rules/copy_file.bzl", "copy_file") load("//third_party/github.com/bazelbuild/bazel-skylib:rules/write_file.bzl", "write_file") @@ -145,3 +145,21 @@ nodejs_test( ], entry_point = ":data_resolution_built.spec.js", ) + +npm_package_bin( + name = "run_terser", + srcs = ["terser_input.js"], + outs = ["minified.js"], + args = [ + "$(location terser_input.js)", + "--output", + "$(location minified.js)", + ], + package = "terser", +) + +nodejs_test( + name = "npm_package_bin_test", + data = ["minified.js"], + entry_point = "npm_package_bin.spec.js", +) diff --git a/internal/node/test/npm_package_bin.spec.js b/internal/node/test/npm_package_bin.spec.js new file mode 100644 index 0000000000..1aa20b6068 --- /dev/null +++ b/internal/node/test/npm_package_bin.spec.js @@ -0,0 +1,8 @@ +const fs = require('fs'); +const path = require('path'); + +const content = fs.readFileSync(path.join(require.resolve(__dirname + '/minified.js')), 'utf-8'); +if (!content.includes('{console.error("thing")}')) { + console.error(content); + process.exitCode = 1; +} diff --git a/internal/node/test/terser_input.js b/internal/node/test/terser_input.js new file mode 100644 index 0000000000..c82df82a3e --- /dev/null +++ b/internal/node/test/terser_input.js @@ -0,0 +1,5 @@ +class A { + doThing() { + console.error('thing'); + } +} diff --git a/internal/npm_install/generate_build_file.js b/internal/npm_install/generate_build_file.js index 3b0eb9d414..a60c9edf77 100644 --- a/internal/npm_install/generate_build_file.js +++ b/internal/npm_install/generate_build_file.js @@ -221,14 +221,27 @@ node_module_library( } /** - * Generates all BUILD files for a package. + * Generates all BUILD & bzl files for a package. */ function generatePackageBuildFiles(pkg) { - const buildFile = BUILD_FILE_HEADER + printPackage(pkg); - writeFileSync(path.posix.join(pkg._dir, 'BUILD.bazel'), buildFile); + let buildFile = printPackage(pkg); - const binBuildFile = BUILD_FILE_HEADER + printPackageBin(pkg); - writeFileSync(path.posix.join(pkg._dir, 'bin', 'BUILD.bazel'), binBuildFile); + const binBuildFile = printPackageBin(pkg); + if (binBuildFile.length) { + writeFileSync( + path.posix.join(pkg._dir, 'bin', 'BUILD.bazel'), BUILD_FILE_HEADER + binBuildFile); + } + + const indexFile = printIndexBzl(pkg); + if (indexFile.length) { + writeFileSync(path.posix.join(pkg._dir, 'index.bzl'), indexFile); + buildFile = `${buildFile} +# For integration testing +exports_files(["index.bzl"]) +`; + } + + writeFileSync(path.posix.join(pkg._dir, 'BUILD.bazel'), BUILD_FILE_HEADER + buildFile); } /** @@ -951,12 +964,7 @@ npm_umd_bundle( return result; } -/** - * Given a pkg, return the skylark nodejs_binary targets for the package. - */ -function printPackageBin(pkg) { - let result = ''; - +function _findExecutables(pkg) { const executables = new Map(); // For root packages, transform the pkg.bin entries @@ -982,6 +990,16 @@ function printPackageBin(pkg) { } } + return executables; +} + + +/** + * Given a pkg, return the skylark nodejs_binary targets for the package. + */ +function printPackageBin(pkg) { + let result = ''; + const executables = _findExecutables(pkg); if (executables.size) { result = `load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary") @@ -1024,6 +1042,26 @@ nodejs_binary( return result; } +function printIndexBzl(pkg) { + let result = ''; + const executables = _findExecutables(pkg); + if (executables.size) { + result = `load("@build_bazel_rules_nodejs//:defs.bzl", "npm_package_bin") + +`; + } + for (const name of executables.keys()) { + result = `${result} + +# Generated helper macro to call ${name} +def ${name}(**kwargs): + npm_package_bin(tool = "@${WORKSPACE}//${pkg._dir}/bin:${name}", **kwargs) +`; + } + + return result; +} + /** * Given a scope, return the skylark `node_module_library` target for the scope. */ diff --git a/internal/npm_install/test/check.js b/internal/npm_install/test/check.js index f1a11beb8b..d4f0ebdcf9 100644 --- a/internal/npm_install/test/check.js +++ b/internal/npm_install/test/check.js @@ -56,13 +56,13 @@ module.exports = { '@gregmagolan/BUILD.bazel', '@gregmagolan/test-a/bin/BUILD.bazel', '@gregmagolan/test-a/BUILD.bazel', - '@gregmagolan/test-b/bin/BUILD.bazel', + '@gregmagolan/test-a/index.bzl', '@gregmagolan/test-b/BUILD.bazel', 'ajv/BUILD.bazel', 'jasmine/bin/BUILD.bazel', 'jasmine/BUILD.bazel', + 'jasmine/index.bzl', 'rxjs/BUILD.bazel', - 'unidiff/bin/BUILD.bazel', 'unidiff/BUILD.bazel', 'zone.js/BUILD.bazel', ], diff --git a/internal/npm_install/test/golden/@gregmagolan/test-a/BUILD.bazel.golden b/internal/npm_install/test/golden/@gregmagolan/test-a/BUILD.bazel.golden index 284e302a04..8a25780643 100644 --- a/internal/npm_install/test/golden/@gregmagolan/test-a/BUILD.bazel.golden +++ b/internal/npm_install/test/golden/@gregmagolan/test-a/BUILD.bazel.golden @@ -30,3 +30,4 @@ npm_umd_bundle( entry_point = "//:node_modules/@gregmagolan/test-a/main.js", package = ":test-a", ) +exports_files(["index.bzl"]) diff --git a/internal/npm_install/test/golden/@gregmagolan/test-a/index.bzl.golden b/internal/npm_install/test/golden/@gregmagolan/test-a/index.bzl.golden new file mode 100644 index 0000000000..ddce2e299e --- /dev/null +++ b/internal/npm_install/test/golden/@gregmagolan/test-a/index.bzl.golden @@ -0,0 +1,3 @@ +load("@build_bazel_rules_nodejs//:defs.bzl", "npm_package_bin") +def test(**kwargs): + npm_package_bin(tool = "@fine_grained_goldens//@gregmagolan/test-a/bin:test", **kwargs) diff --git a/internal/npm_install/test/golden/@gregmagolan/test-b/bin/BUILD.bazel.golden b/internal/npm_install/test/golden/@gregmagolan/test-b/bin/BUILD.bazel.golden deleted file mode 100644 index ec33b04a28..0000000000 --- a/internal/npm_install/test/golden/@gregmagolan/test-b/bin/BUILD.bazel.golden +++ /dev/null @@ -1,2 +0,0 @@ - -package(default_visibility = ["//visibility:public"]) diff --git a/internal/npm_install/test/golden/BUILD.bazel.golden b/internal/npm_install/test/golden/BUILD.bazel.golden index 31a6262f98..cdc72241d4 100644 --- a/internal/npm_install/test/golden/BUILD.bazel.golden +++ b/internal/npm_install/test/golden/BUILD.bazel.golden @@ -4754,13 +4754,13 @@ filegroup( "//@gregmagolan:BUILD.bazel", "//@gregmagolan/test-a/bin:BUILD.bazel", "//@gregmagolan/test-a:BUILD.bazel", - "//@gregmagolan/test-b/bin:BUILD.bazel", + "//@gregmagolan/test-a:index.bzl", "//@gregmagolan/test-b:BUILD.bazel", "//ajv:BUILD.bazel", "//jasmine/bin:BUILD.bazel", "//jasmine:BUILD.bazel", + "//jasmine:index.bzl", "//rxjs:BUILD.bazel", - "//unidiff/bin:BUILD.bazel", "//unidiff:BUILD.bazel", "//zone.js:BUILD.bazel", ], diff --git a/internal/npm_install/test/golden/jasmine/BUILD.bazel.golden b/internal/npm_install/test/golden/jasmine/BUILD.bazel.golden index 05e0125cb2..e871e44a4a 100644 --- a/internal/npm_install/test/golden/jasmine/BUILD.bazel.golden +++ b/internal/npm_install/test/golden/jasmine/BUILD.bazel.golden @@ -53,3 +53,4 @@ npm_umd_bundle( entry_point = "//:node_modules/jasmine/lib/jasmine.js", package = ":jasmine", ) +exports_files(["index.bzl"]) diff --git a/internal/npm_install/test/golden/jasmine/index.bzl.golden b/internal/npm_install/test/golden/jasmine/index.bzl.golden new file mode 100644 index 0000000000..b415fcc90d --- /dev/null +++ b/internal/npm_install/test/golden/jasmine/index.bzl.golden @@ -0,0 +1,3 @@ +load("@build_bazel_rules_nodejs//:defs.bzl", "npm_package_bin") +def jasmine(**kwargs): + npm_package_bin(tool = "@fine_grained_goldens//jasmine/bin:jasmine", **kwargs) diff --git a/internal/npm_install/test/golden/manual_build_file_contents.golden b/internal/npm_install/test/golden/manual_build_file_contents.golden index d4e574beba..12eb03bc40 100755 --- a/internal/npm_install/test/golden/manual_build_file_contents.golden +++ b/internal/npm_install/test/golden/manual_build_file_contents.golden @@ -10,13 +10,13 @@ filegroup( "//@gregmagolan:BUILD.bazel", "//@gregmagolan/test-a/bin:BUILD.bazel", "//@gregmagolan/test-a:BUILD.bazel", - "//@gregmagolan/test-b/bin:BUILD.bazel", + "//@gregmagolan/test-a:index.bzl", "//@gregmagolan/test-b:BUILD.bazel", "//ajv:BUILD.bazel", "//jasmine/bin:BUILD.bazel", "//jasmine:BUILD.bazel", + "//jasmine:index.bzl", "//rxjs:BUILD.bazel", - "//unidiff/bin:BUILD.bazel", "//unidiff:BUILD.bazel", "//zone.js:BUILD.bazel", ], diff --git a/internal/npm_install/test/golden/unidiff/bin/BUILD.bazel.golden b/internal/npm_install/test/golden/unidiff/bin/BUILD.bazel.golden deleted file mode 100644 index ec33b04a28..0000000000 --- a/internal/npm_install/test/golden/unidiff/bin/BUILD.bazel.golden +++ /dev/null @@ -1,2 +0,0 @@ - -package(default_visibility = ["//visibility:public"]) diff --git a/packages/less/BUILD.bazel b/packages/less/BUILD.bazel index e236045f94..d118d3e395 100644 --- a/packages/less/BUILD.bazel +++ b/packages/less/BUILD.bazel @@ -33,6 +33,8 @@ npm_package( srcs = [ "@npm_bazel_less//:package_contents", ], + # less package is deprecated + tags = ["do-not-publish"], vendor_external = [ "npm_bazel_less", ], diff --git a/packages/stylus/BUILD.bazel b/packages/stylus/BUILD.bazel index 8b89f69414..075c60c40a 100644 --- a/packages/stylus/BUILD.bazel +++ b/packages/stylus/BUILD.bazel @@ -33,6 +33,8 @@ npm_package( srcs = [ "@npm_bazel_stylus//:package_contents", ], + # stylus package is deprecated + tags = ["do-not-publish"], vendor_external = [ "npm_bazel_stylus", ],