Closed
Bug 329021
Opened 19 years ago
Closed 17 years ago
Get non-static builds of mailnews working when MOZ_XUL_APP set
Categories
(MailNews Core :: Backend, defect)
MailNews Core
Backend
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: standard8, Assigned: neil)
References
Details
Attachments
(7 files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
mnyromyr
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
neil
:
review+
neil
:
superreview+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
mnyromyr
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
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).
Reporter | ||
Comment 1•19 years ago
|
||
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.
Reporter | ||
Comment 2•19 years ago
|
||
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.
Attachment #213659 -
Flags: review?(mnyromyr)
Assignee | ||
Comment 3•19 years ago
|
||
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?
Reporter | ||
Comment 4•19 years ago
|
||
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.
Attachment #213659 -
Flags: review?(mnyromyr)
Reporter | ||
Comment 5•19 years ago
|
||
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•19 years ago
|
||
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•18 years ago
|
||
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)
Reporter | ||
Comment 8•18 years ago
|
||
(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.
Assignee: bugzilla → nobody
QA Contact: backend
Comment 9•18 years ago
|
||
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.
Assignee | ||
Comment 10•17 years ago
|
||
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 :-)
Attachment #275769 -
Flags: review?(mnyromyr)
Reporter | ||
Comment 11•17 years ago
|
||
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.
Attachment #294736 -
Flags: superreview?(neil)
Attachment #294736 -
Flags: review?(neil)
Assignee | ||
Comment 12•17 years ago
|
||
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?
Attachment #294736 -
Flags: superreview?(neil)
Attachment #294736 -
Flags: superreview+
Attachment #294736 -
Flags: review?(neil)
Attachment #294736 -
Flags: review+
Reporter | ||
Comment 13•17 years ago
|
||
(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.
Reporter | ||
Comment 14•17 years ago
|
||
(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.
Reporter | ||
Updated•17 years ago
|
Attachment #294736 -
Attachment description: Add nsAbLDAPDirectoryQuery to AB components → [checked in] Add nsAbLDAPDirectoryQuery to AB components
Assignee | ||
Comment 15•17 years ago
|
||
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.
Attachment #296766 -
Flags: review?(mnyromyr)
Updated•17 years ago
|
Attachment #296766 -
Flags: review?(mnyromyr) → review+
Comment 16•17 years ago
|
||
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.
Attachment #275769 -
Flags: review?(mnyromyr) → review+
Comment 17•17 years ago
|
||
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.
Assignee | ||
Comment 18•17 years ago
|
||
In theory once this patch lands --disable-static-mail should just work.
Assignee | ||
Comment 19•17 years ago
|
||
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.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Assignee | ||
Comment 20•17 years ago
|
||
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...)
Attachment #297431 -
Flags: review?(bugzilla)
Reporter | ||
Comment 21•17 years ago
|
||
Comment on attachment 297431 [details] [diff] [review]
Implement command-line handlers
r=me, though I haven't fully tested it, it all looks ok.
Attachment #297431 -
Flags: review?(bugzilla) → review+
Comment 22•17 years ago
|
||
This should fix non-static mail builds.
Updated•17 years ago
|
Attachment #301880 -
Flags: review?(neil)
Reporter | ||
Comment 23•17 years ago
|
||
(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?
Assignee | ||
Comment 24•17 years ago
|
||
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!
Attachment #301880 -
Flags: review?(neil) → review+
Comment 25•17 years ago
|
||
Checking in Makefile.in;
/cvsroot/mozilla/mailnews/compose/build/Makefile.in,v <-- Makefile.in
new revision: 1.56; previous revision: 1.55
done
Reporter | ||
Comment 26•17 years ago
|
||
(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*
Assignee | ||
Comment 27•17 years ago
|
||
(In reply to comment #26)
>*shrug*
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Updated•16 years ago
|
Product: Core → MailNews Core
You need to log in
before you can comment on or make changes to this bug.
Description
•