407794 - Nightlies should not override the module-specific build settings.

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P2)

Description

[Robert Sayre]

This is bad. At least, it's bad that we do this in nightlies and release builds. It's unlikely that a global setting is good. For example, sqlite should be built with very high optimization, not "-O2".

Nightlies should not override the module-specific build settings.

Comment 1

[Robert Sayre]

sqlite would like to be built with:

-O3 on gcc
-xO5 on sun
-Ox on msvc

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/db/sqlite3/src/Makefile.in&rev=1.30#59

Comment 2

[(not currently active) Ted Mielczarek]

I suggested in another bug that we switch to using
--enable-optimize
--enable-debugger-info-modules

This won't quite work on Linux, afaik, since we need to explicitly set -gstabs+, but we might be able to work around that in other ways.

Comment 3

[(not currently active) Ted Mielczarek]

For reference:
http://lxr.mozilla.org/mozilla/source/config/config.mk#538
http://lxr.mozilla.org/mozilla/source/tools/tinderbox-configs/firefox/macosx/mozconfig#15

Comment 4

[(not currently active) Ted Mielczarek]

Linux is actually goofier, fwiw:
http://lxr.mozilla.org/mozilla/source/tools/tinderbox-configs/firefox/linux/mozconfig#14
Only Win32 doesn't screw with this:
http://lxr.mozilla.org/mozilla/source/tools/tinderbox-configs/firefox/win32/mozconfig

Comment 5

[Benjamin Smedberg]

I believe the correct solution is to add

export CFLAGS=-gstabs+
export CXXFLAGS=-gstabs+

to the mozconfig, and remove the --enable-optimize flag(s)

Neither --enable-debugger-info-modules nor MOZ_DEBUG_SYMBOLS affects NSPR on non-windows correctly

Comment 6

[Robert Sayre]

Attachment: use the right settings for each tier1 by default, so modules can override

Comment 7

[(not currently active) Ted Mielczarek]

Comment on attachment 292478 [details] [diff] [review]
use the right settings for each tier1 by default, so modules can override

Could you add comments to the mozconfigs so that nobody undoes this later?  Just something like
# Don't add explicit optimize flags here, set them in configure.in, see bug 407794.  Also a comment above the CFLAGS/CXXFLAGS explaining that this is for Breakpad would be helpful.

Comment 8

[Robert Sayre]

Attachment: address ted's comments

Comment 9

[Mike Schroepfer]

Comment on attachment 292484 [details] [diff] [review]
address ted's comments

go go sayre

Comment 10

[Robert Sayre]

This had to be backed out due to a test failure on Linux.

It seemed to hurt mac performance on some tests and improve it on others. Makes sense: shifting to -Os in a few too many places.

Comment 11

[Robert Sayre]

Attachment: patch with wallpaper for various issues

Comment 12

[Robert Sayre]

Comment on attachment 293018 [details] [diff] [review]
patch with wallpaper for various issues

This contains a workaround for bug 408258, since we should stop shipping builds with that problem immediately. It reverts the OS X default optimization to O2, since -Os that regressed Tp and Tdhtml slightly. And since -Os is no longer the default for spidermonkey on mac, this incorporates the already-reviewed patch from bug 407600.

Comment 13

[Robert Sayre]

Comment on attachment 293018 [details] [diff] [review]
patch with wallpaper for various issues

it blocks, no need to approve

Comment 14

[Robert Sayre]

This works. Filed bugs on issues uncovered by this change:

bug 408258 
bug 408418
bug 408422