Skip to content

Commit 172caff

Browse files
authored
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
1 parent dcdc98b commit 172caff

6 files changed

Lines changed: 102 additions & 4 deletions

File tree

internal/node/node_repositories.bzl

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -680,8 +680,14 @@ alias(name = "yarn_node_repositories", actual = "{node_repository}//:yarn_node_r
680680
alias(name = "node_files", actual = "{node_repository}//:node_files")
681681
alias(name = "yarn_files", actual = "{node_repository}//:yarn_files")
682682
alias(name = "npm_files", actual = "{node_repository}//:npm_files")
683+
exports_files(["index.bzl"])
683684
""".format(node_repository = "@nodejs_%s" % os_name(repository_ctx)))
684685

686+
# index.bzl file for this repository
687+
repository_ctx.file("index.bzl", content = """# Generated by node_repositories.bzl
688+
host_platform="{host_platform}"
689+
""".format(host_platform = os_name(repository_ctx)))
690+
685691
_nodejs_repo_host_os_alias = repository_rule(_nodejs_host_os_alias_impl)
686692

687693
def node_repositories(**kwargs):

packages/typescript/docs/BUILD.bazel

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,21 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
1516
load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test")
1617
load("//tools/stardoc:index.bzl", "stardoc")
1718

1819
package(default_visibility = ["//visibility:public"])
1920

2021
exports_files(["install.md"])
2122

23+
# Work-around since we don't have and don't wnat a bzl_library in the generated
24+
# @nodejs//:BUILD.bazel file
25+
bzl_library(
26+
name = "nodejs_bzl",
27+
srcs = ["@nodejs//:index.bzl"],
28+
)
29+
2230
stardoc(
2331
name = "docs",
2432
out = "index.md",
@@ -37,6 +45,7 @@ stardoc(
3745
"//internal/js_library:bzl",
3846
"//internal/node:bzl",
3947
"//internal/pkg_web:bzl",
48+
":nodejs_bzl",
4049
],
4150
)
4251

packages/typescript/src/devserver/BUILD.bazel

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,30 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
filegroup(
16+
name = "devserver_darwin_amd64",
17+
srcs = ["devserver-darwin_x64"],
18+
# Don't build on CI
19+
tags = ["manual"],
20+
visibility = ["//visibility:public"],
21+
)
22+
23+
filegroup(
24+
name = "devserver_linux_amd64",
25+
srcs = ["devserver-linux_x64"],
26+
# Don't build on CI
27+
tags = ["manual"],
28+
visibility = ["//visibility:public"],
29+
)
30+
31+
filegroup(
32+
name = "devserver_windows_amd64",
33+
srcs = ["devserver-windows_x64.exe"],
34+
# Don't build on CI
35+
tags = ["manual"],
36+
visibility = ["//visibility:public"],
37+
)
38+
1539
config_setting(
1640
name = "darwin_x64",
1741
constraint_values = [
@@ -39,9 +63,9 @@ config_setting(
3963
filegroup(
4064
name = "devserver",
4165
srcs = select({
42-
":darwin_x64": ["devserver-darwin_x64"],
43-
":linux_x64": ["devserver-linux_x64"],
44-
":windows_x64": ["devserver-windows_x64.exe"],
66+
":darwin_x64": [":devserver_darwin_amd64"],
67+
":linux_x64": [":devserver_linux_amd64"],
68+
":windows_x64": [":devserver_windows_amd64"],
4569
}),
4670
# Don't build on CI
4771
tags = ["manual"],

packages/typescript/src/index.from_src.bzl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ load(
2525
def ts_devserver(**kwargs):
2626
_ts_devserver(
2727
devserver = "@build_bazel_rules_typescript//devserver:devserver_bin",
28+
devserver_host = "@build_bazel_rules_typescript//devserver:devserver_bin",
2829
**kwargs
2930
)
3031

packages/typescript/src/internal/devserver/launcher_template.sh

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,45 @@ else
4242
fi
4343
# --- end runfiles.bash initialization ---
4444

45+
# Check environment for which node path to use
46+
unameOut="$(uname -s)"
47+
case "${unameOut}" in
48+
Linux*) machine=linux ;;
49+
Darwin*) machine=darwin ;;
50+
CYGWIN*) machine=windows ;;
51+
MINGW*) machine=windows ;;
52+
MSYS_NT*) machine=windows ;;
53+
*) machine=linux
54+
printf "\nUnrecongized uname '${unameOut}'; defaulting to use node for linux.\n" >&2
55+
printf "Please file an issue to https://github.com/bazelbuild/rules_nodejs/issues if \n" >&2
56+
printf "you would like to add your platform to the supported ts_devserver platforms.\n\n" >&2
57+
;;
58+
esac
59+
60+
case "${machine}" in
61+
# The following paths must match up with @npm_bazel_typescript//devserver binaries
62+
darwin) readonly platform_main_manifest="npm_bazel_typescript/devserver/devserver-darwin_x64" ;;
63+
windows) readonly platform_main_manifest="npm_bazel_typescript/devserver/devserver-windows_x64.exe" ;;
64+
*) readonly platform_main_manifest="npm_bazel_typescript/devserver/devserver-linux_x64" ;;
65+
esac
66+
67+
readonly platform_main=$(rlocation "${platform_main_manifest}")
68+
69+
if [ -f "${platform_main}" ]; then
70+
readonly main=${platform_main}
71+
else
72+
# If the devserver binary is overridden then use the templated binary
73+
readonly main=$(rlocation "TEMPLATED_main")
74+
fi
75+
76+
if [ ! -f "${main}" ]; then
77+
printf "\n>>>> FAIL: The ts_devserver binary '${main_platform}' not found in runfiles.\n" >&2
78+
printf "This node toolchain was chosen based on your uname '${unameOut}'.\n" >&2
79+
printf "Please file an issue to https://github.com/bazelbuild/rules_nodejs/issues if \n" >&2
80+
printf "you would like to add your platform to the supported ts_devserver platforms. <<<<\n\n" >&2
81+
exit 1
82+
fi
4583

46-
readonly main=$(rlocation "TEMPLATED_main")
4784
readonly manifest=$(rlocation "TEMPLATED_manifest")
4885
readonly scripts_manifest=$(rlocation "TEMPLATED_scripts_manifest")
4986

packages/typescript/src/internal/devserver/ts_devserver.bzl

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ load(
1919
"@build_bazel_rules_nodejs//internal/js_library:js_library.bzl",
2020
"write_amd_names_shim",
2121
)
22+
load("@nodejs//:index.bzl", "host_platform")
2223

2324
# Avoid using non-normalized paths (workspace/../other_workspace/path)
2425
def _to_manifest_path(ctx, file):
@@ -86,8 +87,14 @@ def _ts_devserver(ctx):
8687
for f in script_files
8788
]))
8889

90+
# With cross-platform RBE for OSX & Windows ctx.executable.devserver will be linux as --cpu and
91+
# --host_cpu must be overridden to k8. However, we still want to be able to run the devserver on the host
92+
# machine so we need to include the host devserver binary, which is ctx.executable.devserver_host, in the
93+
# runfiles. For non-RBE and for RBE with a linux host, ctx.executable.devserver & ctx.executable.devserver_host
94+
# will be the same binary.
8995
devserver_runfiles = [
9096
ctx.executable.devserver,
97+
ctx.executable.devserver_host,
9198
ctx.outputs.manifest,
9299
ctx.outputs.scripts_manifest,
93100
]
@@ -138,11 +145,25 @@ ts_devserver = rule(
138145
),
139146
"devserver": attr.label(
140147
doc = """Go based devserver executable.
148+
149+
With cross-platform RBE for OSX & Windows ctx.executable.devserver will be linux as --cpu and
150+
--host_cpu must be overridden to k8. However, we still want to be able to run the devserver on the host
151+
machine so we need to include the host devserver binary, which is ctx.executable.devserver_host, in the
152+
runfiles. For non-RBE and for RBE with a linux host, ctx.executable.devserver & ctx.executable.devserver_host
153+
will be the same binary.
154+
141155
Defaults to precompiled go binary in @npm_bazel_typescript setup by @bazel/typescript npm package""",
142156
default = Label("//devserver"),
143157
executable = True,
144158
cfg = "host",
145159
),
160+
"devserver_host": attr.label(
161+
doc = """Go based devserver executable for the host platform.
162+
Defaults to precompiled go binary in @npm_bazel_typescript setup by @bazel/typescript npm package""",
163+
default = Label("//devserver:devserver_%s" % host_platform),
164+
executable = True,
165+
cfg = "host",
166+
),
146167
"entry_module": attr.string(
147168
doc = """The `entry_module` should be the AMD module name of the entry module such as `"__main__/src/index".`
148169
`ts_devserver` concats the following snippet after the bundle to load the application:

0 commit comments

Comments
 (0)