338880 - SuiteRunner doesn't compile with --enable-debug

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Karsten Düsterloh]

OSX 10.4.6, GCC 3.3. Compiling optimzed/non-debug builds of SuiteRunner works, but compiling with --enable-debug fails:

c++ -I/usr/X11R6/include -fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Wcast-align -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-long-long -nostdinc -nostdinc++ -I/Developer/SDKs/MacOSX10.2.8.sdk/usr/include/gcc/darwin/3.3/c++ -I/Developer/SDKs/MacOSX10.2.8.sdk/usr/include/gcc/darwin/3.3/c++/ppc-darwin -I/Developer/SDKs/MacOSX10.2.8.sdk/usr/include/
gcc/darwin/3.3/c++/backward -isystem /Developer/SDKs/MacOSX10.2.8.sdk/usr/include/gcc/darwin/3.3 -isystem /Developer/SDKs/MacOSX10.2.8.sdk/usr/include -F/Developer/SDKs/MacOSX10.2.8.sdk/System/Library/Frameworks -fpascal-strings -no-cpp-precomp -fno-common -fshort-wchar -I/Developer/SDKs/MacOSX10.2.8.sdk/Developer/Headers/FlatCarbon -pipe -DDEBUG -D_DEBUG -DDEBUG_karsten -DTRACING -g -O...
...-fPIC -o libappcomps.dylib nsModule.o nsDirectoryViewer.o nsBrowserInstance.onsBrowserStatusFilter. o nsBookmarksService.o nsDownloadManager.o nsGlobalHistory.o nsRelatedLinksHandler.o patricia.o nsAEApplicationClass.o nsAEClassDispatcher.o nsAEClassIterator.o nsAECoercionHandlers.o nsAECompare.o nsAECoreClass.o nsAEDocumentClass.o nsAEEventHandling.o nsAEGenericClass.o nsAEGetURLSuiteHandler.o
 nsAEMozillaSuiteHandler.o nsAESpyglassSuiteHandler.o nsAETokens.o nsAEUtils.o nsAEWindowClass.o nsMacUtils.o nsWindowUtils.o nsDocLoadObserver.o -Wl,-dead_strip -framework Carbon../../../dist/lib/ libunicharutil_s.a -L../../../dist/bin -lxpcom -lxpcom_core -L../../../dist/bin -L../../../dist/lib -lplds4 -lplc4 -lnspr4 -L/Developer/SDKs/MacOSX10.2.8.sdk/usr/lib/gcc/darwin -L/Developer/SDKs/MacOSX1
rm -f libappcomps.dylib
0.2.8.sdk/usr/lib/gcc/darwin/3.3 -L/Developer/SDKs/MacOSX10.2.8.sdk/usr/lib -L../../../dist/bin -lmozjs -bundle -L/Developer/SDKs/MacOSX10.2.8.sdk/usr/lib/gcc/darwin -L/Developer/SDKs/MacOSX10.2.8.sdk/usr/lib/gcc/darwin/3.3 -L/Developer/SDKs/MacOSX10.2.8.sdk/usr/lib
ld: Undefined symbols:
__ZN16nsMacCommandLine15sMacCommandLineE
__ZN16nsMacCommandLine16HandleOpenOneDocERK6FSSpecm
__ZN16nsMacCommandLine17HandlePrintOneDocERK6FSSpecm
__ZN16nsMacCommandLine4QuitE8TAskSave
__ZN16nsMacCommandLine23DispatchURLToNewBrowserEPKc
make[6]: *** [libappcomps.dylib] Error 1
make[5]: *** [libs] Error 2
make[4]: *** [libs] Error 2
make[3]: *** [libs_tier_50] Error 2
make[2]: *** [tier_50] Error 2
make[1]: *** [default] Error 2
make: *** [build] Error 2

Comment 1

[Karsten Düsterloh]

Attachment: don't build appleevents twice

Mark found that we were building appleevents twice, one time in XPFE, one time in toolkit. Stefan's patch here fixes this (I'm just the messenger ;-) ). After a clean build SR comes up now for me...

Comment 2

[Mark Banner (:standard8)]

Taking as I found the solution ;-)

Comment 3

[Karsten Düsterloh]

We still need to make sure that non-debug builds still work - and we should find out, why non-debug builds did compile before at all...

Comment 4

[Stephen Donner [:stephend] Not actively reading bugmail]

Comment on attachment 222933 [details] [diff] [review]
don't build appleevents twice

