Closed
Bug 513924
Opened 15 years ago
Closed 15 years ago
remove tons of options from configure
Categories
(Firefox Build System :: General, defect)
Firefox Build System
General
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla1.9.3a1
People
(Reporter: ted, Assigned: Mitch)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
ted
:
review+
benjamin
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Comment 1•15 years ago
|
||
Comment 2•15 years ago
|
||
I remember Jeremy tried something in configure that didn't work. Don't recall which.
Reporter | ||
Comment 3•15 years ago
|
||
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.
Comment 4•15 years ago
|
||
Things that look removable to me:
--with-x use the X Window System
--disable-freetypetest
Do not try to compile and run a test FreeType program
--disable-pango Disable usage of Pango
--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
--with-crashreporter-enable-percent=NN Enable sending crash reports by default on NN% of users. (default=100)
--disable-canvas Disable html:canvas feature
--enable-xpcom-lea Use Lea malloc in xpcom
--enable-help-viewer Enable help viewer
--enable-faststripe Use faststripe theme
--enable-boehm Enable the Boehm Garbage Collector
--enable-efence Link with Electric Fence
--enable-gczeal Enable zealous JavaScript GCing
--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-xterm-updates Update XTERM titles with current command.
--enable-long-long-warning
Warn about use of non-ANSI long long type
--disable-libIDLtest Do not try to compile and run a test libIDL program
--disable-glibtest Do not try to compile and run a test GLIB program
Things that might be removable:
--with-java-include-path=dir Location of Java SDK headers
--with-java-bin-path=dir Location of Java binaries (java, javac, jar)
--enable-places Enable 'places' bookmark/history implementation
Comment 5•15 years ago
|
||
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
Assignee | ||
Comment 6•15 years ago
|
||
"--disable-xpcom-obsolete".
Thunderbird may still may need xpcom-obsolete, but I'm told that they could enable via confvars.sh.
Comment 7•15 years ago
|
||
(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.
Comment 8•15 years 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
Comment 9•15 years ago
|
||
(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.
Comment 10•15 years ago
|
||
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.
Reporter | ||
Comment 11•15 years ago
|
||
I think a lot of people make that assumption incorrectly, which is a good reason to fix this bug.
Assignee | ||
Comment 12•15 years ago
|
||
I'm taking this bug.
Assignee: nobody → mitch_1_2
Status: NEW → ASSIGNED
Assignee | ||
Comment 13•15 years ago
|
||
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.
Attachment #398642 -
Flags: review?(ted.mielczarek)
Comment 14•15 years ago
|
||
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....
Comment 15•15 years ago
|
||
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).
Assignee | ||
Comment 16•15 years ago
|
||
Updated to apply to current mozilla-central state.
Attachment #398642 -
Attachment is obsolete: true
Attachment #399009 -
Flags: review?(ted.mielczarek)
Attachment #398642 -
Flags: review?(ted.mielczarek)
Reporter | ||
Comment 17•15 years ago
|
||
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+
Updated•15 years ago
|
Attachment #399009 -
Flags: review+
Reporter | ||
Comment 18•15 years ago
|
||
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?
Assignee | ||
Comment 19•15 years ago
|
||
Please resolve fixed; I filed bug 516758 as a followup.
Reporter | ||
Updated•15 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 20•15 years ago
|
||
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...
Keywords: dev-doc-needed
Comment 22•15 years ago
|
||
could this have caused a Ts regression? it's in the range for bug 517915.
Reporter | ||
Comment 23•15 years ago
|
||
I can't imagine how it could, it only removed unused code and configure options.
Updated•15 years ago
|
Target Milestone: --- → mozilla1.9.3a1
Comment 24•14 years ago
|
||
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]
Comment 25•14 years ago
|
||
(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.
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
•