Skip to content

Commit

Permalink
fix(builtin): pkg_npm shouldn't assume the name of the nodejs toolcha…
Browse files Browse the repository at this point in the history
…in (#3129)

Instead it should just ask the resolved toolchain for the info it needs
  • Loading branch information
alexeagle authored Dec 22, 2021
1 parent 90c7fe0 commit 552178e
Show file tree
Hide file tree
Showing 8 changed files with 68 additions and 31 deletions.
8 changes: 8 additions & 0 deletions examples/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,10 @@ example_integration_test(
example_integration_test(
name = "examples_vue",
npm_packages = {},
repositories = {
"//:release": "build_bazel_rules_nodejs",
"//:release-core": "rules_nodejs",
},
)

example_integration_test(
Expand All @@ -249,6 +253,10 @@ example_integration_test(
"@alan-agius4",
"@jbedard",
],
repositories = {
"//:release": "build_bazel_rules_nodejs",
"//:release-core": "rules_nodejs",
},
tags = [
# TODO(alexeagle): re-enable when it stops timing out on 4.x branch
"manual",
Expand Down
15 changes: 15 additions & 0 deletions examples/angular_bazel_architect/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,14 @@ http_archive(
urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/4.4.0/rules_nodejs-4.4.0.tar.gz"],
)

# Temporary state: this example needs to have both build_bazel_rules_nodejs and rules_nodejs.
# Once we have cut over the toolchains to rules_nodejs only, we shouldn't need this.
http_archive(
name = "rules_nodejs",
sha256 = "a2b1b60c51b0193ed1646accf77a28cfd4f4ce1f6c86f32ce11455101be3a9c4",
urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/4.4.3/rules_nodejs-core-4.4.3.tar.gz"],
)

load("@build_bazel_rules_nodejs//nodejs:repositories.bzl", "rules_nodejs_dependencies")

rules_nodejs_dependencies()
Expand All @@ -36,3 +44,10 @@ yarn_install(
symlink_node_modules = False,
yarn_lock = "//:yarn.lock",
)

load("@rules_nodejs//nodejs:repositories.bzl", "nodejs_register_toolchains")

nodejs_register_toolchains(
name = "node16",
node_version = "16.9.0",
)
15 changes: 15 additions & 0 deletions examples/vue/WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,25 @@ http_archive(
urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/4.4.0/rules_nodejs-4.4.0.tar.gz"],
)

# Temporary state: this example needs to have both build_bazel_rules_nodejs and rules_nodejs.
# Once we have cut over the toolchains to rules_nodejs only, we shouldn't need this.
http_archive(
name = "rules_nodejs",
sha256 = "a2b1b60c51b0193ed1646accf77a28cfd4f4ce1f6c86f32ce11455101be3a9c4",
urls = ["https://github.com/bazelbuild/rules_nodejs/releases/download/4.4.3/rules_nodejs-core-4.4.3.tar.gz"],
)

load("@build_bazel_rules_nodejs//nodejs:repositories.bzl", "rules_nodejs_dependencies")

rules_nodejs_dependencies()

load("@rules_nodejs//nodejs:repositories.bzl", "nodejs_register_toolchains")

nodejs_register_toolchains(
name = "node16",
node_version = "16.9.0",
)

load("@build_bazel_rules_nodejs//:index.bzl", "node_repositories", "npm_install")

node_repositories(
Expand Down
9 changes: 3 additions & 6 deletions internal/pkg_npm/npm_script_generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ const fs = require('fs');

function main(args) {
const
[outDir, packPath, publishPath, runNpmTemplatePath, packBatPath, publishBatPath,
runNpmBatTemplatePath] = args;
[outDir, packPath, publishPath, runNpmTemplatePath, packBatPath, publishBatPath] = args;
const cwd = process.cwd();
if (/[\//]sandbox[\//]/.test(cwd)) {
console.error('Error: npm_script_generator must be run with no sandbox');
Expand All @@ -36,10 +35,8 @@ function main(args) {
fs.writeFileSync(packPath, npmTemplate.replace('TMPL_args', `pack "${cwd}/${outDir}"`));
fs.writeFileSync(publishPath, npmTemplate.replace('TMPL_args', `publish "${cwd}/${outDir}"`));

const npmBatTemplate = fs.readFileSync(runNpmBatTemplatePath, {encoding: 'utf-8'});
fs.writeFileSync(packBatPath, npmBatTemplate.replace('TMPL_args', `pack "${cwd}/${outDir}"`));
fs.writeFileSync(
publishBatPath, npmBatTemplate.replace('TMPL_args', `publish "${cwd}/${outDir}"`));
fs.writeFileSync(packBatPath, npmTemplate.replace('TMPL_args', `pack "${cwd}/${outDir}"`));
fs.writeFileSync(publishBatPath, npmTemplate.replace('TMPL_args', `publish "${cwd}/${outDir}"`));
}

if (require.main === module) {
Expand Down
19 changes: 6 additions & 13 deletions internal/pkg_npm/pkg_npm.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -143,22 +143,14 @@ See the section on stamping in the [README](stamping)
),
"_npm_script_generator": attr.label(
default = Label("//internal/pkg_npm:npm_script_generator"),
cfg = "host",
cfg = "exec",
executable = True,
),
"_packager": attr.label(
default = Label("//internal/pkg_npm:packager"),
cfg = "host",
cfg = "exec",
executable = True,
),
"_run_npm_bat_template": attr.label(
default = Label("@nodejs//:run_npm.bat.template"),
allow_single_file = True,
),
"_run_npm_template": attr.label(
default = Label("@nodejs//:run_npm.sh.template"),
allow_single_file = True,
),
})

# Used in angular/angular /packages/bazel/src/ng_package/ng_package.bzl
Expand Down Expand Up @@ -283,22 +275,22 @@ def create_package(ctx, deps_files, nested_packages):

def _create_npm_scripts(ctx, package_dir):
args = ctx.actions.args()
toolchain = ctx.toolchains["@rules_nodejs//nodejs:toolchain_type"].nodeinfo

args.add_all([
package_dir.path,
ctx.outputs.pack_sh.path,
ctx.outputs.publish_sh.path,
ctx.file._run_npm_template.path,
toolchain.run_npm.path,
ctx.outputs.pack_bat.path,
ctx.outputs.publish_bat.path,
ctx.file._run_npm_bat_template.path,
])

ctx.actions.run(
progress_message = "Generating npm pack & publish scripts",
mnemonic = "GenerateNpmScripts",
executable = ctx.executable._npm_script_generator,
inputs = [ctx.file._run_npm_template, ctx.file._run_npm_bat_template, package_dir],
inputs = [toolchain.run_npm, package_dir],
outputs = [ctx.outputs.pack_sh, ctx.outputs.publish_sh, ctx.outputs.pack_bat, ctx.outputs.publish_bat],
arguments = [args],
# Must be run local (no sandbox) so that the pwd is the actual execroot
Expand Down Expand Up @@ -362,6 +354,7 @@ pkg_npm = rule(
attrs = PKG_NPM_ATTRS,
doc = _DOC,
outputs = PKG_NPM_OUTPUTS,
toolchains = ["@rules_nodejs//nodejs:toolchain_type"],
)

def pkg_npm_macro(name, tgz = None, **kwargs):
Expand Down
1 change: 0 additions & 1 deletion nodejs/private/toolchains_repo.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ resolved_toolchain(name = "resolved_toolchain", visibility = ["//visibility:publ
toolchain(
name = "{platform}_toolchain",
exec_compatible_with = {compatible_with},
target_compatible_with = {compatible_with},
toolchain = "@{user_node_repository_name}_{platform}//:node_toolchain",
toolchain_type = "@rules_nodejs//nodejs:toolchain_type",
)
Expand Down
25 changes: 14 additions & 11 deletions nodejs/repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -521,16 +521,16 @@ if %errorlevel% neq 0 exit /b %errorlevel%

# This template file is used by the packager tool and the pkg_npm rule.
# `yarn publish` is not ready for use under Bazel, see https://github.com/yarnpkg/yarn/issues/610
repository_ctx.file("run_npm.sh.template", content = """
if repository_ctx.attr.platform.startswith("windows"):
run_npm = """
"{node}" "{script}" TMPL_args %*
"""
else:
run_npm = """
"{node}" "{script}" TMPL_args "$@"
""".format(
node = repository_ctx.path(node_entry),
script = repository_ctx.path(npm_script),
))
"""

repository_ctx.file("run_npm.bat.template", content = """
"{node}" "{script}" TMPL_args %*
""".format(
repository_ctx.file("run_npm.template", content = run_npm.format(
node = repository_ctx.path(node_entry),
script = repository_ctx.path(npm_script),
))
Expand Down Expand Up @@ -616,8 +616,7 @@ if %errorlevel% neq 0 exit /b %errorlevel%
build_content = """# Generated by node_repositories.bzl
package(default_visibility = ["//visibility:public"])
exports_files([
"run_npm.sh.template",
"run_npm.bat.template",
"run_npm.template",
"{node_entry}",
"{npm_entry}",
"{yarn_entry}",
Expand Down Expand Up @@ -667,7 +666,11 @@ filegroup(
if repository_ctx.attr.platform:
build_content += """
load("@rules_nodejs//nodejs:toolchain.bzl", "node_toolchain")
node_toolchain(name = "node_toolchain", target_tool = ":node_bin")
node_toolchain(
name = "node_toolchain",
target_tool = ":node_bin",
run_npm = ":run_npm.template",
)
"""
repository_ctx.file("BUILD.bazel", content = build_content)

Expand Down
7 changes: 7 additions & 0 deletions nodejs/toolchain.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ NodeInfo = provider(
"tool_files": """Files required in runfiles to make the nodejs executable available.
May be empty if the target_tool_path points to a locally installed node binary.""",
"run_npm": """A template for a script that wraps npm.
On Windows, this is a Batch script, otherwise it uses Bash.""",
},
)

Expand Down Expand Up @@ -57,6 +59,7 @@ def _node_toolchain_impl(ctx):
nodeinfo = NodeInfo(
target_tool_path = target_tool_path,
tool_files = tool_files,
run_npm = ctx.file.run_npm,
)

# Export all the providers inside our ToolchainInfo
Expand Down Expand Up @@ -84,6 +87,10 @@ node_toolchain = rule(
doc = "Path to an existing nodejs executable for the target platform.",
mandatory = False,
),
"run_npm": attr.label(
doc = "A template file that allows us to execute npm",
allow_single_file = True,
),
},
doc = """Defines a node toolchain.
Expand Down

0 comments on commit 552178e

Please sign in to comment.