build system clobbers user RUSTFLAGS

RESOLVED FIXED in Firefox 55

Status

Firefox Build System
General
RESOLVED FIXED
2 years ago
5 months ago

People

(Reporter: bholley, Assigned: marco)

Tracking

(Blocks: 1 bug)

unspecified
mozilla55
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

2 years ago
I noticed this was happening, and it matches my reading of the patch in bug 1300835.

Is this intended behavior? What do we do for CFLAGS?
(Reporter)

Updated

2 years ago
Flags: needinfo?(ted)
We generally try to persist CFLAGS/CXXFLAGS that you set in your mozconfig. We could do the same for RUSTFLAGS. You'll still hit some potential weirdness but it's no different from the CFLAGS case--if you set `CFLAGS=-O2` or `CFLAGS=-g3` or something like that the effect is not really predictable (because we set optimization flags and debug info flags elsewhere).
Flags: needinfo?(ted)
(Reporter)

Comment 2

2 years ago
In general I think the Rust compiler tries to prioritize options at the end of the string over options at the beginning, so if we followed those semantics we should have mostly-predictable behavior. This would be nice so that people can manually pass the correct codegen-units, at least until [1] lands.

[1] https://github.com/rust-lang/rust/issues/38276
(Assignee)

Comment 3

a year ago
This is needed to enable code coverage for Rust.
Blocks: 1335518
To implement this you'd just need two pieces:
1) A bit in moz.configure to take RUSTFLAGS from the environment as an option, something like:
https://dxr.mozilla.org/mozilla-central/rev/8a3aa1701537ea6b8334f432cd030d260d492fa3/build/moz.configure/toolchain.configure#894
+ a set_config('RUSTFLAGS', ...)
2) Pass $(RUSTFLAGS) in the rustflags_override variable which is what we use to pass RUSTFLAGS to cargo invocations currently:
https://dxr.mozilla.org/mozilla-central/rev/8a3aa1701537ea6b8334f432cd030d260d492fa3/config/rules.mk#946
(Assignee)

Comment 5

a year ago
Created attachment 8874645 [details] [diff] [review]
Patch
Assignee: nobody → mcastelluccio
Status: NEW → ASSIGNED
Attachment #8874645 - Flags: review?(ted)
Attachment #8874645 - Flags: review?(ted) → review+

Comment 6

a year ago
Pushed by mcastelluccio@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/06967108134b
Allow setting additional rustflags via mozconfig. r=ted
https://hg.mozilla.org/mozilla-central/rev/06967108134b
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55

Updated

5 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.