Closed Bug 513924 Opened 11 years ago Closed 11 years ago
remove tons of options from configure
Our configure is overloaded with options. We should remove a lot of them. Stuff like --disable-view-source and --disable-jsloader just need to die. Things that we really do want to be controlled per-app or per-platform can stay, but the configure commandline options should be removed anyway. Apps can control things via build.sh, and I'm fine with platform-specific sections of configure controlling enabling/disabling of features.
I remember Jeremy tried something in configure that didn't work. Don't recall which.
Yeah, we get posts to dev.builds all the time about "my build doesn't work" and then they paste their mozconfig and it's 100 lines of junk. We should not have options that don't work, for sure, but we should also take out most of them just for the "attractive nuisance" factor. Even if they work they're rarely what the person actually wants to do.
People still need --without-x occasionally on Darwin and Windows, IIRC. I believe mobile wanted --disable-inspector-apis still, although I'm not certain about that. --with-crashreporter-enable-percent is used by our release mozconfigs --enable-help-view may still be used by SM/TB, although I kinda hope not. If you change --enable-old-abi-compat-wrappers we should remove the code which goes with the configure option in nsAppRunner.cpp and elsewhere. --enable-xterm-updates is still used, although it may be possible to auto-detect --with-java-* is definitely still used for JavaXPCOM
"--disable-xpcom-obsolete". Thunderbird may still may need xpcom-obsolete, but I'm told that they could enable via confvars.sh.
(In reply to comment #6) > "--disable-xpcom-obsolete". > > Thunderbird may still may need xpcom-obsolete, but I'm told that they could > enable via confvars.sh. We got rid of xpcom-obsolete ages ago.
(In reply to comment #4) > Things that look removable to me: > > --with-x use the X Window System That's needed since you can use this to specify the location of the X headers/libraries > --enable-xterm-updates Update XTERM titles with current command. I like that feature... I'd prefer to keep it
(In reply to comment #4) > Things that look removable to me: > > --disable-view-source Disable view source support > --disable-inspector-apis Disable the DOM inspection APIs I am using these options under the impression that there will be some gains in terms of memory consumption and performance. The resulting browser is being used to run automated tests on a continuous basis and hence any gain in performance or memory really matters. If my assumption is not right or if there is a better way to achieve the same then I am totally fine with these options being removed. Otherwise I request you to keep these options.
That is not really a reasonable assumption, and it increases the support matrix. I don't think we want to continue supporting arbitrary "make my build smaller" options.
I think a lot of people make that assumption incorrectly, which is a good reason to fix this bug.
I'm taking this bug.
Assignee: nobody → mitch_1_2
Status: NEW → ASSIGNED
Completely removed corresponding features: --with-embedding-profile=default|basic|minimal see http://wiki.mozilla.org/Gecko:Small_Device_Support --enable-xpcom-lea Use Lea malloc in xpcom --enable-boehm Enable the Boehm Garbage Collector --enable-efence Link with Electric Fence --enable-old-abi-compat-wrappers Support old GCC ABI symbols to ease the pain of the linux compiler change --enable-eazel-profiler-support Enable Corel/Eazel profiler support --enable-profile-modules Enable/disable profiling for specific modules Removed configure options: --disable-postscript Disable PostScript printing support --disable-view-source Disable view source support --disable-jsloader Disable xpcom js loader support --disable-inspector-apis Disable the DOM inspection APIs --disable-canvas Disable html:canvas feature.
Comment #8 says that Christian wants the -with-x option because "you can use this to specify the location of the X headers/libraries." Do the --x-includes=DIR and --x-libraries=DIR options not work for this? One of the problems with having too many options is that some will duplicate each other....
Oh, I forgot about those two. Doesn't really matter anyway since we can't do anything about them (all three come from AC_PATH_XTRA).
Updated to apply to current mozilla-central state.
Comment on attachment 399009 [details] [diff] [review] Patch v1.1 if test "$MOZ_VIEW_SOURCE"; then AC_DEFINE(MOZ_VIEW_SOURCE) fi Feels like we could just remove this completely and scrub the codebase. I'm fine with doing it in a followup, though. if test "$MOZ_JSLOADER"; then AC_DEFINE(MOZ_JSLOADER) fi Same with this. if test -n "$MOZ_ENABLE_CANVAS"; then AC_DEFINE(MOZ_ENABLE_CANVAS) fi AC_SUBST(MOZ_ENABLE_CANVAS) And this. Looks good from my end, you can file a followup on removing the rest of the things I mentioned. You should get signoff from bsmedberg on removing all the boehm gc stuff before landing this, since that's all in xpcom/, even if it doesn't work.
Attachment #399009 - Flags: review?(ted.mielczarek) → review+
Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/21e5393f648c Were you going to do more on this bug, or do you want it resolved fixed?
Please resolve fixed; I filed bug 516758 as a followup.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
OMG, so much cruft removed! \o/ It would be good to update https://developer.mozilla.org/en/Configuring_Build_Options too. Flagging dev-doc-needed for the cleanup, but anyone can do it...
could this have caused a Ts regression? it's in the range for bug 517915.
I can't imagine how it could, it only removed unused code and configure options.
Note remnants of MOZ_NO_INSPECTOR_APIS remain in codebase... (including configure.in) and I don't see _anywhere_ that it is set to a true value, followup worthy? (In reply to comment #13) > Created attachment 398642 [details] [diff] [review] > Patch v1.0 > > Completely removed corresponding features: > --enable-xpcom-lea Use Lea malloc in xpcom FYI we missed |XPCOM_USE_LEA| from autoconf.mk.in [will file a followup with a patch]
(In reply to comment #17) > Comment on attachment 399009 [details] [diff] [review] > Patch v1.1 > > if test "$MOZ_VIEW_SOURCE"; then > AC_DEFINE(MOZ_VIEW_SOURCE) > fi > > Feels like we could just remove this completely and scrub the codebase. I'm > fine with doing it in a followup, though. > > if test "$MOZ_JSLOADER"; then > AC_DEFINE(MOZ_JSLOADER) > fi > > Same with this. Both: Bug 589506 > if test -n "$MOZ_ENABLE_CANVAS"; then > AC_DEFINE(MOZ_ENABLE_CANVAS) > fi > AC_SUBST(MOZ_ENABLE_CANVAS) > > And this. Looks like this one was done already.
You need to log in before you can comment on or make changes to this bug.