feat(builtin): document how nodejs_binary#entry_point can use a direc… (

#2579)

* feat(builtin): document how nodejs_binary#entry_point can use a directory

* refactor: rename to directory_entry_point per discussion

* refactor: make the feature more general, suitable to upstream to bazel-skylib

.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

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

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,
+    )

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

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

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",
+)

internal/node/test/dir_entry_point/a.ts

@@ -0,0 +1,3 @@
+if (require.main === module) {
+  process.stdout.write('running entry point A')
+}

internal/node/test/dir_entry_point/b.ts

@@ -0,0 +1,3 @@
+if (require.main === module) {
+  process.stdout.write('running entry point 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,
+        ),
+    },
+)

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