Closed
Bug 865820
Opened 12 years ago
Closed 11 years ago
nightly, beta, and release mozconfigs should source a common file
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox23+ fixed)
RESOLVED
FIXED
People
(Reporter: bhearsum, Assigned: bhearsum)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
patch
|
ted
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
ted
:
review+
lsblakk
:
approval-mozilla-aurora+
bhearsum
:
checkin+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•11 years ago
|
||
We need this bug to support bug 853071 in release automation. 23.0b1 will be the first release with that code.
tracking-firefox22:
--- → ?
tracking-firefox23:
--- → ?
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → bhearsum
Assignee | ||
Comment 2•11 years ago
|
||
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)
Updated•11 years ago
|
tracking-firefox22:
? → ---
Comment 3•11 years ago
|
||
Comment on attachment 757537 [details] [diff] [review]
alternative solution
Sounds good to me!
Attachment #757537 -
Flags: feedback?(gavin.sharp) → feedback+
Assignee | ||
Comment 4•11 years ago
|
||
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 5•11 years ago
|
||
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+
Assignee | ||
Comment 6•11 years ago
|
||
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)
Assignee | ||
Comment 7•11 years ago
|
||
Same here, just without the --enable-profiling stuff.
Attachment #759145 -
Flags: review?(ted)
Assignee | ||
Comment 8•11 years ago
|
||
Oh, and try runs w/ the new nightly configs are here:
https://tbpl.mozilla.org/?tree=Try&rev=0074876b721d
https://tbpl.mozilla.org/?tree=Try&rev=c99cb442c826
Updated•11 years ago
|
Attachment #759144 -
Flags: review?(ted) → review+
Updated•11 years ago
|
Attachment #759145 -
Flags: review?(ted) → review+
Assignee | ||
Comment 9•11 years ago
|
||
Comment on attachment 759144 [details] [diff] [review]
full patch, for mozilla-central
Landed on mozilla-central.
Attachment #759144 -
Flags: checked-in+
Assignee | ||
Comment 10•11 years ago
|
||
Assignee | ||
Comment 11•11 years ago
|
||
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?
Assignee | ||
Updated•11 years ago
|
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
Updated•11 years ago
|
Attachment #759145 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 12•11 years ago
|
||
Comment on attachment 759145 [details] [diff] [review]
full patch, for mozilla-aurora
Pushed to aurora.
Attachment #759145 -
Flags: checkin+
Assignee | ||
Comment 13•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
status-firefox23:
--- → fixed
Comment 14•11 years ago
|
||
This bug makes me happy.
Updated•6 years ago
|
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.
Description
•