From 053609a758b2e78d8113d8c0461b068f47d7c2e1 Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Thu, 27 Jun 2019 02:23:21 -0700 Subject: [PATCH] fix(builtin): changes on review --- internal/node/node_repositories.bzl | 17 ++++++------ toolchains/node/BUILD.bazel | 2 +- toolchains/node/node_configure.bzl | 40 ++++++----------------------- toolchains/node/node_toolchain.bzl | 3 +++ 4 files changed, 21 insertions(+), 41 deletions(-) diff --git a/internal/node/node_repositories.bzl b/internal/node/node_repositories.bzl index 6c88c141b9..011a806a38 100644 --- a/internal/node/node_repositories.bzl +++ b/internal/node/node_repositories.bzl @@ -20,7 +20,7 @@ See https://docs.bazel.build/versions/master/skylark/repository_rules.html load("//internal/common:check_bazel_version.bzl", "check_bazel_version") load("//internal/common:check_version.bzl", "check_version") -load("//internal/common:os_name.bzl", "OS_NAMES", "os_name") +load("//internal/common:os_name.bzl", "OS_ARCH_NAMES", "os_name") load("//internal/npm_install:npm_install.bzl", "yarn_install") load("//third_party/github.com/bazelbuild/bazel-skylib:lib/paths.bzl", "paths") load("//toolchains/node:node_configure.bzl", node_toolchain_configure = "node_configure") @@ -452,8 +452,7 @@ if %errorlevel% neq 0 exit /b %errorlevel% "TEMPLATED_yarn_actual": yarn_node_repositories_entry, }, ) - host_os = os_name(repository_ctx) - if host_os in repository_ctx.attr.name or repository_ctx.attr.name == "nodejs": + if ("_%s_" % os_name(repository_ctx)) in repository_ctx.attr.name or repository_ctx.attr.name == "nodejs": # We have to use the relative path here otherwise bazel reports a cycle result = repository_ctx.execute([node_entry, "generate_build_file.js"]) else: @@ -610,8 +609,8 @@ def node_repositories( ) # This needs to be setup so toolchains can access nodejs for all different versions - node_repository_names = [] - for os_name in OS_NAMES: + for os_arch_name in OS_ARCH_NAMES: + os_name = "_".join(os_arch_name) node_repository_name = "nodejs_%s" % os_name _maybe( _nodejs_repo, @@ -627,9 +626,11 @@ def node_repositories( yarn_urls = yarn_urls, preserve_symlinks = preserve_symlinks, ) - node_repository_names.append(node_repository_name) - - node_toolchain_configure(node_repository_names) + native.register_toolchains("@build_bazel_rules_nodejs//toolchains/node:node_%s_toolchain" % os_arch_name[0]) + node_toolchain_configure( + name = "%s_config" % node_repository_name, + target_tool = "@%s//:node_bin" % node_repository_name, + ) _maybe( _yarn_repo, diff --git a/toolchains/node/BUILD.bazel b/toolchains/node/BUILD.bazel index 9081e22839..b7cc85f176 100644 --- a/toolchains/node/BUILD.bazel +++ b/toolchains/node/BUILD.bazel @@ -67,7 +67,7 @@ toolchain( ) toolchain( - name = "node_osx_toolchain", + name = "node_darwin_toolchain", target_compatible_with = [ "@bazel_tools//platforms:osx", "@bazel_tools//platforms:x86_64", diff --git a/toolchains/node/node_configure.bzl b/toolchains/node/node_configure.bzl index 490fed123b..e702cfee1b 100644 --- a/toolchains/node/node_configure.bzl +++ b/toolchains/node/node_configure.bzl @@ -19,13 +19,11 @@ def _impl(repository_ctx): if repository_ctx.attr.target_tool and repository_ctx.attr.target_tool_path: fail("Can only set one of target_tool or target_tool_path but both where set.") - host_os = repository_ctx.os.name.lower() - substitutions = {} - default_tool_path = repository_ctx.attr.target_tool_path or repository_ctx.which("node") or "" - substitutions["%{TOOL_ATTRS}"] = " target_tool_path = \"%s\"\n" % default_tool_path - if repository_ctx.attr.target_tool: - substitutions["%{TOOL_ATTRS}"] = " target_tool = \"%s\"\n" % repository_ctx.attr.target_tool + substitutions = {"%{TOOL_ATTRS}": " target_tool = \"%s\"\n" % repository_ctx.attr.target_tool} + else: + default_tool_path = repository_ctx.attr.target_tool_path or repository_ctx.which("node") or "" + substitutions = {"%{TOOL_ATTRS}": " target_tool_path = \"%s\"\n" % default_tool_path} repository_ctx.template( "BUILD", @@ -34,7 +32,7 @@ def _impl(repository_ctx): False, ) -_node_configure = repository_rule( +node_configure = repository_rule( implementation = _impl, attrs = { "target_tool": attr.label( @@ -48,28 +46,6 @@ _node_configure = repository_rule( ), }, ) - -def node_configure(node_repository_names = None, **kwargs): - """Creates an external repository with a node_toolchain target properly configured. - - Args: - node_repository_names: list of strings - **kwargs: all the args of the _node_configure rules - """ - - native.register_toolchains( - "@build_bazel_rules_nodejs//toolchains/node:node_linux_toolchain", - "@build_bazel_rules_nodejs//toolchains/node:node_osx_toolchain", - "@build_bazel_rules_nodejs//toolchains/node:node_windows_toolchain", - ) - - if node_repository_names: - for name in node_repository_names: - _node_configure( - name = "%s_config" % name, - target_tool = "@%s//:node_bin" % name, - ) - else: - _node_configure( - **kwargs - ) +""" +Creates an external repository with a node_toolchain //:toolchain target properly configured. +""" diff --git a/toolchains/node/node_toolchain.bzl b/toolchains/node/node_toolchain.bzl index a427fdcee4..596103b5d1 100644 --- a/toolchains/node/node_toolchain.bzl +++ b/toolchains/node/node_toolchain.bzl @@ -24,8 +24,11 @@ NodeInfo = provider( ) def _node_toolchain_impl(ctx): + if ctx.attr.target_tool and ctx.attr.target_tool_path: + fail("Can only set one of target_tool or target_tool_path but both where set.") if not ctx.attr.target_tool and not ctx.attr.target_tool_path: print("No nodejs binary was found or built, executing run for rules_nodejs targets might not work.") + toolchain_info = platform_common.ToolchainInfo( nodeinfo = NodeInfo( target_tool_path = ctx.attr.target_tool_path,