329021 - Get non-static builds of mailnews working when MOZ_XUL_APP set

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Mark Banner (:standard8)]

As per the summary. SeaMonkey needs non-static builds of mailnews (i.e. those that don't define MOZ_STATIC_MAIL_BUILD) working when MOZ_XUL_APP set.

The changes shouldn't affect Thunderbird (though some MOZ_THUNDERBIRD defs may change to MOZ_XUL_APP).

Comment 1

[Mark Banner (:standard8)]

Attachment: Main build changes for mailnews

This patch has the initial changes for non-static builds of mailnews. Basically we're defining the initialisation routines for the command line handlers using the toolkit version rather than the old xpfe versions.

Comments welcome, especially about where to put the static functions.

Comment 2

[Mark Banner (:standard8)]

Comment on attachment 213659 [details] [diff] [review]
Main build changes for mailnews

Ok, looks like this part will be needed in early MOZ_XUL_APP enabling, so requesting review.

Comment 3

[[e-mail]]

Comment on attachment 213659 [details] [diff] [review]
Main build changes for mailnews

How does the toolkit command line handler work? Might it be easier to #ifdef MOZ_XUL_APP nsICmdLineHandler.idl to generate the right code?

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 213659 [details] [diff] [review]
Main build changes for mailnews

This has brought up various issues with respect to startup and command lines that we need to resolve before we do this patch. Also there may be a better way to do it.

Comment 5

[Mark Banner (:standard8)]

The current idea for this bug is to turn on the MOZ_STATIC_MAIL_BUILD flag for mailnews (for SeaMonkey) when we turn on the MOZ_XUL_APP flag. This will mean we don't have to do code changes and allow SeaMonkey to pick up some of the advantages of static mailnews builds (faster execution for one).

Mnyromyr was commenting on irc that it would be good to leave MOZ_STATIC_MAIL_BUILD OFF for debug builds, to make them faster to build. I'm going to leave that open for debate, as with a fast-ish computer, I don't mind either way, but would prefer not to do the extra work at the moment for it.

Comment 6

[Benjamin Smedberg]

We should get rid of the nonstatic mail build completely: it is not a good idea to have to maintain two sets of component registration manifests, and it allows signifucant simplification in terms of linkage.

Comment 7

[Justin Wood (:Callek)]

overall I think allowing the non-static way is a *good* thing for faster build times on any type that wants it. (eg. Debug).

I currently --disable-static for my *personal* opt builds for that reason, makes dep builds much faster to build.

I will be willing to 'accept' either option chosen of course, (mainly as I do not know enough to actually do the work involved)

Comment 8

[Mark Banner (:standard8)]

(In reply to comment #7)
> I will be willing to 'accept' either option chosen of course, (mainly as I do
> not know enough to actually do the work involved)

I'm not planning to do any work on this myself, so reassigning to default owner. The work that would be required would be to examine nsMailModule.cpp (used for static builds and other files used for non-static builds (e.g. nsAbFactory.cpp) and implement the required changes - mainly the command line handling code I believe.

Comment 9

[Benjamin Smedberg]

I would very much like to only have the static-mail build in all configurations, to reduce the maintenance headache of the mailutils exports and other crap.

Comment 10

[[e-mail]]

Attachment: Fix compile and link errors

Obviously you also have to delete MOZ_STATIC_MAIL_BUILD from suite/confvars.sh

Although this builds I'm not guaranteeing that it runs but it's a start :-)

Comment 11

[Mark Banner (:standard8)]

Attachment: [checked in] Add nsAbLDAPDirectoryQuery to AB components

Neil commented that with his patch it would build but may not run correctly... I've been playing around and I've just found that nsAbLDAPDirectoryQuery is missing from nsAbFactory.

Here's the fix.

Comment 12

[[e-mail]]

Comment on attachment 294736 [details] [diff] [review]
[checked in] Add nsAbLDAPDirectoryQuery to AB components

So the rest of attachment 275769 [details] [diff] [review] is OK then?

Comment 13

[Mark Banner (:standard8)]

(In reply to comment #12)
> (From update of attachment 294736 [details] [diff] [review])
> So the rest of attachment 275769 [details] [diff] [review] is OK then?

I have only really tested the address book parts and the mailnews/base/util parts. Note that the diff for addrbook/build/nsAbFactory.cpp is now obsolete. The rest of it looks good though I haven't actually tested it.

Comment 14

[Mark Banner (:standard8)]

(In reply to comment #13)
> (In reply to comment #12)
> > (From update of attachment 294736 [details] [diff] [review] [details])
> > So the rest of attachment 275769 [details] [diff] [review] [details] is OK then?
> 
> I have only really tested the address book parts and the mailnews/base/util
> parts. Note that the diff for addrbook/build/nsAbFactory.cpp is now obsolete.
> The rest of it looks good though I haven't actually tested it.

I've just found that ./seamonkey -addressbook doesn't work. We're missing something like: http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/mailnews/build/nsMailModule.cpp&rev=1.48&mark=608-653#608 for the individual libs.

Comment 15

[[e-mail]]

Attachment: Add nsMsgShutdownService to base components

This fixes the startup error you were seeing; the shutdown service is a new component that landed after attachment 275769 [details] [diff] [review] was generated.

Comment 16

[Karsten Düsterloh]

Comment on attachment 275769 [details] [diff] [review]
Fix compile and link errors

I toyed around with this a bit and basically it looks good.

There are just two issues which need to be solved in followup patches:
- the commandline handlers "-mail", "-news", "-compose" and "-addressbook" don't work
- confvars.sh sets static as the default, but we only have --enable-static-mail, so users can't override that in their mozconfig easily. We either should change confvars.sh and do dynamic by default or replace/complement --enable-static-mail.

Comment 17

[Robert Kaiser]

I don't think we should have non-static by default, but it might be a good idea to make static mail set as a default and dynamic as a default when debug is enabled - or set static as default and --disable-static-mail the config option.

Comment 18

[[e-mail]]

In theory once this patch lands --disable-static-mail should just work.

Comment 19

[[e-mail]]

Attachment: Implement command-line handlers

Note: This was deliberately diffed against the previous versions in order to make it clearer what's going on. I'll attach an up-to-date patch for review.

Comment 20

[[e-mail]]

Attachment: Implement command-line handlers

The reordering of includes is to match nsMailModule.cpp (Sorry, but I didn't think to check against it when I added the include previously...)

Comment 21

[Mark Banner (:standard8)]

Comment on attachment 297431 [details] [diff] [review]
Implement command-line handlers

r=me, though I haven't fully tested it, it all looks ok.

Comment 22

[jag (Peter Annema)]

Attachment: Link in macmorefiles

This should fix non-static mail builds.

Comment 23

[Mark Banner (:standard8)]

(In reply to comment #22)
> Created an attachment (id=301880) [details]
> Link in macmorefiles
> This should fix non-static mail builds.

So why aren't these included in mailnews/build/Makefile.in if they are required for compose?

Comment 24

[[e-mail]]

Comment on attachment 301880 [details] [diff] [review]
Link in macmorefiles

I guess it might be more accurate to say r=Mnyromyr sr=me ;-)

Standard8, feel free to chime in with better ideas!

Comment 25

[jag (Peter Annema)]

Checking in Makefile.in;
/cvsroot/mozilla/mailnews/compose/build/Makefile.in,v  <--  Makefile.in
new revision: 1.56; previous revision: 1.55
done

Comment 26

[Mark Banner (:standard8)]

(In reply to comment #24)
> (From update of attachment 301880 [details] [diff] [review])
> I guess it might be more accurate to say r=Mnyromyr sr=me ;-)
> Standard8, feel free to chime in with better ideas!

*shrug*

Comment 27

[[e-mail]]

(In reply to comment #26)
>*shrug*