remove tons of options from configure

RESOLVED FIXED in mozilla1.9.3a1

Status

()

Core
Build Config
RESOLVED FIXED
8 years ago
6 years ago

People

(Reporter: ted, Assigned: Mitch)

Tracking

Trunk
mozilla1.9.3a1
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

8 years ago
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

8 years ago
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

8 years ago
I remember Jeremy tried something in configure that didn't work. Don't recall which.
(Reporter)

Comment 3

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

8 years ago
"--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.
(Reporter)

Comment 11

8 years ago
I think a lot of people make that assumption incorrectly, which is a good reason to fix this bug.
(Reporter)

Updated

8 years ago
Depends on: 514131
(Assignee)

Comment 12

8 years ago
I'm taking this bug.
Assignee: nobody → mitch_1_2
Status: NEW → ASSIGNED
(Assignee)

Comment 13

8 years ago
Created attachment 398642 [details] [diff] [review]
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.
Attachment #398642 - Flags: review?(ted.mielczarek)

Comment 14

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

8 years ago
Created attachment 399009 [details] [diff] [review]
Patch v1.1

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

8 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+
Attachment #399009 - Flags: review+
(Reporter)

Comment 18

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

Updated

8 years ago
Blocks: 516758
(Assignee)

Comment 19

8 years ago
Please resolve fixed; I filed bug 516758 as a followup.
(Reporter)

Updated

8 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 8 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...
Keywords: dev-doc-needed
(Assignee)

Comment 21

8 years ago
Docs updated.
Keywords: dev-doc-needed
could this have caused a Ts regression? it's in the range for bug 517915.
(Reporter)

Comment 23

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

Updated

8 years ago
Depends on: 535048
Target Milestone: --- → mozilla1.9.3a1
Depends on: 546174
Blocks: 546177
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.