Closed
Bug 575283
Opened 15 years ago
Closed 14 years ago
Cleanup mozconfig files on all platforms
Categories
(Firefox Build System :: General, enhancement, P5)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla10
People
(Reporter: atzaus, Assigned: emorley)
References
Details
Attachments
(1 file, 9 obsolete files)
27.45 KB,
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.6) Gecko/20100625 Firefox/3.6.6
Build Identifier:
Cleaning up mozconfig files
Win32
Removed --enable-libxul
Removed --enable-tests
Replaced MOZ_DEBUG_SYMBOLS=1 with --enable-debug-symbols
Linux/Linux64/MacOSX/MacOSX64
Removed --enable-libxul
Removed --enable-tests
Removed MOZ_DEBUG_SYMBOLS=1
Removed CFLAGS="-gdwarf-2" and CXXFLAGS="-gdwarf-2"
(Now using --enable-debug-symbols="-gdwarf-2")
Changed all MOZ_MAKE_FLAGS to -j4 for consistency
Added to MOZ_MAKE_FLAGS="-j4" debug builds
Linux/Linux64
Removed
# more options for 1.9 vs mozilla-central perf comparisons
# shouldn't do anything - we don't do profiled builds on linux
mk_add_options PROFILE_GEN_SCRIPT=@TOPSRCDIR@/build/profile_pageloader.pl
Reproducible: Always
Comment 7•15 years ago
|
||
Is this in support of something or is it for good housekeeping?
Updated•15 years ago
|
Severity: normal → enhancement
Priority: -- → P5
Comment 8•15 years ago
|
||
wx24 has been talking with ted last week about all these mozconfig changes.
Some of the changes like MOZ_DEBUG_SYMBOLS are to be taken and the other changes are removing the lines that are now by default like --enable-tests.
Ted is this a patch we could ask your blessings or shall one of us review it first?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Updated•15 years ago
|
Attachment #454527 -
Flags: review?(ted.mielczarek)
Comment 10•15 years ago
|
||
Comment on attachment 454527 [details] [diff] [review]
Combined Patch
Looks good to me. Thanks!
Attachment #454527 -
Flags: review?(ted.mielczarek) → review+
Comment 11•15 years ago
|
||
Comment on attachment 454527 [details] [diff] [review]
Combined Patch
>diff -r 14478e93fbc3 mozilla2/macosx/mozilla-central/nightly/mozconfig
( and release & xulrunner)
>@@ -1,17 +1,8 @@
>-. $topsrcdir/build/macosx/universal/mozconfig
>-
...
>+
>+. $topsrcdir/build/macosx/universal/mozconfig
Is moving this to the end really safe ? What if we want to override anything set in the universal/mozconfig ?
How confident do you feel about these changes ? Should we be taking them for a spin through our staging system ?
Please note that this should not land without signoff from RelEng.
Reporter | ||
Comment 12•15 years ago
|
||
Regarding . $topsrcdir/build/macosx/universal/mozconfig
If you need to override something in the mozconfig it makes sense to keep it on top. I just followed the win32 mozconfig which have this at the bottom. Should I just switch to bottom for all platforms?
Comment 13•15 years ago
|
||
The universal mozconfig and choose-make-flags serve pretty different purposes though. The former is a long file that sets up the complicated cross-compile environment for a two-pass Mac build, used by anyone wanting to do that type of build. The latter is hack for MoCo build slaves to disable parallel make on some of our machines, where it causes hangs.
I think we should preserve the status quo - universal mozconfig at the top, win32 make flags at the bottom.
Note to RelEng: We'll need to map these changes to other branches where we have copies rather than symlinks, ie mozilla-2.0, electrolysis, tracemonkey, try, 3x twigs
Reporter | ||
Comment 14•15 years ago
|
||
Attachment #454522 -
Attachment is obsolete: true
Attachment #454523 -
Attachment is obsolete: true
Attachment #454524 -
Attachment is obsolete: true
Attachment #454525 -
Attachment is obsolete: true
Attachment #454526 -
Attachment is obsolete: true
Attachment #454527 -
Attachment is obsolete: true
Depends on: 642570
642570 is not really a blocker, just related stuff. Not sure what the best way to flag that is.
There is also the recent change for debug symbols. The build configs have
---------------------------------------------
export CFLAGS="-gdwarf-2"
export CXXFLAGS="-gdwarf-2"
# For NSS symbols
export MOZ_DEBUG_SYMBOLS=1
ac_add_options --enable-debug-symbols="-gdwarf-2"
---------------------------------------------
They have no comments on why we need dwarf2, but if we don't we can remove this as debug symbols are now on by default.
Comment 16•14 years ago
|
||
We don't really need -gdwarf-2, since -g == -gdwarf-2 on the platforms we're building. I just added it to be explicit (and since at one time on OS X it was not the default). If we're going to use the defaults, then they're fine.
Comment 17•14 years ago
|
||
Do the mobile mozconfigs need changing? Or does "all platforms" mean "all Firefox desktop platforms" ?
(In reply to comment #16)
> We don't really need -gdwarf-2, since -g == -gdwarf-2 on the platforms we're
> building. I just added it to be explicit (and since at one time on OS X it was
> not the default). If we're going to use the defaults, then they're fine.
According to the manual, the default dwarf version used by gcc 4.5 is 3. Is that OK?
Comment 19•14 years ago
|
||
That's fine, I recall jimb saying there was almost no difference between 2 and 3.
Assignee | ||
Comment 20•14 years ago
|
||
atzaus, I've been working on doing something similar to that in this bug and already have a patch (applies to the now in-tree mozconfigs). Would you object to me taking this bug, or would you like to continue working on it?
Thanks :-)
Assignee | ||
Updated•14 years ago
|
Assignee: atzaus → nobody
Component: Release Engineering → Build Config
Product: mozilla.org → Core
QA Contact: release → build-config
Version: other → Trunk
Assignee | ||
Comment 21•14 years ago
|
||
After the previous comment, I spotted the date on the prior patch/dates of comments above, so have decided to upload the patch I had already created/take assignment, since it's been 14 months since any significant activity. Hope that's ok :-)
Removed the following from the now in-tree mozconfigs...
Redundant options, since they are now the default:
ac_add_options --disable-debug
ac_add_options --enable-optimize
ac_add_options --enable-debug-symbols (bug 636695)
export MOZ_DEBUG_SYMBOLS=1
ac_add_options --enable-application=browser (bug 644861)
Invalid options; no longer exist in configure:
ac_add_options --disable-install-strip
ac_add_options --disable-javaxpcom (bug 648593)
ac_add_options --enable-accessibility
ac_add_options --enable-debugger-info-modules (bug 636695)
ac_add_options --enable-libxul (bug 638429)
ac_add_options --enable-updater
ac_add_options --enable-ipc (bug 638755)
ac_add_options --enable-tests
Redundant if |ac_add_options --enable-debug-symbols="-gdwarf-2"| present:
export CFLAGS="-gdwarf-2"
export CXXFLAGS="-gdwarf-2"
Jemalloc is force disabled if |--enable-trace-malloc| is used (http://mxr.mozilla.org/mozilla-central/source/configure.in#6947), so I removed |--enable-jemalloc| if both present.
I decided against rearranging the options in each mozconfig for now, to make the diff easier to follow.
Assignee: nobody → bmo
Attachment #459555 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #565847 -
Flags: review?(khuey)
Assignee | ||
Comment 22•14 years ago
|
||
Correction to the above comment (thanks Callek for spotting), the following belong under the redundant options section, rather than invalid; but are still being removed, since they are the defaults:
--disable-install-strip
--enable-accessibility
--enable-updater
--enable-tests
I also moved this bug to Core::Build Config since now the mozconfigs are in-tree, releng doesn't have the correct milestones to mark as mozilla10 etc. I can still request dual build peer/releng team reviews if needed.
Thanks!
Comment 23•14 years ago
|
||
(In reply to Ed Morley [:edmorley] from comment #22)
> Correction to the above comment (thanks Callek for spotting), the following
> belong under the redundant options section, rather than invalid; but are
> still being removed, since they are the defaults:
> --disable-install-strip
This is not the default. We use this on Mac builds so symbols aren't stripped during packaging, so you can run Shark on nightly builds.
> --enable-accessibility
Note that this is the default, except on Mac.
Assignee | ||
Comment 24•14 years ago
|
||
Ah yes, I had missed http://mxr.mozilla.org/mozilla-central/source/configure.in#2235 and http://mxr.mozilla.org/mozilla-central/source/configure.in#4369 respectively - thanks for the clarification :-)
Comment on attachment 565847 [details] [diff] [review]
Cleanup in-tree mozconfigs
I believe there is an updated patch forthcoming.
Attachment #565847 -
Flags: review?(khuey)
Assignee | ||
Comment 26•14 years ago
|
||
(As before, but with the lines Ted mentioned removed).
Removes the following...
Redundant options, since they are now the default:
ac_add_options --disable-debug
ac_add_options --enable-optimize
ac_add_options --enable-application=browser (bug 644861)
ac_add_options --enable-tests
ac_add_options --enable-updater
ac_add_options --enable-debug-symbols (bug 636695)
export MOZ_DEBUG_SYMBOLS=1
Invalid options, that no longer exist in configure:
ac_add_options --disable-javaxpcom (bug 648593)
ac_add_options --enable-debugger-info-modules (bug 636695)
ac_add_options --enable-libxul (bug 638429)
ac_add_options --enable-ipc (bug 638755)
These are redundant if |ac_add_options --enable-debug-symbols="-gdwarf-2"| present:
export CFLAGS="-gdwarf-2"
export CXXFLAGS="-gdwarf-2"
Jemalloc is force disabled if |--enable-trace-malloc| is used (http://mxr.mozilla.org/mozilla-central/source/configure.in#6947), so I removed |--enable-jemalloc| if both are present.
Attachment #565847 -
Attachment is obsolete: true
Attachment #565908 -
Flags: review?(catlee)
Comment 27•14 years ago
|
||
Comment on attachment 565908 [details] [diff] [review]
Cleanup in-tree mozconfigs v2
Looks good to me. May want to push it to try to make sure everything still works.
Attachment #565908 -
Flags: review?(ted.mielczarek)
Attachment #565908 -
Flags: review?(catlee)
Attachment #565908 -
Flags: review+
Comment 28•14 years ago
|
||
Comment on attachment 565908 [details] [diff] [review]
Cleanup in-tree mozconfigs v2
Review of attachment 565908 [details] [diff] [review]:
-----------------------------------------------------------------
::: browser/config/mozconfig
@@ -1,5 @@
> # This file specifies the build flags for Firefox. You can use it by adding:
> # . $topsrcdir/browser/config/mozconfig
> # to the top of your mozconfig file.
> -
> -ac_add_options --enable-application=browser
I think it might be good to leave this in here just for sanity's sake, but it's not that big of a deal.
::: browser/config/mozconfigs/linux32/debug
@@ -20,1 @@
> ac_add_options --enable-debug-symbols="-gdwarf-2"
I think we could remove all of these as well, since debug symbols are on by default, and dwarf is the default. (You don't have to do that now if you don't want.)
Attachment #565908 -
Flags: review?(ted.mielczarek) → review+
Assignee | ||
Comment 29•14 years ago
|
||
I've just run the attached patch (with the changes in comment 28 added locally), past try:
https://tbpl.mozilla.org/?tree=Try&rev=63160fd92f97
However, OSX 10.6 is busted, with:
{
configure: error: --enable-application value not recognized (i386/build.mk does not exist).
}
(https://tbpl.mozilla.org/php/getParsedLog.php?id=6811759&tree=Try#error0)
Looking at http://mxr.mozilla.org/mozilla-central/source/configure.in#4441 implies that MOZ_BUILD_APP had been set to i386 (hence configure looking for i386/build.mk), so the default of "browser" isn't being applied. But yet the other platforms (including OS X 10.5) seem fine, so presumably aren't having MOZ_BUILD_APP set prior.
I've searched m-c and also buildbot-config to see if it's being set somewhere specifically for 10.6 (and why), but the closest I can find is the check here: http://mxr.mozilla.org/mozilla-central/source/build/macosx/universal/mozconfig.common#54
I guess I could just leave |ac_add_options --enable-application=browser| in for OSX 10.6, but it would be nice to either comment next to it explaining why, or else fix the underlying problem - if it turns out to actually be a bug.
Any ideas? :-)
Comment 30•14 years ago
|
||
10.6 is the only one platform doing universal builds, so it sounds like the universal build process doesn't respect the default --enable-application.
Assignee | ||
Comment 31•14 years ago
|
||
Ah thanks!
I found |mk_add_options MOZ_BUILD_PROJECTS="i386 x86_64"| in the universal mozconfig, which gets fed finally into:
http://mxr.mozilla.org/mozilla-central/source/client.mk#152
Seems like that line should be:
> BUILD_PROJECT_ARG = $(MOZ_CURRENT_PROJECT)
instead of:
> BUILD_PROJECT_ARG = MOZ_BUILD_APP=$(MOZ_CURRENT_PROJECT)
...and then the check here http://mxr.mozilla.org/mozilla-central/source/build/macosx/universal/mozconfig.common#54 changed to use BUILD_PROJECT_ARG rather than MOZ_BUILD_APP perhaps?
Does that sounds about right? If so I'll file a bug on fixing that and make it a dep of this.
Thanks again :-)
Comment 32•14 years ago
|
||
Well, client.mk historically has had support for building multiple projects, which is why that is there. (You can do MOZ_BUILD_PROJECTS="xulrunner browser", for example.)
Historically that's been useful to build xulrunner + an app built on xulrunner, which is how we used to build Fennec.
Assignee | ||
Comment 33•14 years ago
|
||
So really sounds like the universal mozconfig.common is abusing MOZ_BUILD_PROJECTS when it should be using another variable, since i386 or x86_64 aren't valid projects per se. I guess I could just add a bodge to http://mxr.mozilla.org/mozilla-central/source/configure.in#4441 which sets back to browser if i386 or x86_64 found, but it's not exactly an ideal solution.
For now I'm just going to leave |ac_add_options --enable-application=browser| in the OS X universal build mozconfigs, and add a comment explaining why, so I can get this landed.
If you'd like the universal build behaviour fixed, let me know and I'll file a followup.
Assignee | ||
Comment 34•14 years ago
|
||
As v2, but with the changes mentioned in comment 28 and comment 33.
Carrying forwards ted and catlee's r+.
https://tbpl.mozilla.org/?tree=Try&rev=3c998b954ded
Attachment #565908 -
Attachment is obsolete: true
Assignee | ||
Comment 35•14 years ago
|
||
Flags: in-testsuite-
Target Milestone: --- → mozilla10
Assignee | ||
Comment 36•14 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•