Closed
Bug 338880
Opened 19 years ago
Closed 19 years ago
SuiteRunner doesn't compile with --enable-debug
Categories
(SeaMonkey :: Build Config, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mnyromyr, Assigned: standard8)
References
Details
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
benjamin
:
review+
standard8
:
superreview+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Comment 1•19 years ago
|
||
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...
Reporter | ||
Comment 3•19 years ago
|
||
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•19 years ago
|
||
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>
Assignee | ||
Comment 5•19 years ago
|
||
(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.
Reporter | ||
Comment 6•19 years ago
|
||
> 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. ;-)
Assignee | ||
Comment 7•19 years ago
|
||
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 :-)
Attachment #222933 -
Flags: superreview?(neil)
Attachment #222933 -
Flags: review?(neil)
Comment 8•19 years ago
|
||
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?
Attachment #222933 -
Flags: review?(neil) → review?(benjamin)
Assignee | ||
Comment 9•19 years ago
|
||
(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•19 years ago
|
||
I believe that minimo is building as MOZ_XUL_APP now. But that doesn't affect appleevents at all... ;-)
Assignee | ||
Comment 11•19 years ago
|
||
(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.
Assignee | ||
Updated•19 years ago
|
Attachment #222933 -
Attachment is obsolete: true
Attachment #222933 -
Flags: superreview?(neil)
Attachment #222933 -
Flags: review?(benjamin)
Assignee | ||
Comment 12•19 years ago
|
||
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.
Attachment #223226 -
Flags: superreview?(neil)
Attachment #223226 -
Flags: review?(benjamin)
Comment 13•19 years ago
|
||
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/ ?
Assignee | ||
Comment 14•19 years ago
|
||
(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•19 years ago
|
||
(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•19 years ago
|
||
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
Assignee | ||
Comment 17•19 years ago
|
||
(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 :-)
Assignee | ||
Updated•19 years ago
|
Attachment #223226 -
Attachment is obsolete: true
Attachment #223226 -
Flags: superreview?(neil)
Attachment #223226 -
Flags: review?(benjamin)
Comment 18•19 years ago
|
||
(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.
Assignee | ||
Comment 19•19 years ago
|
||
I'm not going to request reviews yet as I need to do some builds first, however, this is the current WIP.
Comment 20•19 years ago
|
||
Comment on attachment 223344 [details] [diff] [review] don't build appleevents and lots of tidy up I prefer #ifdef to #if defined()
Assignee | ||
Comment 21•19 years ago
|
||
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 ;-)
Attachment #223344 -
Attachment is obsolete: true
Attachment #223488 -
Flags: superreview?(neil)
Comment 22•19 years ago
|
||
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?
Attachment #223488 -
Flags: superreview?(neil) → superreview+
Assignee | ||
Comment 23•19 years ago
|
||
This patch is the same as the last one, but simplifies the DIRS include that Neil commented on. Requesting r.
Attachment #223488 -
Attachment is obsolete: true
Attachment #223555 -
Flags: superreview+
Attachment #223555 -
Flags: review?(benjamin)
Updated•19 years ago
|
Attachment #223555 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 24•19 years ago
|
||
Checked in - One minor fixup for Camino build bustage (some includes aren't necessary for Camino).
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•