Adapt Firefox build system to use an allow-list in RUSTC_BOOTSTRAP
Categories
(Firefox Build System :: General, task, P4)
Tracking
(firefox-esr78 fixed, firefox87 fixed)
People
(Reporter: hsivonen, Assigned: glandium)
References
Details
Attachments
(2 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-esr78+
|
Details | Review |
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
.
Updated•4 years ago
|
Comment 1•4 years ago
|
||
This landed in 1.50, which was promoted to stable last week. I'm not sure what toolchain Firefox is using, though.
Assignee | ||
Comment 2•4 years ago
|
||
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).
Updated•4 years ago
|
Comment 3•4 years ago
|
||
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•4 years ago
|
||
(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.
Assignee | ||
Comment 6•4 years ago
|
||
(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•4 years ago
|
||
bugherder |
Assignee | ||
Comment 9•4 years ago
|
||
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:
Assignee | ||
Comment 10•4 years ago
|
||
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.
Assignee | ||
Comment 11•4 years ago
|
||
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).
Assignee | ||
Comment 12•4 years ago
|
||
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•4 years ago
|
||
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•4 years ago
|
||
bugherder uplift |
Comment 15•4 years ago
|
||
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•4 years ago
|
||
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.
Description
•