Add support for browser and platform allowlists#1
Conversation
4e71e1f to
c7d2b90
Compare
jjudd
left a comment
There was a problem hiding this comment.
Thanks for doing this. Had a few questions.
tools/release/BUILD.bazel
Outdated
| [ | ||
| platform( | ||
| name = "{}_{}".format(os, cpu), | ||
| name = "{}_{}".format("linux", cpu), |
There was a problem hiding this comment.
I think this hard codes things to linux, right? I doubt we can submit this upstream.
There was a problem hiding this comment.
That's right. I don't intend on submitting this commit upstream. I just didn't think it'd be worth it to wrestle with MacOS cross-compilation when I could just rip it out.
There was a problem hiding this comment.
What is being cross-compiled? The Rust code?
If it's something native that needs to be cross compiled, then I recommend dropping https://github.com/cerisier/toolchains_llvm_bootstrapped in and calling it good.
I think it will be less work to get this working and contribute it back upstream than it will be to maintain a forever fork of this ruleset.
| name = "cli", | ||
| src = select( | ||
| { | ||
| "//tools/platforms:macos_x86_64": ":artifacts/cli-x86_64-apple-darwin", |
There was a problem hiding this comment.
Similar thought here: how do we intend to upstream this if we're removing support for non-linux OS
There was a problem hiding this comment.
We don't 🙃
I should've clarified that I only intend to upstream the second commit.
Cross-compilation for MacOS wasn't working with `toolchains_llvm`, so I switched us to `toolchains_llvm_bootstrapped`. This toolchain also has the advantage of being more hermetic and downloading less. In order to do this, I had to upgrade `rules_rust` to v0.69.0, as older versions aren't compatible with the linker `toolchains_llvm_boostrapped` supplies. Finally, I added `/bazel/bazel_downloader.cfg` to `.gitignore` so we can configure the Bazel downloader to use our corporate cache, as I'm unable to download the MacOS toolchain doesn't without it.
c7d2b90 to
bd03ccd
Compare
For some reason, Bazel is downloading
rules_playwrightbrowsers, causing many warnings to appear that they're missing integrity values. It's also probably slowing down our builds. This PR addsallowed_browsersandallowed_platformsoptions torules_playwright's module extension that allow us to configure which browser names and platforms it declares browsers for.It also includes the changes from this one, which are necessary for us to use newer versions of Playwright:
mrmeku#36