Last Comment Bug 575283 - Cleanup mozconfig files on all platforms
: Cleanup mozconfig files on all platforms
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Build Config (show other bugs)
: Trunk
: All All
: P5 enhancement (vote)
: mozilla10
Assigned To: Ed Morley [:emorley]
:
:
Mentors:
Depends on: 642570
Blocks:
  Show dependency treegraph
 
Reported: 2010-06-28 08:25 PDT by atzaus
Modified: 2012-01-06 05:18 PST (History)
11 users (show)
emorley: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Win32 Patch (2.48 KB, patch)
2010-06-28 08:26 PDT, atzaus
no flags Details | Diff | Splinter Review
Linux Patch (4.46 KB, patch)
2010-06-28 08:26 PDT, atzaus
no flags Details | Diff | Splinter Review
Linux64 Patch (4.39 KB, patch)
2010-06-28 08:27 PDT, atzaus
no flags Details | Diff | Splinter Review
MacOSX Patch (3.88 KB, patch)
2010-06-28 08:27 PDT, atzaus
no flags Details | Diff | Splinter Review
MacOSX64 Patch (2.82 KB, patch)
2010-06-28 08:28 PDT, atzaus
no flags Details | Diff | Splinter Review
Combined Patch (18.01 KB, patch)
2010-06-28 08:29 PDT, atzaus
mitchell.field: review+
Details | Diff | Splinter Review
Combined Patch 2.0 (17.98 KB, patch)
2010-07-22 13:28 PDT, atzaus
no flags Details | Diff | Splinter Review
Cleanup in-tree mozconfigs (25.94 KB, patch)
2011-10-09 18:06 PDT, Ed Morley [:emorley]
no flags Details | Diff | Splinter Review
Cleanup in-tree mozconfigs v2 (25.91 KB, patch)
2011-10-10 05:22 PDT, Ed Morley [:emorley]
catlee: review+
ted: review+
Details | Diff | Splinter Review
Cleanup in-tree mozconfigs v3 (27.45 KB, patch)
2011-10-14 03:36 PDT, Ed Morley [:emorley]
no flags Details | Diff | Splinter Review

Description atzaus 2010-06-28 08:25:41 PDT
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 1 atzaus 2010-06-28 08:26:21 PDT
Created attachment 454522 [details] [diff] [review]
Win32 Patch
Comment 2 atzaus 2010-06-28 08:26:58 PDT
Created attachment 454523 [details] [diff] [review]
Linux Patch
Comment 3 atzaus 2010-06-28 08:27:22 PDT
Created attachment 454524 [details] [diff] [review]
Linux64 Patch
Comment 4 atzaus 2010-06-28 08:27:57 PDT
Created attachment 454525 [details] [diff] [review]
MacOSX Patch
Comment 5 atzaus 2010-06-28 08:28:24 PDT
Created attachment 454526 [details] [diff] [review]
MacOSX64 Patch
Comment 6 atzaus 2010-06-28 08:29:12 PDT
Created attachment 454527 [details] [diff] [review]
Combined Patch
Comment 7 John Ford [:jhford] CET/CEST Berlin Time 2010-06-28 12:54:44 PDT
Is this in support of something or is it for good housekeeping?
Comment 8 Armen Zambrano [:armenzg] (EDT/UTC-4) 2010-06-29 07:33:14 PDT
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?
Comment 9 Ted Mielczarek [:ted.mielczarek] 2010-06-29 08:00:51 PDT
Yeah, I can review these.
Comment 10 Mitchell Field [:Mitch] 2010-07-02 07:50:54 PDT
Comment on attachment 454527 [details] [diff] [review]
Combined Patch

Looks good to me. Thanks!
Comment 11 Nick Thomas [:nthomas] 2010-07-02 15:23:54 PDT
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.
Comment 12 atzaus 2010-07-17 22:40:17 PDT
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 Nick Thomas [:nthomas] 2010-07-19 02:11:41 PDT
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
Comment 14 atzaus 2010-07-22 13:28:22 PDT
Created attachment 459555 [details] [diff] [review]
Combined Patch 2.0
Comment 15 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-03-23 10:44:22 PDT
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 Ted Mielczarek [:ted.mielczarek] 2011-03-23 10:47:17 PDT
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 Aki Sasaki [:aki] 2011-03-23 11:25:55 PDT
Do the mobile mozconfigs need changing? Or does "all platforms" mean "all Firefox desktop platforms" ?
Comment 18 Rafael Ávila de Espíndola (:espindola) (not reading bugmail) 2011-03-23 12:00:06 PDT
(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 Ted Mielczarek [:ted.mielczarek] 2011-03-23 16:23:24 PDT
That's fine, I recall jimb saying there was almost no difference between 2 and 3.
Comment 20 Ed Morley [:emorley] 2011-10-07 11:52:16 PDT
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 :-)
Comment 21 Ed Morley [:emorley] 2011-10-09 18:06:18 PDT
Created attachment 565847 [details] [diff] [review]
Cleanup in-tree mozconfigs

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.
Comment 22 Ed Morley [:emorley] 2011-10-09 18:56:29 PDT
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 Ted Mielczarek [:ted.mielczarek] 2011-10-10 04:43:04 PDT
(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.
Comment 24 Ed Morley [:emorley] 2011-10-10 05:01:50 PDT
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 25 Kyle Huey [:khuey] (Exited; not receiving bugmail, email if necessary) 2011-10-10 05:19:38 PDT
Comment on attachment 565847 [details] [diff] [review]
Cleanup in-tree mozconfigs

I believe there is an updated patch forthcoming.
Comment 26 Ed Morley [:emorley] 2011-10-10 05:22:40 PDT
Created attachment 565908 [details] [diff] [review]
Cleanup in-tree mozconfigs v2

(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.
Comment 27 Chris AtLee [:catlee] 2011-10-11 12:27:14 PDT
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.
Comment 28 Ted Mielczarek [:ted.mielczarek] 2011-10-12 05:36:37 PDT
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.)
Comment 29 Ed Morley [:emorley] 2011-10-12 11:48:48 PDT
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 Ted Mielczarek [:ted.mielczarek] 2011-10-12 11:53:49 PDT
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.
Comment 31 Ed Morley [:emorley] 2011-10-12 13:26:47 PDT
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 Ted Mielczarek [:ted.mielczarek] 2011-10-13 04:48:00 PDT
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.
Comment 33 Ed Morley [:emorley] 2011-10-14 03:30:10 PDT
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.
Comment 34 Ed Morley [:emorley] 2011-10-14 03:36:25 PDT
Created attachment 567041 [details] [diff] [review]
Cleanup in-tree mozconfigs v3

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
Comment 36 Ed Morley [:emorley] 2011-10-15 05:14:26 PDT
https://hg.mozilla.org/mozilla-central/rev/788f62b4e539

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