>Index: xpfe/components/build/Makefile.in
>===================================================================
>RCS file: /cvsroot/mozilla/xpfe/components/build/Makefile.in,v
>retrieving revision 1.76

<snip>

>+# XXX When Suite is a fully fledged xul app, this ifneq can be reduced.
>+ifneq (11,$(MOZ_SUITE)$(MOZ_XUL_APP))

Instead of "reduced," didn't you actually mean eliminated?

<snip>

Comment 5

[Mark Banner (:standard8)]

(In reply to comment #4)
> (From update of attachment 222933 [details] [diff] [review] [edit])
> >Index: xpfe/components/build/Makefile.in
> >===================================================================
> >RCS file: /cvsroot/mozilla/xpfe/components/build/Makefile.in,v
> >retrieving revision 1.76
> <snip>
> >+# XXX When Suite is a fully fledged xul app, this ifneq can be reduced.
> >+ifneq (11,$(MOZ_SUITE)$(MOZ_XUL_APP))
> Instead of "reduced," didn't you actually mean eliminated?
> <snip>

Nope, we'd still need the MOZ_SUITE part at the moment I think. Even when we are a MOZ_XUL_APP we'd still be needing the build part of components, and we can't remove the extra code as I believe Camino still requires it.

Comment 6

[Karsten Düsterloh]

> We still need to make sure that non-debug builds still work

They do, just tested it (--disable-debug --disable-tests '--enable-optimize=-O2 -g'). Comes up and works to the best of its current capabilities. ;-)

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 222933 [details] [diff] [review]
don't build appleevents twice

