This is a followup to bug 513924.

Open to suggestions.

Note bug 517557 is removing --disable-canvas.

how about: 5288 MOZ_ARG_ENABLE_BOOL(native-uconv, 5305 MOZ_ARG_ENABLE_BOOL(plaintext-editor-only, 6841 MOZ_ARG_ENABLE_BOOL(long-long-warning, 7142 MOZ_ARG_ENABLE_BOOL(static, 4951 MOZ_ARG_DISABLE_BOOL(pango, 5219 MOZ_ARG_DISABLE_BOOL(accessibility, possibly: 5932 MOZ_ARG_ENABLE_BOOL(leaky, 6656 MOZ_ARG_ENABLE_BOOL(insure,

A good set of suggestions, just some comments: 5288 MOZ_ARG_ENABLE_BOOL(native-uconv, smontagu would need to comment on this, I think 7142 MOZ_ARG_ENABLE_BOOL(static, Thunderbird is still using this, we do error if you try to use it with Firefox, at least. 5219 MOZ_ARG_DISABLE_BOOL(accessibility, I know we're not shipping with a11y enabled on Mac yet, so this might still be useful. In addition, I think removing this would make it difficult to build on mingw. I don't think removing any of those other options would cause any problems.

From my bug 513924 comment 17, you could ditch MOZ_JSLOADER and MOZ_VIEW_SOURCE throughout the codebase here or in a separate bug. Things from bz's bug 513924 comment 4 that didn't get fixed there: --enable-places Enable 'places' bookmark/history implementation Not sure we need this option anymore. Dietrich? --enable-help-viewer Enable help viewer bsmedberg suggested that TB/SM might be using this, but I'm not sure. I guess you could kill the option and leave it for confvars.sh if they need it. dmose? --disable-freetypetest Do not try to compile and run a test FreeType program This comes wholesale from http://mxr.mozilla.org/mozilla-central/source/build/autoconf/freetype2.m4. Maybe we can remove that whole file? roc, do we still need freetype2 for anything? (Aside from the one in the tree that WinCE builds are using.) The only place I see it being used is if you --disable-pango, which we can probably remove anyway. --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 These unfortunately come from http://mxr.mozilla.org/mozilla-central/source/build/autoconf/libIDL.m4 and http://mxr.mozilla.org/mozilla-central/source/build/autoconf/glib.m4 respectively, and I don't think we can ditch those currently. Maybe if someone finishes rewriting xpidl in Python we can ditch the IDL one. --disable-postscript Disable PostScript printing support This I don't know, would need to ask someone with printing knowledge (good luck).

(In reply to comment #5) > --enable-help-viewer Enable help viewer > > bsmedberg suggested that TB/SM might be using this, but I'm not sure. I guess > you could kill the option and leave it for confvars.sh if they need it. dmose? SeaMonkey still uses it (although I guess at some stage that code should be moved). Thunderbird doesn't.

(In reply to comment #5) > Things from bz's bug 513924 comment 4 that didn't get fixed there: > --enable-places Enable 'places' bookmark/history implementation > Not sure we need this option anymore. Thunderbird doesn't quite want places yet... they may do soon though.

(In reply to comment #6) > (In reply to comment #5) > > --enable-help-viewer Enable help viewer > SeaMonkey still uses it (although I guess at some stage that code should be > moved). Thunderbird doesn't. (In reply to comment #7) > (In reply to comment #5) > > Things from bz's bug 513924 comment 4 that didn't get fixed there: > > --enable-places Enable 'places' bookmark/history implementation > Thunderbird doesn't quite want places yet... they may do soon though. In light of these, we should be able to remove the configure options, but leave the configure variables, so TB/SM can continue to disable them in confvars.sh. Nobody should need to disable them from the commandline.

(In reply to comment #8) > In light of these, we should be able to remove the configure options, but leave > the configure variables, so TB/SM can continue to disable them in confvars.sh. > Nobody should need to disable them from the commandline. Agreed, I'm happy with that.

Yeah, I did a test build of Tb with Places the other day, and never for a second thought that there might still be a configure option, so I just hacked my confvars :)

(In reply to comment #4) > 5288 MOZ_ARG_ENABLE_BOOL(native-uconv, > > smontagu would need to comment on this, I think I would if I knew exactly what is being proposed here ;-) I believe there are Linux distros that enable native uconv in their Firefox builds.

(In reply to comment #5) > --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 > > These unfortunately come from > http://mxr.mozilla.org/mozilla-central/source/build/autoconf/libIDL.m4 and > http://mxr.mozilla.org/mozilla-central/source/build/autoconf/glib.m4 > respectively, and I don't think we can ditch those currently. Maybe if someone > finishes rewriting xpidl in Python we can ditch the IDL one. Those two flags are used for the old libIDL. For the new libIDL (libIDL 2) we use pkgconfig: 7333 PKG_CHECK_MODULES(LIBIDL, libIDL-2.0 >= 0.8.0 glib-2.0 gobject-2.0, _LIBIDL_FOUND=1,_LIBIDL _FOUND=) So we could probably ditch libIDL.m4/glib.m4

This patch removes the following configure options (leaving corresponding configure variables): --enable-help-viewer --enable-places. Insure++ instrumentation support, and configure options, are completely removed: --enable-insure --with-insure-dirs --with-insure-exclude-dirs.

(In reply to comment #4) > 5219 MOZ_ARG_DISABLE_BOOL(accessibility, > > I know we're not shipping with a11y enabled on Mac yet, so this might still be > useful. In addition, I think removing this would make it difficult to build on > mingw. There's also https://developer.mozilla.org/en/atlbase.h

Comment on attachment 434612 [details] [diff] [review] Patch (v1) Good stuff! You should also be able to remove the MOZ_PLACES=1 from browser/confvars.sh: http://mxr.mozilla.org/mozilla/source/browser/confvars.sh#44

(In reply to comment #15) > Good stuff! You should also be able to remove the MOZ_PLACES=1 from > browser/confvars.sh: > http://mxr.mozilla.org/mozilla/source/browser/confvars.sh#44 Done.

[Places] > we should be able to remove the configure options, but leave > the configure variables, so TB/SM can continue to disable them in confvars.sh. What about xulrunner? Other apps are likely not to want places either, for the same reasons. You shouldn't force xulrunner devs to know undocumented env vars and set them via some obscure or custom script. That's what configure is for. If you want to force it "on" for Firefox, then do something like #if app=browser and not defined PLACES #error Places is needed for Firefox

(I'm speaking as xulrunner dev here.)

The codesize is not significant enough to matter, so allowing developers to disable it for custom xulrunner builds is of negligible gain. Having it in configure means an extra codepath that we may or may not be supporting, meaning that the configure option might stop working due to disuse anyway. I'd prefer to have it not present and pretend we actually support it. We're not obligated to provide options every single feature in the platform. The fewer options we have, the smaller our support matrix, and the more likely everything is to work consistently.

Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/f95fc26312b8

Bustage fix followup: http://hg.mozilla.org/mozilla-central/rev/aea2b08ae4a3

Under VS2010 when building Seamonkey, one needs to add the '--disable-activex' option, otherwise bug 559537 pops up. In IRC, I was told that the nightlies and the official releases don't have activeX enabled (i.e., I believe option --disable-activex is added to the configuration). If that's the case, why not simply have the configure script default to disable activeX and have an '--enable-activeX' for those who require it for whatever reasons?

That's incorrect. Our official builds build ActiveX by default. In any case, we haven't removed that option.

AFAICT this broke SeaMonkey's Help window (bug 561663), cf. #ifdef MOZ_HELP_VIEWER in mozilla/toolkit/locales/jar.mn. SeaMonkey defines MOZ_HELP_VIEWER in suite/confvars.sh and comm-central's configure.in still contains the related code that was removed from mozilla-central's version but that doesn't seem to suffice. Unfortunately I know too little about the setup to tell what's needed to fix it but I'm sure one of you does. ;-)

Yup, removing the help viewer stuff in this patch was completely the wrong thing to do given that this code exists, is off by default and needs some way to actually be turned on for those that need it (SeaMonkey happens to be the consumer we have inside Mozilla). Please revert that part. And note that it needs to happen within a week as this problem blocks the SeaMonkey 2.1 Alpha that should be cut early next week (code freeze is Tuesday 23:59 PDT).

Comment on attachment 441686 [details] [diff] [review] Fix for SeaMonkey bustage I don't really want this in the top-level configure. Can't you fix this in SeaMonkey's build system instead?

Comment on attachment 441686 [details] [diff] [review] Fix for SeaMonkey bustage Oh, I see, this code still lives in toolkit. If SeaMonkey is the only consumer, then this really should move to comm-central/suite. r=me on the condition that you file a bug on moving that directory to suite, and removing it from toolkit (along with this configure block).

Ted, Gavin previously agreed to keep the code in toolkit as there are external consumers as well.

Gavin notes this is already filed as bug 425541, so carry on.

We currently warn about using "long long" types, but I'm told that's inane. This patch adds -Wno-long-long to CFLAGS/CXXFLAGS where -pedantic is used, and removes the long-long-warning option.

Comment on attachment 445745 [details] [diff] [review] Remove long-long-warning I approve of the concept, but I am not qualified to review patches to configure.in. Over to ted.

From the C++ end of things, the argument for unconditionally disabling this warning goes like so: The only purpose of the warning is to notify the programmer that this type is nonstandard and might not be supported by every compiler out there. Indeed, it is not in C++98. However, it is in C99, and will be in C++1x; it has been supported by G++ since the mid-nineties if not longer; and as best I can tell from MSDN, it has been supported by VC++ since earlier than VS2005. Empirically, we have tons of unqualified uses of 'long long' in our source tree, so if it were going to be a problem it would have been a problem already.

On the gripping hand, we appear to be using 'long long' to mean 'int64_t' and maybe we should start saying what we mean more consistently.

Comment on attachment 445745 [details] [diff] [review] Remove long-long-warning ># HG changeset patch ># Parent 763e567872bba47eb6e0e78e5c747e34e4b6e88e ># User Mitchell Field <mitchell.field@live.com.au> >Bug 516758 - Remove --disable-long-long-warning. > >diff --git a/configure.in b/configure.in >--- a/configure.in >+++ b/configure.in >@@ -1489,22 +1489,18 @@ if test "$GNU_CC"; then > ;; > esac > fi > > dnl Turn pedantic on but disable the warnings for long long > _PEDANTIC=1 Fix this comment, please. If Zack is ok with it, I'm ok with it. r=me with that fix.

Comment on attachment 445745 [details] [diff] [review] Remove long-long-warning Pushed to m-c: http://hg.mozilla.org/mozilla-central/rev/d5d5ed6d3e1c

Merged patch of attachment 435945 [details] [diff] [review], attachment 441686 [details] [diff] [review], and attachment 445745 [details] [diff] [review].

Comment on attachment 454457 [details] [diff] [review] Sync for comm-central >+dnl ======================================================== > dnl = Enable help viewer (off by default) > dnl ======================================================== Please keep this option intact as far as c-c has it. It was only retained in _any_ form in m-c because c-c actually uses it. My reasoning -- We should allow SeaMonkey to build without this (--disable-*) though it has little use for Thunderbird. We may wish to drop it later, but for now I'd rather keep it in. Also for future, new bugs are better for c-c ports like this it helps keep clutter down in the original bug(s).

(In reply to comment #39) OK. I file as bug 575509 for this bug in comm-central.

BTW, why this bug still open?

(In reply to comment #41) > BTW, why this bug still open? IMO it shouldn't be... resolving. Mitch lets use a fresh bug for any followup work. If you disagree reopen, but please lets identify _when_ this will be closed if you do.