From 172caff4e487a5912b6da326bf9f8abd328001ac Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Wed, 4 Dec 2019 09:08:33 -0800 Subject: [PATCH] fix(typescript): fix for cross platform ts_devserver issue #1409 (#1413) This change allows a ts_devserver target to be build with cross platform RBE (host platform is osx or Windows & execution platform is linux) and still run on the host platform. With cross-platform RBE for OSX & Windows ctx.executable.devserver will be linux as --cpu and --host_cpu must be overridden to k8. However, we still want to be able to run the devserver on the host machine so we need to include the host devserver binary, which is ctx.executable.devserver_host, in the runfiles. For non-RBE and for RBE with a linux host, ctx.executable.devserver & ctx.executable.devserver_host will be the same binary. Issue for re-visiting this in the future is: Revisit cross-platform RBE mechanics #1415 --- internal/node/node_repositories.bzl | 6 +++ packages/typescript/docs/BUILD.bazel | 9 +++++ packages/typescript/src/devserver/BUILD.bazel | 30 ++++++++++++-- packages/typescript/src/index.from_src.bzl | 1 + .../internal/devserver/launcher_template.sh | 39 ++++++++++++++++++- .../src/internal/devserver/ts_devserver.bzl | 21 ++++++++++ 6 files changed, 102 insertions(+), 4 deletions(-) diff --git a/internal/node/node_repositories.bzl b/internal/node/node_repositories.bzl index 50cae1bec0..2b917b0567 100644 --- a/internal/node/node_repositories.bzl +++ b/internal/node/node_repositories.bzl @@ -680,8 +680,14 @@ alias(name = "yarn_node_repositories", actual = "{node_repository}//:yarn_node_r alias(name = "node_files", actual = "{node_repository}//:node_files") alias(name = "yarn_files", actual = "{node_repository}//:yarn_files") alias(name = "npm_files", actual = "{node_repository}//:npm_files") +exports_files(["index.bzl"]) """.format(node_repository = "@nodejs_%s" % os_name(repository_ctx))) + # index.bzl file for this repository + repository_ctx.file("index.bzl", content = """# Generated by node_repositories.bzl +host_platform="{host_platform}" +""".format(host_platform = os_name(repository_ctx))) + _nodejs_repo_host_os_alias = repository_rule(_nodejs_host_os_alias_impl) def node_repositories(**kwargs): diff --git a/packages/typescript/docs/BUILD.bazel b/packages/typescript/docs/BUILD.bazel index 46e243f401..6c8f421c50 100644 --- a/packages/typescript/docs/BUILD.bazel +++ b/packages/typescript/docs/BUILD.bazel @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +load("@bazel_skylib//:bzl_library.bzl", "bzl_library") load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test") load("//tools/stardoc:index.bzl", "stardoc") @@ -19,6 +20,13 @@ package(default_visibility = ["//visibility:public"]) exports_files(["install.md"]) +# Work-around since we don't have and don't wnat a bzl_library in the generated +# @nodejs//:BUILD.bazel file +bzl_library( + name = "nodejs_bzl", + srcs = ["@nodejs//:index.bzl"], +) + stardoc( name = "docs", out = "index.md", @@ -37,6 +45,7 @@ stardoc( "//internal/js_library:bzl", "//internal/node:bzl", "//internal/pkg_web:bzl", + ":nodejs_bzl", ], ) diff --git a/packages/typescript/src/devserver/BUILD.bazel b/packages/typescript/src/devserver/BUILD.bazel index 510b888d03..f738e72d28 100644 --- a/packages/typescript/src/devserver/BUILD.bazel +++ b/packages/typescript/src/devserver/BUILD.bazel @@ -12,6 +12,30 @@ # See the License for the specific language governing permissions and # limitations under the License. +filegroup( + name = "devserver_darwin_amd64", + srcs = ["devserver-darwin_x64"], + # Don't build on CI + tags = ["manual"], + visibility = ["//visibility:public"], +) + +filegroup( + name = "devserver_linux_amd64", + srcs = ["devserver-linux_x64"], + # Don't build on CI + tags = ["manual"], + visibility = ["//visibility:public"], +) + +filegroup( + name = "devserver_windows_amd64", + srcs = ["devserver-windows_x64.exe"], + # Don't build on CI + tags = ["manual"], + visibility = ["//visibility:public"], +) + config_setting( name = "darwin_x64", constraint_values = [ @@ -39,9 +63,9 @@ config_setting( filegroup( name = "devserver", srcs = select({ - ":darwin_x64": ["devserver-darwin_x64"], - ":linux_x64": ["devserver-linux_x64"], - ":windows_x64": ["devserver-windows_x64.exe"], + ":darwin_x64": [":devserver_darwin_amd64"], + ":linux_x64": [":devserver_linux_amd64"], + ":windows_x64": [":devserver_windows_amd64"], }), # Don't build on CI tags = ["manual"], diff --git a/packages/typescript/src/index.from_src.bzl b/packages/typescript/src/index.from_src.bzl index 4a5189148b..4c9214f91b 100644 --- a/packages/typescript/src/index.from_src.bzl +++ b/packages/typescript/src/index.from_src.bzl @@ -25,6 +25,7 @@ load( def ts_devserver(**kwargs): _ts_devserver( devserver = "@build_bazel_rules_typescript//devserver:devserver_bin", + devserver_host = "@build_bazel_rules_typescript//devserver:devserver_bin", **kwargs ) diff --git a/packages/typescript/src/internal/devserver/launcher_template.sh b/packages/typescript/src/internal/devserver/launcher_template.sh index 5ff470773a..8f9fc43699 100644 --- a/packages/typescript/src/internal/devserver/launcher_template.sh +++ b/packages/typescript/src/internal/devserver/launcher_template.sh @@ -42,8 +42,45 @@ else fi # --- end runfiles.bash initialization --- +# Check environment for which node path to use +unameOut="$(uname -s)" +case "${unameOut}" in + Linux*) machine=linux ;; + Darwin*) machine=darwin ;; + CYGWIN*) machine=windows ;; + MINGW*) machine=windows ;; + MSYS_NT*) machine=windows ;; + *) machine=linux + printf "\nUnrecongized uname '${unameOut}'; defaulting to use node for linux.\n" >&2 + printf "Please file an issue to https://github.com/bazelbuild/rules_nodejs/issues if \n" >&2 + printf "you would like to add your platform to the supported ts_devserver platforms.\n\n" >&2 + ;; +esac + +case "${machine}" in + # The following paths must match up with @npm_bazel_typescript//devserver binaries + darwin) readonly platform_main_manifest="npm_bazel_typescript/devserver/devserver-darwin_x64" ;; + windows) readonly platform_main_manifest="npm_bazel_typescript/devserver/devserver-windows_x64.exe" ;; + *) readonly platform_main_manifest="npm_bazel_typescript/devserver/devserver-linux_x64" ;; +esac + +readonly platform_main=$(rlocation "${platform_main_manifest}") + +if [ -f "${platform_main}" ]; then + readonly main=${platform_main} +else + # If the devserver binary is overridden then use the templated binary + readonly main=$(rlocation "TEMPLATED_main") +fi + +if [ ! -f "${main}" ]; then + printf "\n>>>> FAIL: The ts_devserver binary '${main_platform}' not found in runfiles.\n" >&2 + printf "This node toolchain was chosen based on your uname '${unameOut}'.\n" >&2 + printf "Please file an issue to https://github.com/bazelbuild/rules_nodejs/issues if \n" >&2 + printf "you would like to add your platform to the supported ts_devserver platforms. <<<<\n\n" >&2 + exit 1 +fi -readonly main=$(rlocation "TEMPLATED_main") readonly manifest=$(rlocation "TEMPLATED_manifest") readonly scripts_manifest=$(rlocation "TEMPLATED_scripts_manifest") diff --git a/packages/typescript/src/internal/devserver/ts_devserver.bzl b/packages/typescript/src/internal/devserver/ts_devserver.bzl index 7c80665b33..22126fb19e 100644 --- a/packages/typescript/src/internal/devserver/ts_devserver.bzl +++ b/packages/typescript/src/internal/devserver/ts_devserver.bzl @@ -19,6 +19,7 @@ load( "@build_bazel_rules_nodejs//internal/js_library:js_library.bzl", "write_amd_names_shim", ) +load("@nodejs//:index.bzl", "host_platform") # Avoid using non-normalized paths (workspace/../other_workspace/path) def _to_manifest_path(ctx, file): @@ -86,8 +87,14 @@ def _ts_devserver(ctx): for f in script_files ])) + # With cross-platform RBE for OSX & Windows ctx.executable.devserver will be linux as --cpu and + # --host_cpu must be overridden to k8. However, we still want to be able to run the devserver on the host + # machine so we need to include the host devserver binary, which is ctx.executable.devserver_host, in the + # runfiles. For non-RBE and for RBE with a linux host, ctx.executable.devserver & ctx.executable.devserver_host + # will be the same binary. devserver_runfiles = [ ctx.executable.devserver, + ctx.executable.devserver_host, ctx.outputs.manifest, ctx.outputs.scripts_manifest, ] @@ -138,11 +145,25 @@ ts_devserver = rule( ), "devserver": attr.label( doc = """Go based devserver executable. + + With cross-platform RBE for OSX & Windows ctx.executable.devserver will be linux as --cpu and + --host_cpu must be overridden to k8. However, we still want to be able to run the devserver on the host + machine so we need to include the host devserver binary, which is ctx.executable.devserver_host, in the + runfiles. For non-RBE and for RBE with a linux host, ctx.executable.devserver & ctx.executable.devserver_host + will be the same binary. + Defaults to precompiled go binary in @npm_bazel_typescript setup by @bazel/typescript npm package""", default = Label("//devserver"), executable = True, cfg = "host", ), + "devserver_host": attr.label( + doc = """Go based devserver executable for the host platform. + Defaults to precompiled go binary in @npm_bazel_typescript setup by @bazel/typescript npm package""", + default = Label("//devserver:devserver_%s" % host_platform), + executable = True, + cfg = "host", + ), "entry_module": attr.string( doc = """The `entry_module` should be the AMD module name of the entry module such as `"__main__/src/index".` `ts_devserver` concats the following snippet after the bundle to load the application: