513924 - remove tons of options from configure

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[(not currently active) Ted Mielczarek]

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.

Comment 1

[(not currently active) Ted Mielczarek]

Of course I meant confvars.sh:
http://mxr.mozilla.org/mozilla-central/source/browser/confvars.sh
http://mxr.mozilla.org/mobile-browser/source/confvars.sh
http://mxr.mozilla.org/mozilla-central/source/xulrunner/confvars.sh

Comment 2

[Axel Hecht [:Pike]]

I remember Jeremy tried something in configure that didn't work. Don't recall which.

Comment 3

[(not currently active) Ted Mielczarek]

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

[Boris Zbarsky [:bzbarsky]]

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

[Benjamin Smedberg]

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

Comment 6

[Mitchell Field [:Mitch]]

"--disable-xpcom-obsolete".

Thunderbird may still may need xpcom-obsolete, but I'm told that they could enable via confvars.sh.

Comment 7

[Mark Banner (:standard8)]

(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

[Christian :Biesinger (don't email me, ping me on IRC)]

(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

[Srirang G Doddihal (Brahmana)]

(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

[Benjamin Smedberg]

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.

Comment 11

[(not currently active) Ted Mielczarek]

I think a lot of people make that assumption incorrectly, which is a good reason to fix this bug.

Comment 12

[Mitchell Field [:Mitch]]

I'm taking this bug.

Comment 13

[Mitchell Field [:Mitch]]

Attachment: Patch v1.0

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 14

[Ray Kiddy]

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

[Christian :Biesinger (don't email me, ping me on IRC)]

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).

Comment 16

[Mitchell Field [:Mitch]]

Attachment: Patch v1.1

Updated to apply to current mozilla-central state.

Comment 17

[(not currently active) Ted Mielczarek]

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.

Comment 18

[(not currently active) Ted Mielczarek]

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?

Comment 19

[Mitchell Field [:Mitch]]

Please resolve fixed; I filed bug 516758 as a followup.

Comment 20

[Justin Dolske [:Dolske]]

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...

Comment 21

[Mitchell Field [:Mitch]]

Docs updated.

Comment 22

[Dietrich Ayala (:dietrich)]

could this have caused a Ts regression? it's in the range for bug 517915.

Comment 23

[(not currently active) Ted Mielczarek]

I can't imagine how it could, it only removed unused code and configure options.

Comment 24

[Justin Wood (:Callek)]

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

[Justin Wood (:Callek)]

(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.