Closed Bug 865820 Opened 7 years ago Closed 7 years ago

nightly, beta, and release mozconfigs should source a common file

Categories

(Firefox Build System :: General, defect)

x86_64
Linux
defect
Not set

Tracking

(firefox23+ fixed)

RESOLVED FIXED
Tracking Status
firefox23 + fixed

People

(Reporter: bhearsum, Assigned: bhearsum)

References

Details

Attachments

(2 files, 1 obsolete file)

After bug 853071 lands, the release mozconfig is going to be almost empty, because it sources everything from the beta one. We need to have the mozconfig comparison logic take that into account before comparing.
We need this bug to support bug 853071 in release automation. 23.0b1 will be the first release with that code.
Assignee: nobody → bhearsum
Attached patch alternative solution (obsolete) — Splinter Review
Upon looking into this bug a bit further I realized that implementing it as described is going to be tough because we don't have a full gecko repository available when we do the comparison (we just download the mozconfigs individually). Rather than trying to replicate bash sourcing logic in Python, this approach lets lets the nightly, beta and release mozconfigs all inherit one thing. I think that factoring all of the things for opt builds into one file is generally a good thing, but I'm certainly biased at the moment!

If this approach seems OK, I'll do a full patch for all platforms.

I also noticed that we still have qt/rpm mozconfigs in the tree. Those builds have been dead for a long time.
Attachment #757537 - Flags: feedback?(gps)
Attachment #757537 - Flags: feedback?(gavin.sharp)
Comment on attachment 757537 [details] [diff] [review]
alternative solution

Sounds good to me!
Attachment #757537 - Flags: feedback?(gavin.sharp) → feedback+
Comment on attachment 757537 [details] [diff] [review]
alternative solution

Forgot that gps is away. Ted, any chance you can give feedback on this this week? We need to resolve this bug before the next uplift.
Attachment #757537 - Flags: feedback?(gps) → feedback?(ted)
Comment on attachment 757537 [details] [diff] [review]
alternative solution

Review of attachment 757537 [details] [diff] [review]:
-----------------------------------------------------------------

Having the extra layer of indirection for the beta mozconfig kind of sucks, but it's not terrible and it does make things more straightforward otherwise. My only complaint is that "common-opt" is not the most descriptive name, but I also don't have a better suggestion. Maybe you could just put a descriptive comment at the top of it indicating that it's sourced by nightly/beta/release builds?
Attachment #757537 - Flags: feedback?(ted) → feedback+
Here's the full patch w/ all platforms. I killed shark mozconfigs too.

I used --no-renames because I thought it was a little easier to read this way, let me know if you want the other version.

I also did some automated verification to make sure the new fully sourced mozconfigs matched the old fully sourced ones.
Attachment #757537 - Attachment is obsolete: true
Attachment #759144 - Flags: review?(ted)
Same here, just without the --enable-profiling stuff.
Attachment #759145 - Flags: review?(ted)
Attachment #759144 - Flags: review?(ted) → review+
Attachment #759145 - Flags: review?(ted) → review+
Comment on attachment 759144 [details] [diff] [review]
full patch, for mozilla-central

Landed on mozilla-central.
Attachment #759144 - Flags: checked-in+
Comment on attachment 759145 [details] [diff] [review]
full patch, for mozilla-aurora

This is looking fine on central. A few options have changed order when looking at the output of client.mk, but that's to be expected.

[Approval Request Comment]
User impact if declined: Potential for mozconfig changes required for release to be missed in the 24 cycle.
Testing completed (on m-c, etc.): Landed fine on m-c, local diffs of before/after changes to mozconfigs done.
Risk to taking this patch (and alternatives if risky): Small potential for build config to change. Very unlikely given testing already completed.
String or IDL/UUID changes made by this patch: None
Attachment #759145 - Flags: approval-mozilla-aurora?
Component: Release Engineering: Automation (Release Automation) → Build Config
Flags: checked-in+
Product: mozilla.org → Firefox
QA Contact: bhearsum
Summary: mozconfig comparison needs to source files before comparing → nightly, beta, and release mozconfigs should source a common file
Version: other → Trunk
Attachment #759145 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment on attachment 759145 [details] [diff] [review]
full patch, for mozilla-aurora

Pushed to aurora.
Attachment #759145 - Flags: checkin+
https://hg.mozilla.org/releases/mozilla-aurora/rev/58bef88a459c
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
This bug makes me happy.
Component: Build Config → General
Product: Firefox → Firefox Build System
You need to log in before you can comment on or make changes to this bug.