(In reply to comment #6)
> > We still need to make sure that non-debug builds still work
> 
> They do, just tested it (--disable-debug --disable-tests '--enable-optimize=-O2
> -g'). Comes up and works to the best of its current capabilities. ;-)
> 

Looks like we can go for reviews then :-)

Comment 8

[[e-mail]]

Comment on attachment 222933 [details] [diff] [review]
don't build appleevents twice

Are there any MOZ_XUL_APPs that do need to build appleevents here?

Comment 9

[Mark Banner (:standard8)]

(In reply to comment #8)
> (From update of attachment 222933 [details] [diff] [review] [edit])
> Are there any MOZ_XUL_APPs that do need to build appleevents here?
> 
I believe the following applications build in xpfe/components/build (using their configure.in names):

suite, browser, xulrunner, minimo, macbrowser.

Of those, only the first 3 are currently able to build as MOZ_XUL_APPs. Therefore I think we can reduce that to just ifndef MOZ_XUL_APP as well as a few others in that file.

Comment 10

[Benjamin Smedberg]

I believe that minimo is building as MOZ_XUL_APP now. But that doesn't affect appleevents at all... ;-)

Comment 11

[Mark Banner (:standard8)]

(In reply to comment #10)
> I believe that minimo is building as MOZ_XUL_APP now. But that doesn't affect
> appleevents at all... ;-)
> 

Yes you're right, it is building as MOZ_XUL_APP, however MOZ_XPFE_COMPONENTS isn't defined which means it just uses the exports from components. Therefore I think we can start to tidy this up a bit - new patch coming up later once I verify a couple of builds still work.

Comment 12

[Mark Banner (:standard8)]

Attachment: don't build appleevents twice and tidy up some ifdefs

Revised patch using checks on just MOZ_XUL_APP, I've also included a few other tidy ups now that we know more about who is using what.

Comment 13

[[e-mail]]

Comment on attachment 223226 [details] [diff] [review]
don't build appleevents twice and tidy up some ifdefs

It's not clear to me, at least at this time of night, how this interacts with Thunderbird and/or Camino.

>-# don't build ALERTS_SERVICE for firefox, xulrunner or toolkit-based suite
>-ifndef MOZ_PHOENIX
>-ifndef MOZ_XULRUNNER
>-# XXX When Suite is a fully fledged xul app, this ifdef can be reduced.
>-ifneq (11,$(MOZ_SUITE)$(MOZ_XUL_APP))
>+ifndef MOZ_XUL_APP
> ifneq (,$(filter $(MOZ_GFX_TOOLKIT),windows gtk gtk2))
> ALERTS_SERVICE=1
> DEFINES += -DALERTS_SERVICE
> endif
> endif
>-endif
>-endif
Does this mean a followup bug to implement s/ALERTS_SERVICE/!MOZ_XUL_APP/ ?

Comment 14

[Mark Banner (:standard8)]

(In reply to comment #13)
> (From update of attachment 223226 [details] [diff] [review] [edit])
> It's not clear to me, at least at this time of night, how this interacts with
> Thunderbird and/or Camino.

Thunderbird doesn't enter xpfe/components/build (see xpfe/components/Makefile.in) - the change in the xpfe/components/Makefile.in also wouldn't affect Thunderbird (its not a browser).

Camino is macbrowser and doesn't build as a MOZ_XUL_APP currently. If they do decide to then it'll mean they'll have to pickup the toolkit components unless they change the ifdefs again. If you look at the dirs (updates, startup, alerts, extensions) we're excluding from MOZ_XUL_APP builds with this patch, then I think you would probably agree its the minimalish set of components you need to take from toolkit to get a reasonable MOZ_XUL_APP build starting up anyway.

> >-# don't build ALERTS_SERVICE for firefox, xulrunner or toolkit-based suite
> >-ifndef MOZ_PHOENIX
> >-ifndef MOZ_XULRUNNER
> >-# XXX When Suite is a fully fledged xul app, this ifdef can be reduced.
> >-ifneq (11,$(MOZ_SUITE)$(MOZ_XUL_APP))
> >+ifndef MOZ_XUL_APP
> > ifneq (,$(filter $(MOZ_GFX_TOOLKIT),windows gtk gtk2))
> > ALERTS_SERVICE=1
> > DEFINES += -DALERTS_SERVICE
> > endif
> > endif
> >-endif
> >-endif
> Does this mean a followup bug to implement s/ALERTS_SERVICE/!MOZ_XUL_APP/ ?

We could, but then we'd have to make it take account of the gfx toolkit as well, so you'd end up with 4 checks again - !MOZ_XUL_APP && (win || gtk || gtk2). Which could be done depending on what we think is nicer.

Comment 15

[[e-mail]]

(In reply to comment #14)
>Thunderbird doesn't enter xpfe/components/build (see
>xpfe/components/Makefile.in) - the change in the xpfe/components/Makefile.in
>also wouldn't affect Thunderbird (its not a browser).
OK, so basically we're changing MOZ_PHOENIX && MOZ_XULRUNNER to MOZ_XUL_APP?

Comment 16

[[e-mail]]

Comment on attachment 223226 [details] [diff] [review]
don't build appleevents twice and tidy up some ifdefs

>   $(NULL)
> endif
> 
>+endif # MOZ_XULRUNNER
>+endif # MOZ_PHOENIX
>+
>+ifndef $(MOZ_XUL_APP)
> ifneq (,$(filter mac cocoa,$(MOZ_WIDGET_TOOLKIT)))
> ifneq ($(MOZ_BUILD_APP),macbrowser)
> SHARED_LIBRARY_LIBS += ../../bootstrap/appleevents/$(LIB_PREFIX)appleevents_s.$(LIB_SUFFIX)
> EXTRA_DSO_LDOPTS += $(TK_LIBS)
> endif
> endif
>-
>-endif # MOZ_XULRUNNER
>-endif # MOZ_PHOENIX
>+endif
So rather than moving the endif, can we not change this to MOZ_XUL_APP too?

> #include "nsDocShellCID.h"
> #include "nsDownloadManager.h"
> #include "nsDownloadProxy.h"
>-// XXX When Suite is a fully fledged xul app, this ifdef can be reduced.
>-#if !defined(MOZ_SUITE) || !defined(MOZ_XUL_APP)
>+#endif // !defined(MOZ_PHOENIX) && !defined(MOZ_XULRUNNER) && !defined(MOZ_MACBROWSER)
>+#if !defined(MOZ_XUL_APP)
> #include "nsAppStartup.h"
> #include "nsCommandLineService.h"
> #include "nsUserInfo.h"
> #endif
> 
>-#endif // !defined(MOZ_PHOENIX) && !defined(MOZ_XULRUNNER) && !defined(MOZ_MACBROWSER)
I don't think this is right, it looks as if camino will now get the last three includes. Would it make more sense to change that big if to ifdef MOZ_SUITE and then ifndef MOZ_XUL_APP for those last three includes? (Same goes for the constructor macros and components list too, I guess).

>+#if !defined(MOZ_XUL_APP)
Nit: should be #ifndef MOZ_XUL_APP

Comment 17

[Mark Banner (:standard8)]

(In reply to comment #15)
> (In reply to comment #14)
> >Thunderbird doesn't enter xpfe/components/build (see
> >xpfe/components/Makefile.in) - the change in the xpfe/components/Makefile.in
> >also wouldn't affect Thunderbird (its not a browser).
> OK, so basically we're changing MOZ_PHOENIX && MOZ_XULRUNNER to MOZ_XUL_APP?

Yes, but only where SeaMonkey doesn't want to include items currently.

(In reply to comment #16)
> (From update of attachment 223226 [details] [diff] [review] [edit])
> >   $(NULL)
> > endif
> > 
> >+endif # MOZ_XULRUNNER
> >+endif # MOZ_PHOENIX
> >+
> >+ifndef $(MOZ_XUL_APP)
> > ifneq (,$(filter mac cocoa,$(MOZ_WIDGET_TOOLKIT)))
> > ifneq ($(MOZ_BUILD_APP),macbrowser)
> > SHARED_LIBRARY_LIBS += ../../bootstrap/appleevents/$(LIB_PREFIX)appleevents_s.$(LIB_SUFFIX)
> > EXTRA_DSO_LDOPTS += $(TK_LIBS)
> > endif
> > endif
> >-
> >-endif # MOZ_XULRUNNER
> >-endif # MOZ_PHOENIX
> >+endif
> So rather than moving the endif, can we not change this to MOZ_XUL_APP too?

Errm, I am changing it to ifndef MOZ_XUL_APP? The endif is moving to encapsulate other stuff that suiterunner still wants.

Though I think we should be able to remove that section completely once we throw the switch on MOZ_XUL_APP for suite (camino doesn't require it) - so I should add an XXX note for that. It might actually be better to change it to ifndef MOZ_XUL_APP ifdef MOZ_SUITE with an XXX as that's essentially what it will end up being.

> > #include "nsDocShellCID.h"
> > #include "nsDownloadManager.h"
> > #include "nsDownloadProxy.h"
> >-// XXX When Suite is a fully fledged xul app, this ifdef can be reduced.
> >-#if !defined(MOZ_SUITE) || !defined(MOZ_XUL_APP)
> >+#endif // !defined(MOZ_PHOENIX) && !defined(MOZ_XULRUNNER) && !defined(MOZ_MACBROWSER)
> >+#if !defined(MOZ_XUL_APP)
> > #include "nsAppStartup.h"
> > #include "nsCommandLineService.h"
> > #include "nsUserInfo.h"
> > #endif
> > 
> >-#endif // !defined(MOZ_PHOENIX) && !defined(MOZ_XULRUNNER) && !defined(MOZ_MACBROWSER)
> I don't think this is right, it looks as if camino will now get the last three
> includes. Would it make more sense to change that big if to ifdef MOZ_SUITE and
> then ifndef MOZ_XUL_APP for those last three includes? (Same goes for the
> constructor macros and components list too, I guess).

Yep, I think you're right here. !browser && !xulrunner && !macbrowser = suite only as minimo doesn't use xpfe/components/build.

That should tidy things up a bit :-)

Comment 18

[[e-mail]]

(In reply to comment #17)
>Errm, I am changing it to ifndef MOZ_XUL_APP? The endif is moving to
>encapsulate other stuff that suiterunner still wants.
Sorry, I think I mixed up MOZ_XUL_APP with MOZ_SUITE there.
I'm trying to compare the SHARED_LIBRARY_LIBS with the LOCAL_INCLUDES lines.
Since only Suite uses the bookmarks/downloadmanager/history/related libs,
I'm hoping that only Suite needs the appropriate local includes.

Comment 19

[Mark Banner (:standard8)]

Attachment: don't build appleevents and lots of tidy up

I'm not going to request reviews yet as I need to do some builds first, however, this is the current WIP.

Comment 20

[[e-mail]]

Comment on attachment 223344 [details] [diff] [review]
don't build appleevents and lots of tidy up

I prefer #ifdef to #if defined()

Comment 21

[Mark Banner (:standard8)]

Attachment: don't build appleevents and lots of tidy up v2

Fixed a couple of bugs I'd introduced and corrected #if defined() to #ifdef in the couple of places I'd moved stuff but not corrected it.

Requesting sr from Neil first as he generally gets there before benjamin anyway ;-)

Comment 22

[[e-mail]]

Comment on attachment 223488 [details] [diff] [review]
don't build appleevents and lots of tidy up v2

>Index: xpfe/components/Makefile.in
It it worth simplifying the DIRS += winhooks urlwidget ifdefs?

Comment 23

[Mark Banner (:standard8)]

Attachment: don't build appleevents and lots of tidy up v3

This patch is the same as the last one, but simplifies the DIRS include that Neil commented on. Requesting r.

Comment 24

[Mark Banner (:standard8)]

Checked in - One minor fixup for Camino build bustage (some includes aren't necessary for Camino).