Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: change js_helpers gather_* utils to accept list, not depset #1787

Merged
merged 2 commits into from
Jun 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions js/private/js_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
27 changes: 12 additions & 15 deletions js/private/js_helpers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 = [],
Expand All @@ -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

Expand Down Expand Up @@ -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:
Expand Down
46 changes: 29 additions & 17 deletions js/private/js_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 4 additions & 10 deletions npm/private/npm_package.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
),
]

Expand Down
Loading