Skip to content

Commit

Permalink
fix(typescript): fix for cross platform ts_devserver issue #1409 (#1413)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gregmagolan authored Dec 4, 2019
1 parent dcdc98b commit 172caff
Show file tree
Hide file tree
Showing 6 changed files with 102 additions and 4 deletions.
6 changes: 6 additions & 0 deletions internal/node/node_repositories.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
9 changes: 9 additions & 0 deletions packages/typescript/docs/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,21 @@
# 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")

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",
Expand All @@ -37,6 +45,7 @@ stardoc(
"//internal/js_library:bzl",
"//internal/node:bzl",
"//internal/pkg_web:bzl",
":nodejs_bzl",
],
)

Expand Down
30 changes: 27 additions & 3 deletions packages/typescript/src/devserver/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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"],
Expand Down
1 change: 1 addition & 0 deletions packages/typescript/src/index.from_src.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down
39 changes: 38 additions & 1 deletion packages/typescript/src/internal/devserver/launcher_template.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
21 changes: 21 additions & 0 deletions packages/typescript/src/internal/devserver/ts_devserver.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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,
]
Expand Down Expand Up @@ -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:
Expand Down

0 comments on commit 172caff

Please sign in to comment.