Skip to content

blake3 fixups: fix unsafe overbroad enabling of fancy instructions#1396

Open
lf- wants to merge 1 commit into
facebook:mainfrom
lf-:jade/push-wkqvxwstyyxk
Open

blake3 fixups: fix unsafe overbroad enabling of fancy instructions#1396
lf- wants to merge 1 commit into
facebook:mainfrom
lf-:jade/push-wkqvxwstyyxk

Conversation

@lf-

@lf- lf- commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

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?

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?
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 22, 2026
@meta-codesync

meta-codesync Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

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.)

Comment on lines +31 to +34
compatible_with = [
"prelude//os:linux-x86_64",
"prelude//os:macos-x86_64",
]

@dtolnay dtolnay Jul 22, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"]).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants