Last Comment Bug 394502 - make SeaMonkey build with libxul
: make SeaMonkey build with libxul
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Build Config (show other bugs)
: Trunk
: All All
: -- normal (vote)
: ---
Assigned To: Mark Banner (:standard8)
:
Mentors:
Depends on: 381157 382187 389070 390025 397277 450781 531292 593355 597465 599809
Blocks: 598646 394470 451923 534689 545172 SM-OOPP 585947 588067 require-libxul 598546 598613 598644
  Show dependency treegraph
 
Reported: 2007-08-31 15:20 PDT by Robert Kaiser (not working on stability any more)
Modified: 2014-04-26 02:22 PDT (History)
31 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Allows some SM components to build with --enable-libxul if options are given as well. (1.16 KB, patch)
2007-09-23 13:36 PDT, Mark Banner (:standard8)
no flags Details | Diff | Review
Hooks in mozilla-central for adding components to libxul (3.17 KB, patch)
2010-09-01 20:00 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
no flags Details | Diff | Review
Glue Suite internal linkage comps into libxul (9.69 KB, patch)
2010-09-01 20:04 PDT, Kyle Huey [:khuey] (khuey@mozilla.com)
no flags Details | Diff | Review
Hooks in mozilla-central for adding components to libxul v2 (3.31 KB, patch)
2010-09-04 10:53 PDT, Mark Banner (:standard8)
no flags Details | Diff | Review
comm-central changes (13.79 KB, patch)
2010-09-04 11:08 PDT, Mark Banner (:standard8)
no flags Details | Diff | Review
Hooks in mozilla-central for adding components to libxul v3 (19.53 KB, patch)
2010-09-08 13:18 PDT, Mark Banner (:standard8)
no flags Details | Diff | Review
Hooks in mozilla-central for adding components to libxul v4 (28.34 KB, patch)
2010-09-17 12:05 PDT, Mark Banner (:standard8)
no flags Details | Diff | Review
Possible Fix (30.09 KB, patch)
2010-09-18 12:16 PDT, Mark Banner (:standard8)
no flags Details | Diff | Review
The fix (40.98 KB, patch)
2010-09-20 05:36 PDT, Mark Banner (:standard8)
kairo: review+
bugspam.Callek: feedback+
Details | Diff | Review
The fix v2 (40.04 KB, patch)
2010-09-21 13:59 PDT, Mark Banner (:standard8)
no flags Details | Diff | Review
The fix v2a [Checked in: See comment 31+40] (39.92 KB, patch)
2010-09-21 14:13 PDT, Mark Banner (:standard8)
no flags Details | Diff | Review

Description Robert Kaiser (not working on stability any more) 2007-08-31 15:20:12 PDT
We want to get at a stage where we can build SeaMonkey with libxul like Firefox already does.
For this, we need to get rid of using non-frozen linkage, from what I know.

This bug acts as a tracker until it makes sense to do the real switchover, which can also happen here. Adding several bugs that I know or think are currently prohibiting this.
Comment 1 Mark Banner (:standard8) 2007-09-22 14:27:57 PDT
We also need places as toolkit/library requires all components that can normally be built in toolkit/components to be built... (unless we find an interesting way of doing ifdefs ;-) )
Comment 2 Mark Banner (:standard8) 2007-09-23 13:36:32 PDT
Created attachment 282033 [details] [diff] [review]
Allows some SM components to build with --enable-libxul if options are given as well.

I don't know if this will be useful in the future for anyone who works on this, however, as I've done it, I may as well document it just in case.

To build a reduced version of SeaMonkey with --enable-libxul, apply the patch I'm attaching now, and the one from bug 397277 (unless that is already fixed, of course). Then in your .mozconfig add the lines:

# for libxul
ac_add_options --enable-libxul
ac_add_options --disable-mailnews
ac_add_options --enable-places
ac_add_options --enable-extensions=default,-wallet,-typeaheadfind

All the disable/enables here are covered by dependent bugs. The patch may not be the correct/complete fix - I'm fairly sure that the ldap xpcom sdk won't be compiled into libxul currently.

The autocomplete changes would probably also be dependent on bug 263042 being fixed, but may be acceptable as a short term get us running on libxul fix.

