diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index a4224c515d..14840d2b3d 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -379,7 +379,7 @@ tasks: test_flags: # Firefox not supported on Windows with rules_webtesting (if run it exit with success) # See https://github.com/bazelbuild/rules_webtesting/blob/0.3.3/browsers/BUILD.bazel#L66. - - "--test_tag_filters=-e2e,-examples,-fix-windows,-manual,-browser:firefox-local,-cypress" + - "--test_tag_filters=-e2e,-examples,-fix-windows,-no-bazelci-windows,-manual,-browser:firefox-local,-cypress" test_targets: - "//..." # //internal/node/test:nodejs_toolchain_windows_amd64_test is a "manual" test that must be run diff --git a/index.bzl b/index.bzl index d5be2aa343..8e33324b4d 100644 --- a/index.bzl +++ b/index.bzl @@ -34,6 +34,10 @@ 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/pkg_npm:pkg_npm.bzl", _pkg_npm = "pkg_npm_macro") load("//internal/pkg_web:pkg_web.bzl", _pkg_web = "pkg_web") +load( + "//internal/providers:tree_artifacts.bzl", + _directory_file_path = "directory_file_path", +) check_bazel_version = _check_bazel_version nodejs_binary = _nodejs_binary @@ -46,6 +50,7 @@ copy_to_bin = _copy_to_bin params_file = _params_file generated_file_test = _generated_file_test js_library = _js_library +directory_file_path = _directory_file_path # 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/internal/common/assert.bzl b/internal/common/assert.bzl new file mode 100644 index 0000000000..b3911228e7 --- /dev/null +++ b/internal/common/assert.bzl @@ -0,0 +1,27 @@ +"helpers for test assertions" + +load("@bazel_skylib//rules:diff_test.bzl", "diff_test") +load("@bazel_skylib//rules:write_file.bzl", "write_file") +load("@build_bazel_rules_nodejs//:index.bzl", "npm_package_bin") + +def assert_program_produces_stdout(name, tool, stdout, tags = []): + write_file( + name = "_write_expected_" + name, + out = "expected_" + name, + content = stdout, + tags = tags, + ) + + npm_package_bin( + name = "_write_actual_" + name, + tool = tool, + stdout = "actual_" + name, + tags = tags, + ) + + diff_test( + name = name, + file1 = "expected_" + name, + file2 = "actual_" + name, + tags = tags, + ) diff --git a/internal/node/launcher.sh b/internal/node/launcher.sh index ef168eb583..878fa5ab7a 100644 --- a/internal/node/launcher.sh +++ b/internal/node/launcher.sh @@ -357,6 +357,9 @@ else register_source_map_support=$(rlocation build_bazel_rules_nodejs/third_party/github.com/source-map-support/register.js) LAUNCHER_NODE_OPTIONS+=( "--require" "${register_source_map_support}" ) fi +if [[ -n "TEMPLATED_entry_point_main" ]]; then + MAIN="${MAIN}/"TEMPLATED_entry_point_main +fi # The EXPECTED_EXIT_CODE lets us write bazel tests which assert that # a binary fails to run. Otherwise any failure would make such a test diff --git a/internal/node/node.bzl b/internal/node/node.bzl index a3bebbe31c..946fe1504a 100644 --- a/internal/node/node.bzl +++ b/internal/node/node.bzl @@ -20,7 +20,7 @@ They support module mapping: any targets in the transitive dependencies with a `module_name` attribute can be `require`d by that name. """ -load("//:providers.bzl", "ExternalNpmPackageInfo", "JSModuleInfo", "JSNamedModuleInfo", "NodeRuntimeDepsInfo", "node_modules_aspect") +load("//:providers.bzl", "DirectoryFilePathInfo", "ExternalNpmPackageInfo", "JSModuleInfo", "JSNamedModuleInfo", "NodeRuntimeDepsInfo", "node_modules_aspect") load("//internal/common:expand_into_runfiles.bzl", "expand_location_into_runfiles") load("//internal/common:module_mappings.bzl", "module_mappings_runtime_aspect") load("//internal/common:path_utils.bzl", "strip_external") @@ -106,11 +106,17 @@ def _ts_to_js(entry_point_path): return entry_point_path[:-4] + ".js" return entry_point_path -def _write_loader_script(ctx): - if len(ctx.attr.entry_point.files.to_list()) != 1: +def _get_entry_point_file(ctx): + if len(ctx.attr.entry_point.files.to_list()) > 1: fail("labels in entry_point must contain exactly one file") + if len(ctx.files.entry_point) == 1: + return ctx.files.entry_point[0] + if DirectoryFilePathInfo in ctx.attr.entry_point: + return ctx.attr.entry_point[DirectoryFilePathInfo].directory + fail("entry_point must either be a file, or provide DirectoryFilePathInfo") - entry_point_path = _ts_to_js(_to_manifest_path(ctx, ctx.file.entry_point)) +def _write_loader_script(ctx): + entry_point_path = _ts_to_js(_to_manifest_path(ctx, _get_entry_point_file(ctx))) ctx.actions.expand_template( template = ctx.file._loader_template, @@ -310,8 +316,12 @@ fi #else: # substitutions["TEMPLATED_script_path"] = "$(rlocation \"%s\")" % _to_manifest_path(ctx, ctx.file.entry_point) # For now we need to look in both places - substitutions["TEMPLATED_entry_point_execroot_path"] = "\"%s\"" % _ts_to_js(_to_execroot_path(ctx, ctx.file.entry_point)) - substitutions["TEMPLATED_entry_point_manifest_path"] = "$(rlocation \"%s\")" % _ts_to_js(_to_manifest_path(ctx, ctx.file.entry_point)) + substitutions["TEMPLATED_entry_point_execroot_path"] = "\"%s\"" % _ts_to_js(_to_execroot_path(ctx, _get_entry_point_file(ctx))) + substitutions["TEMPLATED_entry_point_manifest_path"] = "$(rlocation \"%s\")" % _ts_to_js(_to_manifest_path(ctx, _get_entry_point_file(ctx))) + if DirectoryFilePathInfo in ctx.attr.entry_point: + substitutions["TEMPLATED_entry_point_main"] = ctx.attr.entry_point[DirectoryFilePathInfo].path + else: + substitutions["TEMPLATED_entry_point_main"] = "" ctx.actions.expand_template( template = ctx.file._launcher_template, @@ -326,9 +336,10 @@ fi else: executable = ctx.outputs.launcher_sh + # syntax sugar: allows you to avoid repeating the entry point in data # entry point is only needed in runfiles if it is a .js file - if ctx.file.entry_point.extension == "js": - runfiles.append(ctx.file.entry_point) + if len(ctx.files.entry_point) == 1 and ctx.files.entry_point[0].extension == "js": + runfiles.extend(ctx.files.entry_point) return [ DefaultInfo( @@ -351,7 +362,7 @@ fi # TODO(alexeagle): remove sources and node_modules from the runfiles # when downstream usage is ready to rely on linker NodeRuntimeDepsInfo( - deps = depset([ctx.file.entry_point], transitive = [node_modules, sources]), + deps = depset(ctx.files.entry_point, transitive = [node_modules, sources]), pkgs = ctx.attr.data, ), # indicates that the this binary should be instrumented by coverage @@ -462,7 +473,7 @@ nodejs_binary( ``` """, mandatory = True, - allow_single_file = True, + allow_files = True, ), "env": attr.string_dict( doc = """Specifies additional environment variables to set when the target is executed, subject to location diff --git a/internal/node/test/dir_entry_point/BUILD.bazel b/internal/node/test/dir_entry_point/BUILD.bazel new file mode 100644 index 0000000000..5c5924b8d7 --- /dev/null +++ b/internal/node/test/dir_entry_point/BUILD.bazel @@ -0,0 +1,88 @@ +load("@bazel_skylib//rules:write_file.bzl", "write_file") +load("@build_bazel_rules_nodejs//:index.bzl", "directory_file_path", "nodejs_binary", "pkg_npm") +load("@npm//typescript:index.bzl", "tsc") +load("//internal/common:assert.bzl", "assert_program_produces_stdout") + +filegroup( + name = "ts_sources", + srcs = [ + "a.ts", + "b.ts", + ], +) + +# As the test fixture, produce a directory of js files +tsc( + name = "build", + args = [ + "$(execpaths :ts_sources)", + "--outDir", + "$(@D)", + ], + data = [ + ":ts_sources", + "@npm//@types/node", + ], + output_dir = True, +) + +############# +# Test Case 1 +# An pkg_npm can be an entry_point, using the standard npm semantics. +# First, it needs to declare a "main" field in the package.json ... +write_file( + name = "write_package_json", + out = "package.json", + content = [json.encode({ + "main": "build/a.js", + })], +) + +# ... then declare the package ... +pkg_npm( + name = "package", + deps = [ + "build", + "package.json", + ], +) + +# ... and finally run the program with the package as the entry_point. +nodejs_binary( + name = "run_a", + data = ["package"], + entry_point = "package", +) + +# Now just assert that running that program produces the expected stdout +assert_program_produces_stdout( + name = "test_a", + stdout = ["running entry point A"], + # This requires runfiles + tags = ["no-bazelci-windows"], + tool = "run_a", +) + +############# +# Test Case 2 +# Without the pkg_npm, we should still be able to run the program. +# But we need an adapter rule that gives us the equivalent of the "main" field + +directory_file_path( + name = "entry_point_b", + directory = "build", + path = "b.js", +) + +nodejs_binary( + name = "run_b", + data = ["build"], + entry_point = "entry_point_b", +) + +# Now just assert that running that program produces the expected stdout +assert_program_produces_stdout( + name = "test_b", + stdout = ["running entry point B"], + tool = "run_b", +) diff --git a/internal/node/test/dir_entry_point/a.ts b/internal/node/test/dir_entry_point/a.ts new file mode 100644 index 0000000000..d4813de6d2 --- /dev/null +++ b/internal/node/test/dir_entry_point/a.ts @@ -0,0 +1,3 @@ +if (require.main === module) { + process.stdout.write('running entry point A') +} diff --git a/internal/node/test/dir_entry_point/b.ts b/internal/node/test/dir_entry_point/b.ts new file mode 100644 index 0000000000..c2b03753ea --- /dev/null +++ b/internal/node/test/dir_entry_point/b.ts @@ -0,0 +1,3 @@ +if (require.main === module) { + process.stdout.write('running entry point B') +} diff --git a/internal/providers/tree_artifacts.bzl b/internal/providers/tree_artifacts.bzl new file mode 100644 index 0000000000..80257cc985 --- /dev/null +++ b/internal/providers/tree_artifacts.bzl @@ -0,0 +1,52 @@ +# 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. + +"""This module contains providers for working with TreeArtifacts. + +See https://github.com/bazelbuild/bazel-skylib/issues/300 +(this feature could be upstreamed to bazel-skylib in the future) + +These are also called output directories, created by `ctx.actions.declare_directory`. +""" + +DirectoryFilePathInfo = provider( + doc = "Joins a label pointing to a TreeArtifact with a path nested within that directory.", + fields = { + "directory": "a TreeArtifact (ctx.actions.declare_directory)", + "path": "path relative to the directory", + }, +) + +def _directory_file_path(ctx): + if not ctx.file.directory.is_directory: + fail("directory attribute must be created with Bazel declare_directory (TreeArtifact)") + return [DirectoryFilePathInfo(path = ctx.attr.path, directory = ctx.file.directory)] + +directory_file_path = rule( + doc = """Provide DirectoryFilePathInfo to reference some file within a directory. + + Otherwise there is no way to give a Bazel label for it.""", + implementation = _directory_file_path, + attrs = { + "directory": attr.label( + doc = "a directory", + mandatory = True, + allow_single_file = True, + ), + "path": attr.string( + doc = "a path within that directory", + mandatory = True, + ), + }, +) diff --git a/providers.bzl b/providers.bzl index 0fa7151f76..9ba545f5fd 100644 --- a/providers.bzl +++ b/providers.bzl @@ -46,6 +46,10 @@ load( _NodeRuntimeDepsInfo = "NodeRuntimeDepsInfo", _run_node = "run_node", ) +load( + "//internal/providers:tree_artifacts.bzl", + _DirectoryFilePathInfo = "DirectoryFilePathInfo", +) DeclarationInfo = _DeclarationInfo declaration_info = _declaration_info @@ -88,3 +92,4 @@ to create a NodeContextInfo. NodeRuntimeDepsInfo = _NodeRuntimeDepsInfo run_node = _run_node +DirectoryFilePathInfo = _DirectoryFilePathInfo diff --git a/stardoc.patch b/stardoc.patch deleted file mode 100644 index 2d35e4f954..0000000000 --- a/stardoc.patch +++ /dev/null @@ -1,13 +0,0 @@ -diff --git stardoc/stardoc.bzl stardoc/stardoc.bzl -index 456aca7..a065104 100644 ---- stardoc/stardoc.bzl -+++ stardoc/stardoc.bzl -@@ -32,7 +32,7 @@ def _stardoc_impl(ctx): - ]) - stardoc_args = ctx.actions.args() - stardoc_args.add("--input=" + str(ctx.file.input.owner)) -- stardoc_args.add("--workspace_name=" + ctx.label.workspace_name) -+ stardoc_args.add("--workspace_name=" + ctx.workspace_name) - stardoc_args.add_all( - ctx.attr.symbol_names, - format_each = "--symbols=%s",