From 9cbf5c648f0104d3e2dfc2fc3bf267e42fc261dc Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Fri, 21 Jun 2024 15:09:45 -0700 Subject: [PATCH] perf: change js_helpers gather_* utils to accept list, not depset (#1787) --- js/private/js_binary.bzl | 2 -- js/private/js_helpers.bzl | 27 ++++++++++------------ js/private/js_library.bzl | 46 +++++++++++++++++++++++-------------- npm/private/npm_package.bzl | 14 ++++------- 4 files changed, 45 insertions(+), 44 deletions(-) diff --git a/js/private/js_binary.bzl b/js/private/js_binary.bzl index 4b4732cb5..aa2191b09 100644 --- a/js/private/js_binary.bzl +++ b/js/private/js_binary.bzl @@ -525,10 +525,8 @@ def _create_launcher(ctx, log_prefix_rule_set, log_prefix_rule, fixed_args = [], runfiles = gather_runfiles( ctx = ctx, - sources = [], data = ctx.attr.data, data_files = [entry_point] + ctx.files.data, - deps = [], copy_data_files_to_bin = ctx.attr.copy_data_to_bin, no_copy_to_bin = ctx.files.no_copy_to_bin, include_sources = ctx.attr.include_sources, diff --git a/js/private/js_helpers.bzl b/js/private/js_helpers.bzl index c4b6eab0a..c3a70ef8c 100644 --- a/js/private/js_helpers.bzl +++ b/js/private/js_helpers.bzl @@ -8,39 +8,35 @@ def gather_transitive_sources(sources, targets): """Gathers transitive sources from a list of direct sources and targets Args: - sources: list or depset of direct sources which should be included in `transitive_sources` + sources: list of direct sources which should be included in `transitive_sources` targets: list of targets to gather `transitive_sources` from `JsInfo` Returns: A depset of transitive sources """ - if type(sources) == "list": - sources = depset(sources) transitive = [ target[JsInfo].transitive_sources for target in targets if JsInfo in target ] - return depset(transitive = [sources] + transitive) + return depset(sources, transitive = transitive) def gather_transitive_types(types, targets): """Gathers transitive types from a list of direct types and targets Args: - types: list or depset of direct sources which should be included in `transitive_types` + types: list of direct sources which should be included in `transitive_types` targets: list of targets to gather `transitive_types` from `JsInfo` Returns: A depset of transitive sources """ - if type(types) == "list": - types = depset(types) transitive = [ target[JsInfo].transitive_types for target in targets if JsInfo in target ] - return depset(transitive = [types] + transitive) + return depset(types, transitive = transitive) def gather_npm_sources(srcs, deps): """Gathers npm sources from a list of srcs and deps targets @@ -116,9 +112,9 @@ this option is not needed. def gather_runfiles( ctx, - sources, - data, - deps, + sources = None, + data = [], + deps = [], data_files = [], copy_data_files_to_bin = False, no_copy_to_bin = [], @@ -138,7 +134,7 @@ def gather_runfiles( Args: ctx: the rule context - sources: list or depset of files which should be included in runfiles + sources: depset which should be included in runfiles deps: list of dependency targets; only transitive runfiles are gather from these targets @@ -176,10 +172,11 @@ def gather_runfiles( A [runfiles](https://bazel.build/rules/lib/runfiles) object created with [ctx.runfiles](https://bazel.build/rules/lib/ctx#runfiles). """ + transitive_files_depsets = [] + # Includes sources - if type(sources) == "list": - sources = depset(sources) - transitive_files_depsets = [sources] + if sources: + transitive_files_depsets.append(sources) # Gather the default outputs of data targets for target in data: diff --git a/js/private/js_library.bzl b/js/private/js_library.bzl index 47fc76f19..60db7868b 100644 --- a/js/private/js_library.bzl +++ b/js/private/js_library.bzl @@ -24,7 +24,7 @@ js_library( """ load("@aspect_bazel_lib//lib:copy_to_bin.bzl", "COPY_FILE_TO_BIN_TOOLCHAINS") -load(":js_helpers.bzl", "copy_js_file_to_bin_action", "gather_npm_package_store_infos", "gather_npm_sources", "gather_runfiles", "gather_transitive_sources", "gather_transitive_types") +load(":js_helpers.bzl", "copy_js_file_to_bin_action", "gather_runfiles") load(":js_info.bzl", "JsInfo", "js_info") _DOC = """A library of JavaScript sources. Provides JsInfo, the primary provider used in rules_js @@ -204,33 +204,45 @@ def _js_library_impl(ctx): files = ctx.files.types, ) + # Direct sources and types sources = depset(transitive = [sources, additional_sources]) types = depset(transitive = [types, additional_sources, additional_types]) - transitive_sources = gather_transitive_sources( - sources = sources, - targets = ctx.attr.srcs + ctx.attr.types + ctx.attr.deps, - ) + # Transitive sources and types + transitive_sources = [sources] + transitive_types = [types] - transitive_types = gather_transitive_types( - types = types, - targets = ctx.attr.srcs + ctx.attr.types + ctx.attr.deps, - ) + # npm providers + npm_sources = [] + npm_package_store_infos = [] - npm_sources = gather_npm_sources( - srcs = ctx.attr.srcs + ctx.attr.types, - deps = ctx.attr.deps, - ) + # Concat the srcs+types+deps once + srcs_types_deps = ctx.attr.srcs + ctx.attr.types + ctx.attr.deps - npm_package_store_infos = gather_npm_package_store_infos( - targets = ctx.attr.srcs + ctx.attr.data + ctx.attr.deps, - ) + # Collect transitive sources, types and npm providers from srcs+types+deps + for target in srcs_types_deps: + if JsInfo in target: + jsinfo = target[JsInfo] + transitive_sources.append(jsinfo.transitive_sources) + transitive_types.append(jsinfo.transitive_types) + npm_sources.append(jsinfo.npm_sources) + npm_package_store_infos.append(jsinfo.npm_package_store_infos) + + # Also add npm_package_store_infos from ctx.attr.data + for target in ctx.attr.data: + if JsInfo in target: + npm_package_store_infos.append(target[JsInfo].npm_package_store_infos) + + transitive_sources = depset(transitive = transitive_sources) + transitive_types = depset(transitive = transitive_types) + npm_sources = depset(transitive = npm_sources) + npm_package_store_infos = depset(transitive = npm_package_store_infos) runfiles = gather_runfiles( ctx = ctx, sources = transitive_sources, data = ctx.attr.data, - deps = ctx.attr.srcs + ctx.attr.types + ctx.attr.deps, + deps = srcs_types_deps, data_files = ctx.files.data, copy_data_files_to_bin = ctx.attr.copy_data_to_bin, no_copy_to_bin = ctx.files.no_copy_to_bin, diff --git a/npm/private/npm_package.bzl b/npm/private/npm_package.bzl index a33d3cd7f..7da689e59 100644 --- a/npm/private/npm_package.bzl +++ b/npm/private/npm_package.bzl @@ -16,7 +16,6 @@ load("@bazel_skylib//lib:dicts.bzl", "dicts") load("@bazel_skylib//lib:versions.bzl", "versions") load("//js:defs.bzl", "js_binary") load("//js:libs.bzl", "js_lib_helpers") -load("//js:providers.bzl", "JsInfo") load(":npm_package_info.bzl", "NpmPackageInfo") # Pull in all copy_to_directory attributes except for exclude_prefixes @@ -70,14 +69,9 @@ def _npm_package_impl(ctx): dst = ctx.actions.declare_directory(ctx.attr.out if ctx.attr.out else ctx.attr.name) # forward all npm_package_store_infos - npm_package_store_infos = [ - target[JsInfo].npm_package_store_infos - for target in ctx.attr.srcs - if JsInfo in target - ] - npm_package_store_infos.append(js_lib_helpers.gather_npm_package_store_infos( - targets = ctx.attr.data, - )) + npm_package_store_infos = js_lib_helpers.gather_npm_package_store_infos( + targets = ctx.attr.srcs + ctx.attr.data, + ) copy_to_directory_bin_action( ctx, @@ -109,7 +103,7 @@ def _npm_package_impl(ctx): package = ctx.attr.package, version = ctx.attr.version, src = dst, - npm_package_store_infos = depset(transitive = npm_package_store_infos), + npm_package_store_infos = npm_package_store_infos, ), ]