[Bug]: js_run_binary defaults to opt runfiles for all compilation modes.

[paullewis]

What happened?

It looks like the runfiles defaults to opt for js_run_binary, irrespective of the Bazel compilation mode, and I'm not sure I understand why. In my particular case I'm using esbuild to generate files and toggling splitting and minifying based on the compilation mode (on for opt and off for fastbuild), which in turn means that the hashes in the file names differ based on compilation mode.

I have a script that goes looking in the runfiles for the generated JS, and under fastbuild it goes looking for those that would've been generated under opt.

Version

Development (host) and target OS/architectures: Mac

Output of bazel --version: bazel 5.3.2

Version of the Aspect rules, or other relevant rules from your
WORKSPACE or MODULE.bazel file: 1.6.9

Language(s) and/or frameworks involved: JavaScript & Node

How to reproduce

Given a js_binary like this:

js_binary(
    name = "test",
    entry_point = ":test.js",
)

Running bazel run //:test and logging process.env.RUNFILES I see a fastbuild path:

RUNFILES: '/path/execroot/__main__/bazel-out/darwin_arm64-fastbuild/bin/test.sh.runfiles'

However, if I now run that same js_binary as the tool for a js_run_binary like this:

js_run_binary(
    name = "test-bin",
    outs = ["foo"],
    silent_on_success = False,
    tool = "//:test",
)

The process.env.RUNFILES is now set to an opt path:

RUNFILES: '/path/execroot/__main__/bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin/src/test.sh.runfiles'

[gregmagolan]

Thanks for the bug report @paullewis. I don't think we'll have any time to look at it until the end of the week or early next week as its BazelCon in NYC this week and that is keeping us busier than usual this week.

@alexeagle is doing a short talk about our upcoming products on Wednesday that may interest you: https://opensourcelive.withgoogle.com/events/bazelcon2022?talk=day1-talk6

[gregjacobs]

I think I've run into the same issue, where every target in my repo is building twice if they are used by a js_run_binary target: once for the default compilation mode, and then a second time where Bazel reports the same target building "[for tool]"

This is resulting in very long build times for our repo, since we actually pull a lot of packages into js_run_binary targets in order to do code generation, documentation from generated typings, etc.

I assume a workaround may be to set the default --compilation_mode to 'opt', but would definitely appreciate this one being fixed

Update on the above: never mind, that doesn't work. Check @gregmagolan's reply below

[gregmagolan]

Bazel is transitioning the js_binary tool to the execution platform. This is correct and necessary if the execution platform is different than the target platform. Unfortunately, even if the target & exec platforms are the same, Bazel still uses as sparate output tree.

Under the hood js_run_binary uses run_binary. The wiring is here:

rules_js/js/private/js_run_binary.bzl

Line 336 in 8dc6d52

tool = tool,

and the tool in run_binary is transitioned to the execution platform (the bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin output tree) because run_binary brings the tool in with cfg = "exec": https://github.com/aspect-build/bazel-lib/blob/803d5ecda8f5df9a0a1996cd8fde897ed24cbb83/lib/private/run_binary.bzl#L91

You could work around this by patching bazel-lib run_binary to use cfg = "target". An alternate pattern, which I've used in a few places around rules_js is to create two different tool attributes in run_binary, one with cfg = "exec" and one with cfg = "target" and then choose one or the other depending on another attribute.

An example of that approach is in js_run_devserver,

rules_js/js/private/js_run_devserver.bzl

Line 8 in 8dc6d52

"tool_exec_cfg": attr.label(

[gregjacobs]

Thanks for this @gregmagolan, this is some very helpful insight.

I guess the part that I'm confused about then is that I still get the second build "[for tool]" even though the execution platform is the same as the target platform. (Or at least, I'm not setting any --platforms, so I would assume so?)

In this case, the js_binary targets should already be built for the execution platform, no?

[gregmagolan]

If you bazel build //path/to/js_binary:target you'll build that binary for the target platform. However if you bazel build //path/to/js_run_binary:target then the tool of that js_run_binary will be transitioned to the execution platform (the bazel-out/darwin_arm64-opt-exec-2B5CBBC6/bin output tree) and built for that. If that happens to be the same //path/to/js_binary:target that was built for the target platform then it will be built again into the opt-exec output tree.

I'd have to look more closely at your build graph & profile to see if the transitions are related to the long build times you are experiencing. We do paid offer consulting and Bazel support if you're interesting in more hands on help, https://www.aspect.dev/hello.

[gregjacobs]

Thanks for this @gregmagolan, and good to know you guys offer support!

I ended up patching bazel-lib's run_binary rule for now to change cfg="exec" to cfg="target" on the tool attribute, and this solves the problem, so thanks for that! However it doesn't feel quite right, especially if, for some reason, we need another tool that does in fact need to compile for the execution platform.

I didn't quite understand the workaround with using tool_exec_cfg and tool_target_cfg attributes. Is this something that needs to be built directly into js_run_binary? Or do I need to implement my own js_run_binary rule that implements these? I'm still having a little trouble making sense of how executable attributes work in Bazel so probably missing a few details on this