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.

Sign up for GitHub

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

refactor: deprecate npm_files list attribute in favor of npm_srcs depset #3738

Merged
merged 1 commit into from
Apr 3, 2024
Merged

refactor: deprecate npm_files list attribute in favor of npm_srcs depset #3738

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions docs/Core.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ Additional parameters
**USAGE**

<pre>
node_toolchain(<a href="#node_toolchain-name">name</a>, <a href="#node_toolchain-node">node</a>, <a href="#node_toolchain-node_path">node_path</a>, <a href="#node_toolchain-npm">npm</a>, <a href="#node_toolchain-npm_path">npm_path</a>, <a href="#node_toolchain-npm_files">npm_files</a>, <a href="#node_toolchain-headers">headers</a>, <a href="#node_toolchain-kwargs">kwargs</a>)
node_toolchain(<a href="#node_toolchain-name">name</a>, <a href="#node_toolchain-node">node</a>, <a href="#node_toolchain-node_path">node_path</a>, <a href="#node_toolchain-npm">npm</a>, <a href="#node_toolchain-npm_path">npm_path</a>, <a href="#node_toolchain-npm_srcs">npm_srcs</a>, <a href="#node_toolchain-headers">headers</a>, <a href="#node_toolchain-kwargs">kwargs</a>)
</pre>

Defines a node toolchain for a platform.
Expand Down Expand Up @@ -232,17 +232,17 @@ Defaults to `None`

<h4 id="node_toolchain-npm_path">npm_path</h4>

Path to npm JavaScript entry point
Path to npm JavaScript entry point.

This is typically an absolute path to a non-hermetic npm installation.

Only one of `npm` and `npm_path` may be set.

Defaults to `""`

<h4 id="node_toolchain-npm_files">npm_files</h4>
<h4 id="node_toolchain-npm_srcs">npm_srcs</h4>

Additional files required to run npm
Additional source files required to run npm.

Not necessary if specifying `npm_path` to a non-hermetic npm installation.

Expand Down
5 changes: 2 additions & 3 deletions nodejs/repositories.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ filegroup(
)
filegroup(
name = "npm_files",
srcs = {npm_files_glob}[":node_files"],
srcs = glob(["bin/nodejs/**"]) + [":node_files"],
)
cc_library(
name = "headers",
Expand All @@ -261,7 +261,6 @@ cc_library(
node_bin_export = "\n \"%s\"," % node_bin,
npm_bin_export = "\n \"%s\"," % npm_bin,
npx_bin_export = "\n \"%s\"," % npx_bin,
npm_files_glob = "glob([\"bin/nodejs/**\"]) + ",
node_bin_label = node_bin_label,
npm_bin_label = npm_bin_label,
npx_bin_label = npx_bin_label,
Expand All @@ -277,7 +276,7 @@ node_toolchain(
name = "node_toolchain",
node = ":node_bin",
npm = ":npm",
npm_files = [":npm_files"],
npm_srcs = [":npm_files"],
headers = ":headers",
)
"""
Expand Down
25 changes: 18 additions & 7 deletions nodejs/toolchain.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ For backward compability, if set then npm_path will be set to the runfiles path

For backward compability, npm_path is set to the runfiles path of npm if npm is set.
""",
"npm_files": """Additional files required to run npm""",
"npm_srcs": """Additional source files required to run npm""",
"headers": """\
(struct) Information about the header files, with fields:
* providers_map: a dict of string to provider instances. The key should be
Expand All @@ -57,6 +57,7 @@ For backward compability, npm_path is set to the runfiles path of npm if npm is
# DEPRECATED
"target_tool_path": "(DEPRECATED) Path to Node.js executable for backward compatibility",
"tool_files": """(DEPRECATED) Alias for [node] for backward compatibility""",
"npm_files": """(DEPRECATED) Alias for npm_srcs.to_list()""",
},
)

Expand Down Expand Up @@ -85,12 +86,13 @@ def _nodejs_toolchain_impl(ctx):
files = depset([ctx.file.node]) if ctx.attr.node else depset(),
runfiles = ctx.runfiles(files = [ctx.file.node] if ctx.attr.node else []),
)
npm_srcs = depset([ctx.file.npm] + ctx.files.npm_srcs)
nodeinfo = NodeInfo(
node = ctx.file.node,
node_path = ctx.attr.node_path,
npm = ctx.file.npm,
npm_path = ctx.attr.npm_path if ctx.attr.npm_path else _to_manifest_path(ctx, ctx.file.npm), # _to_manifest_path for backward compat
npm_files = depset([ctx.file.npm] + ctx.files.npm_files).to_list() if ctx.attr.npm else [],
npm_srcs = npm_srcs,
headers = struct(
providers_map = {
"CcInfo": ctx.attr.headers[CcInfo],
Expand All @@ -100,6 +102,7 @@ def _nodejs_toolchain_impl(ctx):
# For backward compat
target_tool_path = _to_manifest_path(ctx, ctx.file.node) if ctx.attr.node else ctx.attr.node_path,
tool_files = [ctx.file.node] if ctx.attr.node else [],
npm_files = npm_srcs.to_list(),
)

# Export all the providers inside our ToolchainInfo
Expand All @@ -126,7 +129,7 @@ _nodejs_toolchain = rule(
"node_path": attr.string(),
"npm": attr.label(allow_single_file = True),
"npm_path": attr.string(),
"npm_files": attr.label_list(),
"npm_srcs": attr.label_list(),
"headers": attr.label(),
},
)
Expand All @@ -137,7 +140,7 @@ def node_toolchain(
node_path = "",
npm = None,
npm_path = "",
npm_files = [],
npm_srcs = [],
headers = None,
**kwargs):
"""Defines a node toolchain for a platform.
Expand Down Expand Up @@ -192,13 +195,13 @@ def node_toolchain(

npm: Npm JavaScript entry point

npm_path: Path to npm JavaScript entry point
npm_path: Path to npm JavaScript entry point.

This is typically an absolute path to a non-hermetic npm installation.

Only one of `npm` and `npm_path` may be set.

npm_files: Additional files required to run npm
npm_srcs: Additional source files required to run npm.

Not necessary if specifying `npm_path` to a non-hermetic npm installation.

Expand All @@ -224,13 +227,21 @@ WARNING: target_tool_path attribute of node_toolchain is deprecated; use node_pa
""")
node_path = target_tool_path

npm_files = kwargs.pop("npm_files", "")
if npm_files:
# buildifier: disable=print
print("""\
WARNING: npm_files attribute of node_toolchain is deprecated; use npm_srcs instead of npm_files
""")
npm_srcs = npm_files

_nodejs_toolchain(
name = name,
node = node,
node_path = node_path,
npm = npm,
npm_path = npm_path,
npm_files = npm_files,
npm_srcs = npm_srcs,
headers = headers,
**kwargs
)
Loading