blake3 fixups: fix unsafe overbroad enabling of fancy instructions#1396
blake3 fixups: fix unsafe overbroad enabling of fancy instructions#1396lf- wants to merge 1 commit into
Conversation
Previously to this commit, these instructions were inadvertently enabled in the portable (must run on all systems) sections of the library. This commit also pares down the compatible_with list so that `buck build shim//third-party/rust:` works better. Sadly this necessitated adding more cursed things like the existing linux-arm64 config_setting in the prelude//os. This is because `target_compatible_with` is a strict AND, `compatible_with` is an OR, and we need an OR-of-ANDs. I understand disagreement with this (I don't much like it); we may want to put them in prelude//platforms/common or something like that instead?
|
This pull request has been imported. If you are a Meta employee, you can view this in D113168775. (Because this pull request was imported automatically, there will not be any future comments.) |
| compatible_with = [ | ||
| "prelude//os:linux-x86_64", | ||
| "prelude//os:macos-x86_64", | ||
| ] |
There was a problem hiding this comment.
Fixups.toml compatible_with is not a good fit for expressing real-world Cargo cfg predicates of any complexity. Please see if you can do something like https://github.com/dtolnay/buck2-rustc-bootstrap/pull/94/files. This is what we use internally for the great big third-party/rust (6000 crates) which has 0 compatible_with fixups, only 2 target_compatible_with fixups, and supports buck2 build ... on all platforms.
The important bits are platform_compatibility_on_all_targets = true and selects.apply(REINDEER_PLATFORMS, lambda p: target_compatible_with if p in platforms else ["prelude//:none"]).
Previously to this commit, these instructions were inadvertently enabled in the portable (must run on all systems) sections of the library.
This commit also pares down the compatible_with list so that
buck build shim//third-party/rust:works better.Sadly this necessitated adding more cursed things like the existing linux-arm64 config_setting in the prelude//os. This is because
target_compatible_withis a strict AND,compatible_withis an OR, and we need an OR-of-ANDs. I understand disagreement with this (I don't much like it); we may want to put them in prelude//platforms/common or something like that instead?