1670538 - Adapt Firefox build system to use an allow-list in RUSTC_BOOTSTRAP

Status: RESOLVED FIXED

(Firefox Build System :: General, task, P4)

Description

[Henri Sivonen (:hsivonen)]

https://github.com/rust-lang/rust/pull/77802 changes RUSTC_BOOTSTRAP to take a comma-separated list of crates to be compiled with unstable features allowed. Once that change reaches our toolchain, we should use the feature to allow-list encoding_rs,packed_simd.

Comment 1

[Joshua Nelson]

This landed in 1.50, which was promoted to stable last week. I'm not sure what toolchain Firefox is using, though.

Comment 2

[Mike Hommey [:glandium]]

Attachment: Bug 1670538 - Use an allow-list in RUSTC_BOOTSTRAP for rustc >= 1.50.0.

While we could change qcms, encoding_rs and packed_simd to not emit
RUSTC_BOOTSTRAP on newer versions of rust, like we do for gkrust-shared,
it's not worth the effort (especially for those that are vendored).

Comment 3

[Joshua Nelson]

While we could change qcms, encoding_rs and packed_simd to not emit
RUSTC_BOOTSTRAP on newer versions of rust, like we do for gkrust-shared,
it's not worth the effort (especially for those that are vendored).

Just so you know, cargo is eventually planning to give loud warnings for crates that do this: https://github.com/rust-lang/cargo/issues/7088. They'll still build, but it will warn even if built as a dependency.

Comment 4

[Joshua Nelson]

(In reply to Joshua Nelson from comment #3)

While we could change qcms, encoding_rs and packed_simd to not emit
RUSTC_BOOTSTRAP on newer versions of rust, like we do for gkrust-shared,
it's not worth the effort (especially for those that are vendored).

Just so you know, cargo is eventually planning to give loud warnings for crates that do this: https://github.com/rust-lang/cargo/issues/7088. They'll still build, but it will warn even if built as a dependency.

(not sure how to edit) Specifically when I say "still build", I mean if you set RUSTC_BOOTSTRAP=packed_simd at the top-level when invoking cargo. Crates will be unable to add RUSTC_BOOTSTRAP themselves.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b182a3466285
Use an allow-list in RUSTC_BOOTSTRAP for rustc >= 1.50.0. r=firefox-build-system-reviewers,andi,sheehan,mhentges

Comment 6

[Mike Hommey [:glandium]]

(In reply to Joshua Nelson from comment #4)

(In reply to Joshua Nelson from comment #3)

While we could change qcms, encoding_rs and packed_simd to not emit
RUSTC_BOOTSTRAP on newer versions of rust, like we do for gkrust-shared,
it's not worth the effort (especially for those that are vendored).

Just so you know, cargo is eventually planning to give loud warnings for crates that do this: https://github.com/rust-lang/cargo/issues/7088. They'll still build, but it will warn even if built as a dependency.

(not sure how to edit) Specifically when I say "still build", I mean if you set RUSTC_BOOTSTRAP=packed_simd at the top-level when invoking cargo. Crates will be unable to add RUSTC_BOOTSTRAP themselves.

As long as cargo doesn't actively fail in that case, that's fine.

Comment 7

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/b182a3466285

Comment 9

[Mike Hommey [:glandium]]

Comment on attachment 9203634 [details]
Bug 1670538 - Use an allow-list in RUSTC_BOOTSTRAP for rustc >= 1.50.0.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Build failure with newer versions of the rust compiler.
• User impact if declined:
• Fix Landed on Version: 87
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very limited and well understood build-system only change.
• String or UUID changes made by this patch:

Comment 10

[Mike Hommey [:glandium]]

Comment on attachment 9203634 [details]
Bug 1670538 - Use an allow-list in RUSTC_BOOTSTRAP for rustc >= 1.50.0.

Actually, esr78 will need a slightly different patch.

Comment 11

[Mike Hommey [:glandium]]

Attachment: Bug 1670538 - Use an allow-list in RUSTC_BOOTSTRAP for rustc >= 1.50.0.

While we could change qcms, encoding_rs and packed_simd to not emit
RUSTC_BOOTSTRAP on newer versions of rust, like we do for gkrust-shared,
it's not worth the effort (especially for those that are vendored).

Comment 12

[Mike Hommey [:glandium]]

Comment on attachment 9208528 [details]
Bug 1670538 - Use an allow-list in RUSTC_BOOTSTRAP for rustc >= 1.50.0.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Build failure with newer versions of the rust compiler.
• User impact if declined:
• Fix Landed on Version: 87
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very limited and well understood build-system only change. The patch itself is not the same as the one that landed in 87 because the minimum supported rustc version is lower on esr78 and needed adaptation, and qcms only landed in 83. The interdiff is straightforward: https://phabricator.services.mozilla.com/D108098?vs=411807&id=411810#toc
• String or UUID changes made by this patch:

Comment 13

[Julien Cristau [:jcristau]]

Comment on attachment 9208528 [details]
Bug 1670538 - Use an allow-list in RUSTC_BOOTSTRAP for rustc >= 1.50.0.

approved for 78.9esr

Comment 14

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-esr78/rev/56a69a9c36f5

Comment 15

[mss]

i am the person that requested this fix to be applied to 78esr and i am still getting RUSTC_BOOTSTRAP with rust 1.52 beta even on 78.10.0esr so i cannot enable rust-simd.

Comment 16

[mss]

after a clean rebuild of llvm12 stable and a newer commit of rust 1.52 beta the issue seems to be gone when building 78.10.1esr, disregard my comment.