Skip to content

Commit

Permalink
fix(builtin): pkg_npm shouldn't assume the name of the nodejs toolchain
Browse files Browse the repository at this point in the history
Instead it should just ask the resolved toolchain for the info it needs
  • Loading branch information
alexeagle committed Dec 22, 2021
1 parent 90c7fe0 commit 4034eaa
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 4034eaa

Please sign in to comment.