Closed Bug 1670538 Opened 4 years ago Closed 4 years ago

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

Categories

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

task

Tracking

(firefox-esr78 fixed, firefox87 fixed)

RESOLVED FIXED
87 Branch
Tracking Status
firefox-esr78 --- fixed
firefox87 --- fixed

People

(Reporter: hsivonen, Assigned: glandium)

References

Details

Attachments

(2 files)

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.

Severity: -- → S3
Priority: -- → P4

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

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: nobody → mh+mozilla
Status: NEW → ASSIGNED

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.

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

Pushed by mh@glandium.org: 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

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

Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 87 Branch

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:
Attachment #9203634 - Flags: approval-mozilla-esr78?

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.

Attachment #9203634 - Flags: approval-mozilla-esr78?

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 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:
Attachment #9208528 - Flags: approval-mozilla-esr78?

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

approved for 78.9esr

Attachment #9208528 - Flags: approval-mozilla-esr78? → approval-mozilla-esr78+

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.

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.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: