fix: always use the target platform for `tool` in `js_run_devserver` (#2853)
I realized that the logic here was more complicated than it needed to
be. With `js_run_binary`, we're running a JavaScript tool that often
operates on JavaScript sources, so it makes sense that we need to think
which inputs are built for the exec platform and which ones are built
for the target platform.
But `js_run_devserver` is different, because the `tool` we are working
with is not intended to run as part of a build action. All we are doing
is packaging up the tool in a launcher script so that it can be executed
with `bazel run`. Therefore, `tool` should always be built for the
target platform and never for the exec platform.
This simplifies things, because it makes it so that
`use_execroot_entry_point` is just a relatively small runtime behavior
change rather than something that totally changes what platform we are
building for.
Since `bazel run` does not use a sandbox, all the inputs end up present
both in the runfiles directory and directly in the execroot, and
basically all that `use_execroot_entry_point` does now is determine
which of those two places we use as the entry point.
---
### Changes are visible to end-users: yes, but I doubt that anyone will
notice
- Searched for relevant documentation and updated as needed: yes
- Breaking change (forces users to change their own code or config): no
- Suggested release notes appear below: yes
We now always build all sources for the target platform in
`js_run_devserver`, regardless of `use_execroot_entry_point`.
### Test plan
- Covered by existing test cases
- New test cases added
diff --git a/js/private/js_run_devserver.bzl b/js/private/js_run_devserver.bzl
index 4214236..ec2cd26 100644
--- a/js/private/js_run_devserver.bzl
+++ b/js/private/js_run_devserver.bzl
@@ -4,11 +4,7 @@
load(":js_helpers.bzl", _gather_files_from_js_infos = "gather_files_from_js_infos")
_attrs = js_binary_lib.attrs | {
- "tool_exec_cfg": attr.label(
- executable = True,
- cfg = "exec",
- ),
- "tool_target_cfg": attr.label(
+ "tool": attr.label(
executable = True,
cfg = "target",
),
@@ -47,9 +43,7 @@
fixed_args = [config_file.short_path, entries_json_file.short_path],
)
- use_tool = ctx.attr.tool_target_cfg or ctx.attr.tool_exec_cfg
- if use_tool and (not ctx.attr.tool_exec_cfg or not ctx.attr.tool_target_cfg):
- fail("Internal error")
+ use_tool = ctx.attr.tool != None
if not use_tool and not ctx.attr.command:
fail("Either tool or command must be specified")
@@ -85,16 +79,13 @@
runfiles_merge_targets = ctx.attr.data[:]
if use_tool:
+ config["tool"] = ctx.executable.tool.short_path
+ runfiles_merge_targets.append(ctx.attr.tool)
if ctx.attr.use_execroot_entry_point:
- config["tool"] = ctx.executable.tool_target_cfg.short_path
config["use_execroot_entry_point"] = "1"
config["bazel_bindir"] = ctx.bin_dir.path
if ctx.attr.allow_execroot_entry_point_with_no_copy_data_to_bin:
config["allow_execroot_entry_point_with_no_copy_data_to_bin"] = "1"
- runfiles_merge_targets.append(ctx.attr.tool_target_cfg)
- else:
- config["tool"] = ctx.executable.tool_exec_cfg.short_path
- runfiles_merge_targets.append(ctx.attr.tool_exec_cfg)
if ctx.attr.command:
config["command"] = ctx.attr.command
if ctx.attr.grant_sandbox_write_permissions:
@@ -283,8 +274,7 @@
"ibazel_notify_changes",
"supports_incremental_build_protocol",
],
- tool_exec_cfg = tool,
- tool_target_cfg = tool,
+ tool = tool,
command = command,
grant_sandbox_write_permissions = grant_sandbox_write_permissions,
use_execroot_entry_point = use_execroot_entry_point,