cloudflare / workerd

The JavaScript / Wasm runtime that powers Cloudflare Workers
https://blog.cloudflare.com/workerd-open-source-workers-runtime/
Apache License 2.0
6.27k stars 303 forks source link

Install buildifier via Bazel #2777

Closed npaun closed 1 month ago

npaun commented 1 month ago

Based on @danlapid's suggestion, I added some code to let format.py run the tool directly and avoid invoking bazel each time.

fhanau commented 1 month ago

What I meant with my comment re. config_setting_group was to still have them to define an alias for the binary with the right architecture, but to define the groups in a new file in the build folder and use that to fold up the groups that we have so far in rust-deps/BUILD.bazel. That feels cleaner than doing platform selection in python, but I assume that would cause issues with running the binary directly instead of through bazel, so not opposed to this approach.

npaun commented 1 month ago

@fhanau Yep, your suggestion makes sense, but I wanted direct running of the binary to speed things up where possible.

mikea commented 1 month ago

What I meant with my comment re. config_setting_group was to still have them to define an alias for the binary with the right architecture, but to define the groups in a new file in the build folder and use that to fold up the groups that we have so far in rust-deps/BUILD.bazel. That feels cleaner than doing platform selection in python, but I assume that would cause issues with running the binary directly instead of through bazel, so not opposed to this approach.

I thought about this too, and while it can be done I don't see a particular value of doing it that way. You'll have to do bazel run then, which has its own drawbacks. This change is simple enough so lg.