From 0a63964bda175574972517050adcd573aecc9c78 Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Wed, 22 May 2019 00:52:23 -0700 Subject: [PATCH] Fixes and get all tests passing (#480) --- e2e/jasmine/BUILD.bazel | 10 --- e2e/ts_library/some_module/BUILD.bazel | 2 +- e2e/typescript_3.1/BUILD.bazel | 19 +----- internal/node/node.bzl | 61 +++++++++++++++---- internal/npm_install/BUILD.bazel | 2 +- packages/create/BUILD.bazel | 2 +- packages/jasmine/BUILD.bazel | 2 +- packages/jasmine/src/jasmine_node_test.bzl | 17 +++--- packages/jasmine/test/BUILD.bazel | 16 ++--- packages/labs/package.json | 3 + packages/labs/webpack/BUILD.bazel | 13 +++- packages/labs/webpack/src/BUILD.bazel | 26 +------- packages/labs/webpack/src/cli.ts | 10 ++- packages/labs/webpack/src/webpack_bundle.bzl | 2 +- packages/labs/webpack/test/BUILD.bazel | 5 ++ packages/typescript/WORKSPACE | 6 +- scripts/setup_examples_angular.sh | 3 +- .../bazel_workspaces/a/BUILD.bazel | 2 +- .../bazel_workspaces/a/subdir/BUILD.bazel | 2 +- .../bazel_workspaces/b/BUILD.bazel | 2 +- .../bazel_workspaces/b/subdir/BUILD.bazel | 2 +- .../bazel_workspaces/package.json | 2 +- yarn.lock | 2 +- 23 files changed, 114 insertions(+), 97 deletions(-) diff --git a/e2e/jasmine/BUILD.bazel b/e2e/jasmine/BUILD.bazel index 7bf44e81e0..6910dd20c9 100644 --- a/e2e/jasmine/BUILD.bazel +++ b/e2e/jasmine/BUILD.bazel @@ -3,10 +3,6 @@ load("@npm_bazel_jasmine//:index.bzl", "jasmine_node_test") jasmine_node_test( name = "test", srcs = ["test.spec.js"], - jasmine = "@npm_bazel_jasmine//:index.js", - deps = [ - "@npm//@bazel/jasmine", - ], ) jasmine_node_test( @@ -14,9 +10,7 @@ jasmine_node_test( srcs = ["jasmine_shared_env_test.spec.js"], bootstrap = ["e2e_jasmine/jasmine_shared_env_bootstrap.js"], data = ["jasmine_shared_env_bootstrap.js"], - jasmine = "@npm_bazel_jasmine//:index.js", deps = [ - "@npm//@bazel/jasmine", "@npm//jasmine", "@npm//jasmine-core", "@npm//zone.js", @@ -30,8 +24,4 @@ jasmine_node_test( "coverage_source.js", ], coverage = True, - jasmine = "@npm_bazel_jasmine//:index.js", - deps = [ - "@npm//@bazel/jasmine", - ], ) diff --git a/e2e/ts_library/some_module/BUILD.bazel b/e2e/ts_library/some_module/BUILD.bazel index 98e10c3163..ba8e7c3120 100644 --- a/e2e/ts_library/some_module/BUILD.bazel +++ b/e2e/ts_library/some_module/BUILD.bazel @@ -25,7 +25,7 @@ nodejs_binary( ":main", ":some_module", ], - entry_point = ":main.js", + entry_point = ":main.ts", ) sh_test( diff --git a/e2e/typescript_3.1/BUILD.bazel b/e2e/typescript_3.1/BUILD.bazel index 0e1d3a76d0..cb5444307a 100644 --- a/e2e/typescript_3.1/BUILD.bazel +++ b/e2e/typescript_3.1/BUILD.bazel @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary", "rollup_bundle") +load("@build_bazel_rules_nodejs//:defs.bzl", "rollup_bundle") load("@npm_bazel_jasmine//:index.bzl", "jasmine_node_test") load("@npm_bazel_typescript//:index.bzl", "ts_library") @@ -57,23 +57,6 @@ jasmine_node_test( ], ) -nodejs_binary( - name = "main_js", - data = [ - ":main", - ], - entry_point = "e2e_typescript_3_1/main.js", -) - -nodejs_binary( - name = "main_js_sm", - data = [ - ":main", - "@npm//source-map-support", - ], - entry_point = "e2e_typescript_3_1/main.js", -) - rollup_bundle( name = "bundle", entry_point = "main", diff --git a/internal/node/node.bzl b/internal/node/node.bzl index f86154c4fc..3a4cee17da 100644 --- a/internal/node/node.bzl +++ b/internal/node/node.bzl @@ -89,6 +89,13 @@ def _write_loader_script(ctx): if len(ctx.attr.entry_point.files) != 1: fail("labels in entry_point must contain exactly one file") + entry_point_path = expand_path_into_runfiles(ctx, ctx.file.entry_point.short_path) + + # If the entry point specified is a typescript file then set the entry + # point to the corresponding .js file + if entry_point_path.endswith(".ts"): + entry_point_path = entry_point_path[:-3] + ".js" + ctx.actions.expand_template( template = ctx.file._loader_template, output = ctx.outputs.loader, @@ -97,7 +104,7 @@ def _write_loader_script(ctx): "TEMPLATED_bootstrap": "\n " + ",\n ".join( ["\"" + d + "\"" for d in ctx.attr.bootstrap], ), - "TEMPLATED_entry_point": expand_path_into_runfiles(ctx, ctx.file.entry_point.short_path), + "TEMPLATED_entry_point": entry_point_path, "TEMPLATED_gen_dir": ctx.genfiles_dir.path, "TEMPLATED_install_source_map_support": str(ctx.attr.install_source_map_support).lower(), "TEMPLATED_module_roots": "\n " + ",\n ".join(module_mappings), @@ -172,7 +179,11 @@ def _nodejs_binary_impl(ctx): is_executable = True, ) - runfiles = depset([node, ctx.outputs.loader, ctx.file._repository_args, ctx.file.entry_point] + ctx.files._node_runfiles, transitive = [sources]) + runfiles = depset([node, ctx.outputs.loader, ctx.file._repository_args] + ctx.files._node_runfiles, transitive = [sources]) + + # entry point is only needed in runfiles if it is a .js file + if ctx.file.entry_point.short_path.endswith(".js"): + runfiles = depset([ctx.file.entry_point], transitive = [runfiles]) return [DefaultInfo( executable = ctx.outputs.script, @@ -215,32 +226,60 @@ _NODEJS_EXECUTABLE_ATTRS = { "entry_point": attr.label( doc = """The script which should be executed first, usually containing a main function. The `entry_point` accepts a target's name as an entry point. - If the target is a rule, it should produce the JavaScript entry file that will be passed to the nodejs_binary rule). + + If the entry JavaScript file belongs to the same package (as the BUILD file), + you can simply reference it by its relative name to the package directory: + + ``` + nodejs_binary( + name = "my_binary", + ... + entry_point = ":file.js", + ) + ``` + + You can specify the entry point as a typescript file so long as you also include + the ts_library target in data: + + ``` + ts_library( + name = "main", + srcs = ["main.ts"], + ) + + nodejs_binary( + name = "bin", + data = [":main"] + entry_point = ":main.ts", + ) + ``` + + The rule will use the corresponding `.js` output of the ts_library rule as the entry point. + + If the entry point target is a rule, it should produce the JavaScript entry file that will be passed to the nodejs_binary rule). For example: ``` filegroup( name = "entry_file", - srcs = ["workspace/path/to/entry/file"] + srcs = ["main.js"], ) + nodejs_binary( name = "my_binary", - ... entry_point = ":entry_file", ) ``` - If the entry JavaScript file belongs to the same package (as the BUILD file), - you can simply reference it by its relative name to the package directory: + The entry_point can also be a label in another workspace: ``` nodejs_binary( - name = "my_binary", - ... - entry_point = ":file.js", + name = "history-server", + entry_point = "@npm//node_modules/history-server:modules/cli.js", + data = ["@npm//history-server"], ) ``` - """, mandatory = True, allow_single_file = True, diff --git a/internal/npm_install/BUILD.bazel b/internal/npm_install/BUILD.bazel index 2cccbf6c8a..7f45172fad 100644 --- a/internal/npm_install/BUILD.bazel +++ b/internal/npm_install/BUILD.bazel @@ -33,7 +33,7 @@ nodejs_binary( ":browserify-wrapped.js", "//third_party/github.com/browserify/browserify:sources", ], - entry_point = "build_bazel_rules_nodejs/internal/npm_install/browserify-wrapped.js", + entry_point = ":browserify-wrapped.js", install_source_map_support = False, visibility = ["//visibility:public"], ) diff --git a/packages/create/BUILD.bazel b/packages/create/BUILD.bazel index f932fb274d..bfd4dcf86e 100644 --- a/packages/create/BUILD.bazel +++ b/packages/create/BUILD.bazel @@ -27,7 +27,7 @@ nodejs_test( ":npm_package", "@npm//minimist", ], - entry_point = "build_bazel_rules_nodejs/packages/create/test.js", + entry_point = ":test.js", ) # TODO(alexeagle): add e2e testing by running bazel in a newly created project diff --git a/packages/jasmine/BUILD.bazel b/packages/jasmine/BUILD.bazel index f7ac5321c9..02f922473f 100644 --- a/packages/jasmine/BUILD.bazel +++ b/packages/jasmine/BUILD.bazel @@ -60,7 +60,7 @@ npm_package( ) js_library( - name = "jasmine_runner", + name = "jasmine__pkg", srcs = [ "index.js", "src/jasmine_runner.js", diff --git a/packages/jasmine/src/jasmine_node_test.bzl b/packages/jasmine/src/jasmine_node_test.bzl index c17efa7738..d0b5743189 100644 --- a/packages/jasmine/src/jasmine_node_test.bzl +++ b/packages/jasmine/src/jasmine_node_test.bzl @@ -29,22 +29,25 @@ def jasmine_node_test( expected_exit_code = 0, tags = [], coverage = False, - jasmine = "@npm//@bazel/jasmine", + jasmine = "@npm//node_modules/@bazel/jasmine:index.js", **kwargs): """Runs tests in NodeJS using the Jasmine test runner. To debug the test, see debugging notes in `nodejs_test`. Args: - name: name of the resulting label + name: Name of the resulting label srcs: JavaScript source files containing Jasmine specs data: Runtime dependencies which will be loaded while the test executes deps: Other targets which produce JavaScript, such as ts_library expected_exit_code: The expected exit code for the test. Defaults to 0. - tags: bazel tags applied to test - jasmine: a label providing the @bazel/jasmine npm dependency - coverage: Enables code coverage collection and reporting - **kwargs: remaining arguments are passed to the test rule + tags: Bazel tags applied to test + coverage: Enables code coverage collection and reporting. Defaults to False. + jasmine: A label providing the @bazel/jasmine npm dependency. Defaults + to "@npm//@bazel/jasmine" so that the correct jasmine & jasmine-core + dependencies are resolved unless they are overwritten in a bootstrap + file. + **kwargs: Remaining arguments are passed to the test rule """ devmode_js_sources( name = "%s_devmode_srcs" % name, @@ -53,7 +56,7 @@ def jasmine_node_test( tags = tags, ) - all_data = data + srcs + deps + [jasmine] + all_data = data + srcs + deps + [Label(jasmine).relative(":jasmine__pkg")] all_data += [":%s_devmode_srcs.MF" % name] all_data += [Label("@bazel_tools//tools/bash/runfiles")] diff --git a/packages/jasmine/test/BUILD.bazel b/packages/jasmine/test/BUILD.bazel index 6f69df2bf7..058edcd595 100644 --- a/packages/jasmine/test/BUILD.bazel +++ b/packages/jasmine/test/BUILD.bazel @@ -9,7 +9,7 @@ jasmine_node_test( # target if they are not using the default @npm//@bazel/jasmine jasmine = "@npm_bazel_jasmine//:index.js", deps = [ - "//:jasmine_runner", + "//:jasmine__pkg", "@npm//jasmine", ], ) @@ -23,7 +23,7 @@ jasmine_node_test( # target if they are not using the default @npm//@bazel/jasmine jasmine = "@npm_bazel_jasmine//:index.js", deps = [ - "//:jasmine_runner", + "//:jasmine__pkg", "@npm//jasmine", ], ) @@ -37,7 +37,7 @@ jasmine_node_test( # target if they are not using the default @npm//@bazel/jasmine jasmine = "@npm_bazel_jasmine//:index.js", deps = [ - "//:jasmine_runner", + "//:jasmine__pkg", "@npm//jasmine", ], ) @@ -51,7 +51,7 @@ jasmine_node_test( # target if they are not using the default @npm//@bazel/jasmine jasmine = "@npm_bazel_jasmine//:index.js", deps = [ - "//:jasmine_runner", + "//:jasmine__pkg", "@npm//jasmine", ], ) @@ -66,7 +66,7 @@ jasmine_node_test( jasmine = "@npm_bazel_jasmine//:index.js", shard_count = 3, deps = [ - "//:jasmine_runner", + "//:jasmine__pkg", "@npm//jasmine", ], ) @@ -78,7 +78,7 @@ jasmine_node_test( jasmine = "@npm_bazel_jasmine//:index.js", shard_count = 2, deps = [ - "//:jasmine_runner", + "//:jasmine__pkg", "@npm//jasmine", ], ) @@ -102,7 +102,7 @@ jasmine_node_test( # to return a 'incomplete' status tags = ["manual"], deps = [ - "//:jasmine_runner", + "//:jasmine__pkg", "@npm//jasmine", ], ) @@ -116,7 +116,7 @@ jasmine_node_test( coverage = True, jasmine = "@npm_bazel_jasmine//:index.js", deps = [ - "//:jasmine_runner", + "//:jasmine__pkg", "@npm//jasmine", "@npm//v8-coverage", ], diff --git a/packages/labs/package.json b/packages/labs/package.json index 118f87988f..889c2825ef 100644 --- a/packages/labs/package.json +++ b/packages/labs/package.json @@ -11,6 +11,9 @@ "bugs": { "url": "https://github.com/bazelbuild/rules_nodejs/issues" }, + "bin": { + "webpack": "./webpack/src/cli.js" + }, "devDependencies": { "@bazel/typescript": "file:../../dist/npm_bazel_typescript", "@types/jasmine": "^3.3.9", diff --git a/packages/labs/webpack/BUILD.bazel b/packages/labs/webpack/BUILD.bazel index e058ecf5da..6c23a1eea9 100644 --- a/packages/labs/webpack/BUILD.bazel +++ b/packages/labs/webpack/BUILD.bazel @@ -1,5 +1,5 @@ # BEGIN-INTERNAL -load("@build_bazel_rules_nodejs//:defs.bzl", "npm_package") +load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary", "npm_package") npm_package( name = "webpack", @@ -10,9 +10,18 @@ npm_package( replacements = {"#@external\\s": ""}, visibility = ["//:__pkg__"], deps = [ - "//webpack/src:cli", + "//webpack/src:cli_lib", "//webpack/src:package_contents", ], ) +nodejs_binary( + name = "cli", + data = [ + "//webpack/src:cli_lib", + "@npm//webpack", + ], + entry_point = "//webpack/src:cli.ts", + visibility = ["//visibility:public"], +) # END-INTERNAL diff --git a/packages/labs/webpack/src/BUILD.bazel b/packages/labs/webpack/src/BUILD.bazel index d42d6bbf9e..c9aeebed86 100644 --- a/packages/labs/webpack/src/BUILD.bazel +++ b/packages/labs/webpack/src/BUILD.bazel @@ -1,11 +1,12 @@ -load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary") - # BEGIN-INTERNAL load("@npm_bazel_typescript//:defs.bzl", "ts_library") +exports_files(["cli.ts"]) + ts_library( name = "cli_lib", srcs = glob(["*.ts"]), + visibility = ["//:__subpackages__"], deps = [ "@npm//@types", ], @@ -20,24 +21,3 @@ filegroup( visibility = ["//webpack:__subpackages__"], ) # END-INTERNAL - -nodejs_binary( - name = "cli", - data = [ - # BEGIN-INTERNAL - # For local development, we depend on the TypeScript output - ":cli_lib", - # END-INTERNAL - - # For external usage, we depend on the published .JS files - #@external "@npm//@bazel/labs", - "@npm//webpack", - ], - # BEGIN-INTERNAL - # For local development, our module has this AMD name - entry_point = ":cli_lib", - # END-INTERNAL - # For external usage, we resolve the module from node_modules - #@external entry_point = "@bazel/labs/webpack/src:cli_lib", - visibility = ["//visibility:public"], -) diff --git a/packages/labs/webpack/src/cli.ts b/packages/labs/webpack/src/cli.ts index 01b27bc97f..43057154c8 100644 --- a/packages/labs/webpack/src/cli.ts +++ b/packages/labs/webpack/src/cli.ts @@ -21,7 +21,7 @@ function configure(args: string[]): webpack.Configuration { }; } -function main(config: webpack.Configuration): 0|1 { +function compile(config: webpack.Configuration): 0|1 { const compiler = webpack(config); let exitCode: 0|1 = 0; compiler.run((err, stats) => { @@ -41,10 +41,14 @@ function main(config: webpack.Configuration): 0|1 { return exitCode; } -if (require.main === module) { +export function main() { // Avoid limitations of length of argv by using a flagfile // This also makes it easier to debug - you can just look // at this flagfile to see what args were passed to webpack const args = fs.readFileSync(process.argv[2], {encoding: 'utf-8'}).split('\n').map(unquoteArgs); - process.exitCode = main(configure(args)); + process.exitCode = compile(configure(args)); +} + +if (require.main === module) { + main() } diff --git a/packages/labs/webpack/src/webpack_bundle.bzl b/packages/labs/webpack/src/webpack_bundle.bzl index a6410435e5..6b2184da8a 100644 --- a/packages/labs/webpack/src/webpack_bundle.bzl +++ b/packages/labs/webpack/src/webpack_bundle.bzl @@ -20,7 +20,7 @@ This rule is experimental, as part of Angular Labs! There may be breaking change WEBPACK_BUNDLE_ATTRS = { "srcs": attr.label_list(allow_files = True), "entry_point": attr.label(allow_single_file = True, mandatory = True), - "webpack": attr.label(default = "@npm_bazel_labs//webpack/src:cli", executable = True, cfg = "host"), + "webpack": attr.label(default = "@npm//@bazel/labs/bin:webpack", executable = True, cfg = "host"), } WEBPACK_BUNDLE_OUTS = { "bundle": "%{name}.js", diff --git a/packages/labs/webpack/test/BUILD.bazel b/packages/labs/webpack/test/BUILD.bazel index bfb37ace57..f70c338039 100644 --- a/packages/labs/webpack/test/BUILD.bazel +++ b/packages/labs/webpack/test/BUILD.bazel @@ -6,6 +6,11 @@ webpack_bundle( name = "bundle", srcs = glob(["*.js"]), entry_point = "index.js", + # The webpack label here is specific to local testing since + # there is no @bazel/labs npm package here. Users + # will always have this label point to their @foobar//@bazel/labs/bin:webpack + # target if they are not using the default @npm//@bazel/labs/bin:webpack + webpack = "@npm_bazel_labs//webpack:cli", ) ts_library( diff --git a/packages/typescript/WORKSPACE b/packages/typescript/WORKSPACE index e63a64a4fd..c1c6fbe120 100644 --- a/packages/typescript/WORKSPACE +++ b/packages/typescript/WORKSPACE @@ -39,11 +39,11 @@ rules_nodejs_dev_dependencies() # We use git_repository since Renovate knows how to update it. # With http_archive it only sees releases/download/*.tar.gz urls -# TODO(manekinekko): switch to https://github.com/bazelbuild/rules_typescript/ when the changes have been released +# TODO(gregmagolan): switch to https://github.com/bazelbuild/rules_typescript/ HEAD when the changes are ready for release git_repository( name = "build_bazel_rules_typescript", - commit = "68d92c916ef0829113ade3bb24fbf70bd32208de", - remote = "http://github.com/manekinekko/rules_typescript.git", + commit = "586ec251a06852ab4db36003198eec0e20b48389", + remote = "http://github.com/gregmagolan/rules_typescript.git", ) # We have a source dependency on build_bazel_rules_typescript diff --git a/scripts/setup_examples_angular.sh b/scripts/setup_examples_angular.sh index d6e310f3cd..7bfd90a709 100755 --- a/scripts/setup_examples_angular.sh +++ b/scripts/setup_examples_angular.sh @@ -28,7 +28,8 @@ printf "\n\nSetting up /examples/angular\n" # Clean example echo_and_run cd ${EXAMPLES_DIR} rm -rf angular - echo_and_run git clone https://github.com/angular/angular-bazel-example.git angular + # TODO(gmagolan): switch back upstream + echo_and_run git clone --single-branch --branch entry-point https://github.com/gregmagolan/angular-bazel-example.git angular ( echo_and_run cd angular diff --git a/tools/npm_packages/bazel_workspaces/a/BUILD.bazel b/tools/npm_packages/bazel_workspaces/a/BUILD.bazel index 0c0bc63434..f4363550d1 100644 --- a/tools/npm_packages/bazel_workspaces/a/BUILD.bazel +++ b/tools/npm_packages/bazel_workspaces/a/BUILD.bazel @@ -5,5 +5,5 @@ nodejs_binary( data = [ ":index.js", ], - entry_point = "bazel_workspace_a/index.js", + entry_point = ":index.js", ) diff --git a/tools/npm_packages/bazel_workspaces/a/subdir/BUILD.bazel b/tools/npm_packages/bazel_workspaces/a/subdir/BUILD.bazel index 79650d95ef..f4363550d1 100644 --- a/tools/npm_packages/bazel_workspaces/a/subdir/BUILD.bazel +++ b/tools/npm_packages/bazel_workspaces/a/subdir/BUILD.bazel @@ -5,5 +5,5 @@ nodejs_binary( data = [ ":index.js", ], - entry_point = "bazel_workspace_a/subdir/index.js", + entry_point = ":index.js", ) diff --git a/tools/npm_packages/bazel_workspaces/b/BUILD.bazel b/tools/npm_packages/bazel_workspaces/b/BUILD.bazel index 3ab8090ffd..f4363550d1 100644 --- a/tools/npm_packages/bazel_workspaces/b/BUILD.bazel +++ b/tools/npm_packages/bazel_workspaces/b/BUILD.bazel @@ -5,5 +5,5 @@ nodejs_binary( data = [ ":index.js", ], - entry_point = "bazel_workspace_b/index.js", + entry_point = ":index.js", ) diff --git a/tools/npm_packages/bazel_workspaces/b/subdir/BUILD.bazel b/tools/npm_packages/bazel_workspaces/b/subdir/BUILD.bazel index f901d4a405..f4363550d1 100644 --- a/tools/npm_packages/bazel_workspaces/b/subdir/BUILD.bazel +++ b/tools/npm_packages/bazel_workspaces/b/subdir/BUILD.bazel @@ -5,5 +5,5 @@ nodejs_binary( data = [ ":index.js", ], - entry_point = "bazel_workspace_b/subdir/index.js", + entry_point = ":index.js", ) diff --git a/tools/npm_packages/bazel_workspaces/package.json b/tools/npm_packages/bazel_workspaces/package.json index a4538150f1..e78a47f468 100644 --- a/tools/npm_packages/bazel_workspaces/package.json +++ b/tools/npm_packages/bazel_workspaces/package.json @@ -1,6 +1,6 @@ { "name": "bazel_workspace", - "version": "0.0.1", + "version": "0.0.2", "bazelWorkspaces": { "bazel_workspace_a": { "rootPath": "a" diff --git a/yarn.lock b/yarn.lock index 64bffd6049..8a22f33c5a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -264,7 +264,7 @@ base@^0.11.1: pascalcase "^0.1.1" "bazel_workspaces@file:./tools/npm_packages/bazel_workspaces": - version "0.0.1" + version "0.0.2" bn.js@^4.0.0, bn.js@^4.1.0, bn.js@^4.1.1, bn.js@^4.4.0: version "4.11.8"