Closed Bug 407794 Opened 12 years ago Closed 12 years ago
Nightlies should not override the module-specific build settings
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.
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
Summary: --enable-optimize overrides module-specific optimization settings → Nightlies should not override the module-specific build settings.
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.
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
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 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.
Attachment #292478 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 292484 [details] [diff] [review] address ted's comments go go sayre
Attachment #292484 - Flags: approval1.9? → approval1.9+
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 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.
Attachment #293018 - Flags: review?(ted.mielczarek) → review+
Comment on attachment 293018 [details] [diff] [review] patch with wallpaper for various issues it blocks, no need to approve
This works. Filed bugs on issues uncovered by this change: bug 408258 bug 408418 bug 408422
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.