Note: the libxul that this produces won't be the same as xulrunner/firefox due mainly to the differences in xpfe/components/
Comment 3 Tony Mechelynck [:tonymec] 2008-05-08 23:44:58 PDT
No comments in more than seven months. Robert, Mark, is this bug still relevant (and not yet fixed) for Sm-trunk = suiterunner?
Comment 4 Mark Banner (:standard8) 2008-05-09 00:16:47 PDT
(In reply to comment #3)
> No comments in more than seven months. Robert, Mark, is this bug still relevant
> (and not yet fixed) for Sm-trunk = suiterunner?
> 
Oh, look, there's 5 open bugs that block this one, I wonder why this hasn't had any activity.
Comment 5 Kyle Huey [:khuey] (khuey@mozilla.com) 2010-09-01 20:00:33 PDT
Created attachment 471383 [details] [diff] [review]
Hooks in mozilla-central for adding components to libxul

This was originally written by ted in bug 562313 for Firefox.  It got the stamps it needs to land there, though I made some minor changes.

This provides a hook to insert arbitrary static libs and components into libxul and arbitrary directories into tier_platform.
Comment 6 Kyle Huey [:khuey] (khuey@mozilla.com) 2010-09-01 20:04:51 PDT
Created attachment 471384 [details] [diff] [review]
Glue Suite internal linkage comps into libxul

This uses the previously provided hooks to insert mailnews, etc into libxul.  With this patch I can build a seemingly working Seamonkey build if I disable LDAP.  LDAP is hard because of its antiquated build system.
Comment 7 Kyle Huey [:khuey] (khuey@mozilla.com) 2010-09-01 20:08:49 PDT
To expand slightly on the last, if we can convince the pure c parts of LDAP to form a static lib it should be easy.  Alternatively we could probably link LDAP's shared libs to libxul, that would be slightly messier, but it should be doable.
Comment 8 Kyle Huey [:khuey] (khuey@mozilla.com) 2010-09-01 20:26:15 PDT
The build is a little crashy, not sure if that's from LDAP missing or something with the patches.
Comment 9 Serge Gautherie (:sgautherie) 2010-09-02 00:05:43 PDT
(In reply to comment #5)
> This was originally written by ted in bug 562313 for Firefox.

Interesting !
(Though I noticed bug 562313 comment 12 ... but which seems to solve that bug issue only.)

Iiuc, these 2 patches might help people working on c-c libxul bugs, and maybe Thunderbird (Bug 306324 comment 10),
but may not be what SeaMonkey would want w.r.t. bug 451923 (as it stands).

(In reply to comment #6)
> LDAP is hard because of its antiquated build system.

Good news is bug 590494 (work) :-)
Comment 10 Robert Kaiser (not working on stability any more) 2010-09-02 05:45:38 PDT
(In reply to comment #9)
> but may not be what SeaMonkey would want w.r.t. bug 451923 (as it stands).

Building on top of XULRunner is not an important goal right now, while building with libxul enabled in some way is, so just ignore that other bug for the moment while we're working on this one here.
Comment 11 Mark Banner (:standard8) 2010-09-04 10:53:41 PDT
Created attachment 472181 [details] [diff] [review]
Hooks in mozilla-central for adding components to libxul v2

Updated and slightly simplified core patch. Still needs something to deal with LDAP.
Comment 12 Mark Banner (:standard8) 2010-09-04 11:08:56 PDT
Created attachment 472184 [details] [diff] [review]
comm-central changes

This is WIP comm-central changes, mainly been focussing on mailnews and Thunderbird though with this patch. Still to address:

- LDAP
- Are we packaging everything that we should be.
- Sync suite/ changes with mail/ changes.
- Sort out if it is sensible to export symbols to let the two imap cpp tests to build.
- Simplify build and make sure we're not too broken for the --disable-libxul or libxul + separate mailnews libs configurations.

I have a build on try server with the current versions of these two patches. I believe Thunderbird builds and mainly works with these patches, but that'll confirm it and hopefully we'll get some mozmill runs if the packaging is not too wrong.
Comment 13 Robert Kaiser (not working on stability any more) 2010-09-04 12:02:53 PDT
Comment on attachment 472184 [details] [diff] [review]
comm-central changes

>diff --git a/bridge/bridge.mk b/bridge/bridge.mk
>+ifdef MOZ_CALENDAR
>+APP_LIBXUL_DIRS += $(DEPTH)$(SUBDIR)/calendar/lightning
>+endif

Why need calendar here? Lightning should completely be an add-on and not linked into libxul, at least not at this moment.


>diff --git a/bridge/confvars.sh b/bridge/confvars.sh

I'm not yet sure why this file is needed at all and why those very are not just set in the app-specific confvars.sh files, but...

>+MOZ_APP_COMPONENT_LIBS="mail msgsmime import"
>+MOZ_APP_COMPONENT_INCLUDE="MODULE(xpAutoComplete) MODULE(nsMailModule) MODULE(nsMsgSMIMEModule) MODULE(nsImportServiceModule)"

...including mail/-specific stuff here won't work for suite.

>diff --git a/mail/build.mk b/mail/build.mk
>-tier_app_dirs += xpfe/components/autocomplete
>-

Is this directly related? You don't need this version any more? If so, how does suite need to deal with that (we need at least the XBL from there)?

>-ifdef MOZ_CALENDAR
>-tier_app_dirs += calendar/lightning
>-endif

I'd think this should stay here (see above).

> tier_app_dirs += \
> 	mail \
>+	editor/ui \
> 	$(NULL)

So you can't undef MOZ_COMPOSER any more? Should this really be a part of this bug?

>diff --git a/mail/confvars.sh b/mail/confvars.sh
>+MOZ_APP_COMPONENT_LIBS="mail msgsmime import xpautocomplete mailcomps"
>+MOZ_APP_COMPONENT_INCLUDE=nsMailComponents.h

Hmm, you're inconsistent about those. Here you define them directly (which I think i prefer), for suite you include the bridge/ version.

>diff --git a/mail/installer/package-manifest.in b/mail/installer/package-manifest.in

You'll probably be able to clean this up, and will need to add some stuff to removed-files, but that can be dealt with once everything builds and runs.

>diff --git a/suite/build.mk b/suite/build.mk
>-tier_app_dirs += xpfe/components/autocomplete
>-

Umm, that takes away the autocomplete XBL we depend on in urlbar autocomplete, AFAIK. It should work with the binary code from toolkit, if LDAP does, but the XBL is probably needed on our side, so we need to copy that over to comm-central and include it from there (which has been our plan for some time for the point that LDAP autocomplete works with the toolkit impl).

>-ifdef MOZ_COMPOSER
>-tier_app_dirs += editor/ui
>-endif

Umm, I don't think you can just take that away from us completely!

>-tier_app_dirs += $(MOZ_BRANDING_DIRECTORY)

Umm, what about that one?

>-ifdef MOZ_CALENDAR
>-tier_app_dirs += calendar/lightning
>-endif

See above.

>diff --git a/suite/confvars.sh b/suite/confvars.sh
>+MOZ_IPC=

No, we want to kill this line as soon as we're building libxul. We only disable it because it breaks without libxul.

>+if [ "$COMM_BUILD" ]; then
>+    source ${_topsrcdir}/bridge/confvars.sh
>+else
>+    source ${_topsrcdir}/../bridge/confvars.sh
>+fi

See above, we shouldn't be inconsistent with mail/confvars.sh in this one.
Comment 14 Robert Kaiser (not working on stability any more) 2010-09-04 12:04:42 PDT
(In reply to comment #13)
> >+MOZ_APP_COMPONENT_LIBS="mail msgsmime import"
> >+MOZ_APP_COMPONENT_INCLUDE="MODULE(xpAutoComplete) MODULE(nsMailModule) MODULE(nsMsgSMIMEModule) MODULE(nsImportServiceModule)"
> 
> ...including mail/-specific stuff here won't work for suite.

Oh, wait, this "mail" is actually the mailnews "library", nothing mail/-specific.
Comment 15 Mark Banner (:standard8) 2010-09-04 12:34:23 PDT
As I tried to imply in comment 12 this isn't review-ready yet. So I'm not going to respond to the comments especially as you've missed the one about me still needing to sync suite to mail.

However thanks for the comments, as they have pointed out a few things that I can fix.
Comment 16 Robert Kaiser (not working on stability any more) 2010-09-04 12:42:31 PDT
(In reply to comment #15)
> However thanks for the comments, as they have pointed out a few things that I
> can fix.

That was all I intended, actually, I just tried to point out everything I saw so you can address those before requesting a formal review. :)
Comment 17 Mark Banner (:standard8) 2010-09-08 13:18:48 PDT
Created attachment 473192 [details] [diff] [review]
Hooks in mozilla-central for adding components to libxul v3

Still not ready but moved on a bit:

- Supports building of LDAP libraries, and including the xpcom part of those in libxul.
- Attempts to sync suite/ changes with the mail/ changes, though these aren't tested.

Current issues:

- Shared builds are currently broken - this needs an additional switch somewhere. I want to support for now until we get the quicker linking on mac at least.
- Can't disable LDAP with this patch.
Comment 18 Mark Banner (:standard8) 2010-09-17 12:05:07 PDT
Created attachment 476332 [details] [diff] [review]
Hooks in mozilla-central for adding components to libxul v4

Updated patch. Outstanding issues:

- I think --disable-ldap may be broken, but there again it may work if you don't mind it building the ldap components anyway. I'm tempted to leave it like this for now, as the UI when disabled is not good anyway.
- Windows builds fail with export/import error. I think this just needs some extra tweaks to what I've done in msgCore.h, just not had time to investigate yet.
- suite/ stuff is just copied not tested. I suspect a lot of what may need to be done there is to make it compile with internal API, or keep it external API (I can't remember which it is at the mo).
- Still testing the packaging changes for mail/.

This patch also adds back in support for fully shared builds, and removes the option for static builds.

I think the fully shared builds option breaks building in "classic" libxul style, but I'm thinking that we can get this going again once we've got some libxul support into the tree.
Comment 19 Ian Neal 2010-09-17 13:36:40 PDT
Would the export/import issues be covered by either of bug 582195 or bug 591098?
Comment 20 Mark Banner (:standard8) 2010-09-17 13:41:15 PDT
(In reply to comment #19)
> Would the export/import issues be covered by either of bug 582195 or bug
> 591098?

No. Like I said, I'm pretty sure its the tweaks in msgCore.h, and I'm also fairly sure it'll be simple to solve when I next sit down and think about it for a few minutes.
Comment 21 Mark Banner (:standard8) 2010-09-17 13:51:49 PDT
(I now have something on try to test a fix for it).
Comment 22 Mark Banner (:standard8) 2010-09-18 06:41:09 PDT
I've now fixed the Thunderbird windows builds, although its reminded me that we won't have the 3 c++ unit tests for mailnews for now, and we'll probably break binary extensions to begin with (expect we'll need to export functions - but we can use this as an opportunity to determine which ones are actually needed).

I've fixed one issue with the suite/ set-up, I currently have builds re-running to see how far it gets now.
Comment 23 Mark Banner (:standard8) 2010-09-18 12:16:37 PDT
Created attachment 476561 [details] [diff] [review]
Possible Fix

I believe this should now work for libxul and shared configurations of Thunderbird and libxul configuration of SeaMonkey - I've got the SeaMonkey configuration running on try. I've not tested the shared configuration yet, I also need to figure out the correct packaging changes (try has no package-compare at the moment, I've got a request in to get it back).

It needs the patch on bug 597465 applied to mozilla/ to work.

Note that this patch keeps suite's compiled code in suite/ outside of libxul - it has already been converted to external API and it was easier to do that separately than to put in conditionals for switching back to internal API (this could be done later if desired, but for now gets us going). SeaMonkey's mailnews and related libraries will still be linked inside of libxul with this patch.
Comment 24 Mark Banner (:standard8) 2010-09-20 05:36:07 PDT
Created attachment 476776 [details] [diff] [review]
The fix

This one updates the packaging files so that we hopefully get everything in place for being able to land this.

Note that SeaMonkey will need further clean-ups to its packaging files, but I've agreed with Robert that this can be done afterwards. The current changes for the SM packaging files should be enough that SM will run correctly after landing the patches.

I think I've tested this as far as I sensibly can, time to get it out for review and (hopefully) landing. Requesting review from Justin and Robert, I'll take whichever comes first ;-)
Comment 25 Justin Wood (:Callek) 2010-09-21 09:13:54 PDT
Comment on attachment 476776 [details] [diff] [review]
The fix

Just some drive-by nits for now:

>+++ b/mail/components/build/nsMailComponents.h
>+++ b/suite/components/nsSuiteComponents.h

To avoid the "SHOOT US IN THE FOOT" feeling can you at least add a comment about needing to sync shared stuff between these two apps? (I was tempted to ask for a shared header, but then I remembered all the places that would also need updating too)

>diff --git a/configure.in b/configure.in
>+dnl XXX Temporarily disabled for now until we have finialised how we'll link
>+dnl with libxul in the future.
>+dnl MOZ_ARG_ENABLE_BOOL(static,
>+dnl [  --enable-static         Enable building of internal static libs],
>+dnl    BUILD_STATIC_LIBS=1,
>+dnl    BUILD_STATIC_LIBS=)

I'd much rather we stuff |MOZ_STATIC_BUILD_UNSUPPORTED| into our app-config.mk's rather than commenting out all this. Unless we truely do want to remove it.

a/mailnews/base/test/TestMsgStripRE.cpp

Why this files change (include nsMemory) when we don't even build it if we enableLibxul and thats all this patch does?

mailnews/imap/test/Makefile.in

nite: wrap it in ONLY ONE ifndef instead of splitting it, and use the same comment as the mailnews/base/test case.



Ok all that said, this looks relatively good. I want to test it before I officially stamp it, and I am ok with SeaMonkey deferring packaging changes until after this lands, please file a bug, and beware of packaging bitrot with Bug 575894
Comment 26 Robert Kaiser (not working on stability any more) 2010-09-21 13:08:27 PDT
Comment on attachment 476776 [details] [diff] [review]
The fix

>diff --git a/configure.in b/configure.in
>+dnl XXX Temporarily disabled for now until we have finialised how we'll link
>+dnl with libxul in the future.
>+dnl MOZ_ARG_ENABLE_BOOL(static,
>+dnl [  --enable-static         Enable building of internal static libs],
>+dnl    BUILD_STATIC_LIBS=1,
>+dnl    BUILD_STATIC_LIBS=)
> 
> [...]
> 
>-if test -n "$MOZ_STATIC_BUILD_UNSUPPORTED" -a -n "$BUILD_STATIC_LIBS"; then
>-	AC_MSG_ERROR([--enable-static is not supported for building $MOZ_APP_NAME. You probably want --enable-libxul.])
>-fi
>-
>-if test -n "$MOZ_ENABLE_LIBXUL" -a -n "$BUILD_STATIC_LIBS"; then
>-	AC_MSG_ERROR([--enable-libxul is not compatible with --enable-static])
>-fi

I don't agree with that. For now, we should set MOZ_STATIC_BUILD_UNSUPPORTED and leave those as they are, erroring out with it. We still can go back and changhe that in a patch that enables static libxul if that ever comes along. Until then, we really should error out with static though.


>diff --git a/mail/Makefile.in b/mail/Makefile.in
>+ifndef MOZ_ENABLE_LIBXUL
>+DIRS += components
>+endif

Please add a comment why this is only built (here) for non-libxul and where it's invoked else.

>+ifndef MOZ_ENABLE_LIBXUL
> ifndef MOZ_INCOMPLETE_EXTERNAL_LINKAGE

This reminds me, from where we are, should we file a followup to rename MOZ_INCOMPLETE_EXTERNAL_LINKAGE to something more sensical?

>diff --git a/mailnews/base/test/TestMsgStripRE.cpp b/mailnews/base/test/TestMsgStripRE.cpp

As Callek said, you are excluding this anyhow here, so no need to patch it, let's move that into a followup for making those tests work again.

>diff --git a/suite/confvars.sh b/suite/confvars.sh
>+MOZ_IPC=

I need to do another build to verifiy it builds and works with the browser side, but unless I say otherwise, please leave out this one. The only reason why have this disabled is because we are not building libxul right now. If you don't feel goo with this, then at least please leave the comment before it in there and we'll remove both according to that bug mentioned there, then.

>diff --git a/suite/installer/package-manifest.in b/suite/installer/package-manifest.in
>diff --git a/suite/installer/removed-files.in b/suite/installer/removed-files.in

Yes, we'll need a followup to deal with those more. Adding mozz to removed-files is probably overdoing it, as we probably don't have shared builds to update from anyhow. But then, this can all be dealt with in a followup.


In fast testing, both dist/bin and dist/seamonkey (i.e. post-packaging) builds work fine on Linux32 in a default profile for opening any windows, but unfortunately, once I created a mail profile (i.e. immediately after the wizard, when the account is being used the first time), it crashes. Same with existing profiles that already have accounts, just that here it's on browser startup, when we invoke biff for the account. I don't have symbols on that build, so can't tell more right now, but can possibly look into that tomorrow.

I'm bold and give this an r+ despite this, on the premise of addressing the comments above and with the hope of finding the crash before the checkin. Still, I'd rather have a round of builds with crash reports than no Windows nightlies, so I'm in favor of pushing hard to get this in.
Comment 27 Mark Banner (:standard8) 2010-09-21 13:41:11 PDT
(In reply to comment #26)
> >+ifndef MOZ_ENABLE_LIBXUL
> > ifndef MOZ_INCOMPLETE_EXTERNAL_LINKAGE
> 
> This reminds me, from where we are, should we file a followup to rename
> MOZ_INCOMPLETE_EXTERNAL_LINKAGE to something more sensical?

IMO its still incomplete. The follow-up we should have is for enabling "proper" libxul builds to work.

> >diff --git a/suite/confvars.sh b/suite/confvars.sh
> >+MOZ_IPC=
> 
> I need to do another build to verifiy it builds and works with the browser
> side, but unless I say otherwise, please leave out this one.

I wanted to do one step at a time, so that if there are problems, we're not looking through too many issues - doing it this way also means you can create at least one nightly between the two to have as comparison.
Comment 28 Mark Banner (:standard8) 2010-09-21 13:59:02 PDT
Created attachment 477270 [details] [diff] [review]
The fix v2

This patch addresses the review comments and also moves jsd/jsd3250 from the packaged files to the removed files. It probably wouldn't hurt, but just in case, I didn't want to leave the old libraries hanging around.
Comment 29 Mark Banner (:standard8) 2010-09-21 14:13:35 PDT
Created attachment 477280 [details] [diff] [review]
The fix v2a
[Checked in: See comment 31+40]

This one is better, in that it covers all the review comments I meant to cover.
Comment 30 Robert Kaiser (not working on stability any more) 2010-09-21 17:55:35 PDT
Here's a stack trace of the crash I'm seeing:

Program received signal SIGSEGV, Segmentation fault.
0xb759152d in nsMsgDatabase::GetDBFolderInfo (this=0xacea12f0, result=0xbfffc2f4) at /mnt/mozilla/hg/comm-central/mailnews/db/msgdb/src/nsMsgDatabase.cpp:1450
(gdb) bt
#0  0xb759152d in nsMsgDatabase::GetDBFolderInfo (this=0xacea12f0, result=0xbfffc2f4) at /mnt/mozilla/hg/comm-central/mailnews/db/msgdb/src/nsMsgDatabase.cpp:1450
#1  0xb75b698c in nsImapMailFolder::GetDBFolderInfoAndDB (this=0xac3ddc00, folderInfo=0xbfffc2f4, db=0xbfffc2f0) at /mnt/mozilla/hg/comm-central/mailnews/imap/src/nsImapMailFolder.cpp:2132
#2  0xb7499c7b in nsMsgDBFolder::ReadDBFolderInfo (this=0xac3ddc00, force=1) at /mnt/mozilla/hg/comm-central/mailnews/base/util/nsMsgDBFolder.cpp:661
#3  0xb7495537 in nsMsgDBFolder::UpdateSummaryTotals (this=0xac3ddc00, force=1) at /mnt/mozilla/hg/comm-central/mailnews/base/util/nsMsgDBFolder.cpp:3986
#4  0xb75b66b6 in nsImapMailFolder::UpdateSummaryTotals (this=0xac3ddc00, force=1) at /mnt/mozilla/hg/comm-central/mailnews/imap/src/nsImapMailFolder.cpp:1817
#5  0xb75bb2b8 in nsImapMailFolder::GetDatabase (this=0xac3ddc00) at /mnt/mozilla/hg/comm-central/mailnews/imap/src/nsImapMailFolder.cpp:663
#6  0xb75b695d in nsImapMailFolder::GetDBFolderInfoAndDB (this=0xac3ddc00, folderInfo=0xbfffc524, db=0xbfffc520) at /mnt/mozilla/hg/comm-central/mailnews/imap/src/nsImapMailFolder.cpp:2126
#7  0xb7499c7b in nsMsgDBFolder::ReadDBFolderInfo (this=0xac3ddc00, force=0) at /mnt/mozilla/hg/comm-central/mailnews/base/util/nsMsgDBFolder.cpp:661
#8  0xb7494dd3 in nsMsgDBFolder::GetFlags (this=0xac3ddc00, _retval=0xbfffc6c4) at /mnt/mozilla/hg/comm-central/mailnews/base/util/nsMsgDBFolder.cpp:1237
#9  0xb75b9681 in nsImapMailFolder::AddSubfolderWithPath (this=0xafab3c00, name=..., dbPath=0xab819700, child=0xbfffc974, brandNew=0) at /mnt/mozilla/hg/comm-central/mailnews/imap/src/nsImapMailFolder.cpp:431
#10 0xb75babda in nsImapMailFolder::CreateSubFolders (this=0xafab3c00, path=0xab819480) at /mnt/mozilla/hg/comm-central/mailnews/imap/src/nsImapMailFolder.cpp:570
#11 0xb75bae8a in nsImapMailFolder::GetSubFolders (this=0xbfffca88, aResult=0xb7495061) at /mnt/mozilla/hg/comm-central/mailnews/imap/src/nsImapMailFolder.cpp:614
#12 0xbfffca88 in ?? ()
Backtrace stopped: previous frame inner to this frame (corrupt stack?)



(In reply to comment #27)
> I wanted to do one step at a time, so that if there are problems, we're not
> looking through too many issues - doing it this way also means you can create
> at least one nightly between the two to have as comparison.

OK, I'm seeing that IPC work with SeaMonkey with your patch, but we need some packaging adjustment to make it work accurately (right now, plugin-container isn't packaged) so let's hand it off to a followup. I still don't even want a nightly shipping between landing this and IPC, though. :)
Comment 31 Mark Banner (:standard8) 2010-09-22 01:39:53 PDT
I took a look at the SeaMonkey crash this morning and as it crashed in similar places on my profile, but not only that it was crashing in the browser as well (probably due to the mail check being turned on), I decided that we're not quite ready to land the SM parts of this patch yet.

However, as I also want to verify the results of the builds for the Thunderbird parts, I have landed the mail/ and mailnews/ parts and updated Thunderbird's mozconfigs:

http://hg.mozilla.org/comm-central/rev/47de99b5e40a
http://hg.mozilla.org/build/buildbot-configs/rev/af3aee009254

Hopefully they will verify the try server and my local testing results. In the meantime I have rebuilds running to see if I can then resolve the SM crashing issue.
Comment 32 Robert Kaiser (not working on stability any more) 2010-09-22 03:21:39 PDT
The "SeaMonkey crash" is fully in mailnews, my stack is from launching browser, so that confirms that it's the mail biff check that invokes it, and as I said before, I prefer getting builds with crash reports to having Windows broken completely. So why is the SeaMonkey part not in despite that, and why isn't there at least a SeaMonkey patch then? This could have been the worst news you could give me this morning, really.
Comment 33 Mark Banner (:standard8) 2010-09-22 03:25:12 PDT
(In reply to comment #32)
> So why is the SeaMonkey part not in despite that, and why isn't
> there at least a SeaMonkey patch then?

My testing with a pre existing profile (one that I've had around for ages) showed that even just starting the browser would crash, resulting in a potential DoS for nightly users. IMO that is far worse than a broken Windows nightly.
Comment 34 Robert Kaiser (not working on stability any more) 2010-09-22 03:43:47 PDT
Nightly users know how to download a new one. Those 200-300 users are capable of that if necessary. Also, this landed later than our nightlies start, which means we at least would get better data on the crash from tests running. But now, I guess we are stuck with "it crashes but nobody cares because nobody sees it". And there couldn't be a worse thing to happen for our project. I wonder for what reason this even is a SeaMonkey bug here.
Comment 35 Mark Banner (:standard8) 2010-09-22 04:21:03 PDT
(In reply to comment #34)
> But
> now, I guess we are stuck with "it crashes but nobody cares because nobody sees
> it".

Oh ok, so I'll just dump the build that took 2 hours and stop the hour of SM specific debug that I've done already shall I? (as I said I would do in comment 31).
Comment 36 Mark Banner (:standard8) 2010-09-22 04:39:38 PDT
I've now found the source of the SeaMonkey crasher - the mork factory/components are not being registered.

It turns out the ifdefs for the libxul library mean that if places is enabled, mork isn't registered.

Normally I'd do a core patch, however due to the state of their tree we're probably going to find it hard to push that in quick.

I think I can work around it with some of the new options that we have, I've put some changes in locally and I'm now rebuilding to see if they will get mork registered and SeaMonkey running properly.
Comment 37 Robert Kaiser (not working on stability any more) 2010-09-22 05:26:47 PDT
(In reply to comment #35)
> Oh ok, so I'll just dump the build that took 2 hours and stop the hour of SM
> specific debug that I've done already shall I? (as I said I would do in comment
> 31).

Gah, sorry, I of course am blind and have not seen you telling us you are actually investigating this problem. I guess I am too used to people leaving us in the cold that I don't expect someone helping any more. Sorry again.

(In reply to comment #36)
> I've now found the source of the SeaMonkey crasher - the mork
> factory/components are not being registered.

Interesting. Why does this only affect SeaMonkey, I thought Thunderbird uses the same mork?
Comment 38 Serge Gautherie (:sgautherie) 2010-09-22 06:16:11 PDT
(In reply to comment #37)

> Gah, sorry, I of course am blind and have not seen you telling us you are
> actually investigating this problem. I guess I am too used to people leaving us
> in the cold that I don't expect someone helping any more. Sorry again.

(Yes, please do calm down. Afaict, Mark is doing a wonderful job here and at least the "Core" parts of his patches haven't regressed anything (I hope) on the SeaMonkey side ... which is at least somehow good to check/know as a 1st step.
Also, I would support enabling SM MOZ_IPC as a 3rd step only, but I won't argue.)

> Interesting. Why does this only affect SeaMonkey, I thought Thunderbird uses
> the same mork?

Let me try to guess:
{
http://mxr.mozilla.org/comm-central/find?text=&string=%2Fconfvars.sh

http://mxr.mozilla.org/comm-central/source/mail/confvars.sh
53 MOZ_PLACES=
54 MOZ_MORKREADER=
55 MOZ_MORK=1

http://mxr.mozilla.org/comm-central/source/suite/confvars.sh
56 MOZ_MORK=1
}
So iiuc TB does use the same Mork, but doesn't use Places.
Comment 39 Mark Banner (:standard8) 2010-09-22 06:17:45 PDT
(In reply to comment #37)
> (In reply to comment #35)
> > Oh ok, so I'll just dump the build that took 2 hours and stop the hour of SM
> > specific debug that I've done already shall I? (as I said I would do in comment
> > 31).
> 
> Gah, sorry, I of course am blind and have not seen you telling us you are
> actually investigating this problem. I guess I am too used to people leaving us
> in the cold that I don't expect someone helping any more. Sorry again.

Accepted.

> (In reply to comment #36)
> > I've now found the source of the SeaMonkey crasher - the mork
> > factory/components are not being registered.
> 
> Interesting. Why does this only affect SeaMonkey, I thought Thunderbird uses
> the same mork?

There's an incorrect assumption in the libxul code that having places turned on means you don't want mork. See bug 598613 for the better details. For now I've worked around it by specifying the missing declarations in MOZ_APP_COMPONENT_LIBS and in nsSuiteComponents.h (with comments), so we don't need another core change - we can get that bug fixed when we can and then remove them later.
Comment 40 Mark Banner (:standard8) 2010-09-22 07:31:28 PDT
SeaMonkey patch now landed with the crash fixups and mozconfig changes:

http://hg.mozilla.org/comm-central/rev/3c34cdb6938b
http://hg.mozilla.org/build/buildbot-configs/rev/f97808979395

A few fixups also landed for Thunderbird:

http://hg.mozilla.org/comm-central/rev/6740532dfccb (fixing timeouts on bloat/mozmill tests)
http://hg.mozilla.org/comm-central/rev/8388db079b3f (fixing Linux bustage)

Current indications from the Thunderbird tree are looking good - I believe the current bustage should now resolve itself, but the builds are mainly clobbers so they may be slow for a while.
Comment 41 Robert Kaiser (not working on stability any more) 2010-09-22 07:37:42 PDT
(In reply to comment #39)
> There's an incorrect assumption in the libxul code that having places turned on
> means you don't want mork.

"nice". Thanks for finding this.

(In reply to comment #40)
> SeaMonkey patch now landed with the crash fixups and mozconfig changes:

Ah, cool, I already thought I need to prepare the mozconfig changes as well. :)

I'm currently looking into clobbering all the builds, tracking that in bug 598546. Next step will be to figure out packaging and IPC.
Comment 42 Mark Banner (:standard8) 2010-09-22 14:25:34 PDT
I believe I've now filed all the follow-up bugs. From the indications on tinderbox this bug should now be fixed.

Please file any follow-ups in separate bugs.

Thunderbird tree will remain closed overnight as I want to let the tree stabilise a bit and track what intermittent oranges/bustages we have (I think they are the same, but just in case). I'll let Robert decide when to re-open SeaMonkey.

Note You need to log in before you can comment on or make changes to this bug.