From 16c749f8152dac6138eada22b4582d996058b7ab Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Tue, 25 May 2021 13:53:55 -0700 Subject: [PATCH] refactor: make the feature more general, suitable to upstream to bazel-skylib --- index.bzl | 5 ++ internal/node/node.bzl | 35 +++---------- .../node/test/dir_entry_point/BUILD.bazel | 10 ++-- internal/providers/tree_artifacts.bzl | 52 +++++++++++++++++++ providers.bzl | 5 ++ 5 files changed, 71 insertions(+), 36 deletions(-) create mode 100644 internal/providers/tree_artifacts.bzl 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/node/node.bzl b/internal/node/node.bzl index 7534f3306a..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") @@ -111,9 +111,9 @@ def _get_entry_point_file(ctx): fail("labels in entry_point must contain exactly one file") if len(ctx.files.entry_point) == 1: return ctx.files.entry_point[0] - if EntryPointInfo in ctx.attr.entry_point: - return ctx.attr.entry_point[EntryPointInfo].directory - fail("entry_point must either be a file, or produce EntryPointInfo") + 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") def _write_loader_script(ctx): entry_point_path = _ts_to_js(_to_manifest_path(ctx, _get_entry_point_file(ctx))) @@ -318,8 +318,8 @@ fi # For now we need to look in both places 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 EntryPointInfo in ctx.attr.entry_point: - substitutions["TEMPLATED_entry_point_main"] = ctx.attr.entry_point[EntryPointInfo].entry_point + if DirectoryFilePathInfo in ctx.attr.entry_point: + substitutions["TEMPLATED_entry_point_main"] = ctx.attr.entry_point[DirectoryFilePathInfo].path else: substitutions["TEMPLATED_entry_point_main"] = "" @@ -685,26 +685,3 @@ remote debugger. "@bazel_tools//tools/sh:toolchain_type", ], ) - -EntryPointInfo = provider( - doc = "Tells runners about what JS file is the entry point, or main where node should begin evaluation", - fields = { - "directory": "a TreeArtifact (ctx.actions.declare_directory) containing the entry point", - "entry_point": "path relative to the directory, same as package.json's main/module fields", - }, -) - -def _directory_entry_point(ctx): - if not ctx.file.directory.is_directory: - fail("directory attribute must be created with Bazel declare_directory (TreeArtifact)") - return [EntryPointInfo(entry_point = ctx.attr.entry_point, directory = ctx.file.directory)] - -directory_entry_point = rule( - doc = """Provide EntryPointInfo to give an entry_point within a directory. - Otherwise there is no way to give a Bazel label for it.""", - implementation = _directory_entry_point, - attrs = { - "directory": attr.label(doc = "a directory containing the entry point", mandatory = True, allow_single_file = True), - "entry_point": attr.string(doc = "entry point for the program, same as package.json like main/module", mandatory = True), - }, -) diff --git a/internal/node/test/dir_entry_point/BUILD.bazel b/internal/node/test/dir_entry_point/BUILD.bazel index 63140bc7fa..5c5924b8d7 100644 --- a/internal/node/test/dir_entry_point/BUILD.bazel +++ b/internal/node/test/dir_entry_point/BUILD.bazel @@ -1,12 +1,8 @@ load("@bazel_skylib//rules:write_file.bzl", "write_file") -load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_binary", "pkg_npm") +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") -# For now this is an internal API -# TODO: decide if package.json is sufficient for all use cases -load("//internal/node:node.bzl", "directory_entry_point") - filegroup( name = "ts_sources", srcs = [ @@ -72,10 +68,10 @@ assert_program_produces_stdout( # 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_entry_point( +directory_file_path( name = "entry_point_b", directory = "build", - entry_point = "b.js", + path = "b.js", ) nodejs_binary( 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