Closed
Bug 651845
Opened 14 years ago
Closed 14 years ago
Remove --enable/disable-static-mail
Categories
(MailNews Core :: Build Config, defect)
MailNews Core
Build Config
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 6.0
People
(Reporter: standard8, Assigned: standard8)
Details
Attachments
(1 file)
141.63 KB,
patch
|
Callek
:
review+
|
Details | Diff | Splinter Review |
As discussed on mdat, I want to remove the --enable/disable-static-mail option from mailnews. The reasons for supporting it are no longer significant, and having to keep maintaining two sets of module registrations is not useful.
I also believe that we should be able to reduce some of the complexity around our current mailnews/base/util library and just generally make mailnews build config simpler.
Assignee | ||
Comment 1•14 years ago
|
||
This does the necessary to remove the shared configuration. There's quite a few removals of files in here, which should make our makefiles a lot simpler.
(From the discussion on mdat, I saw no significant objections to doing this, and this simplifies the configuration which makes other build config changes simpler).
Assignee: nobody → mbanner
Status: NEW → ASSIGNED
Attachment #529346 -
Flags: review?(bugspam.Callek)
Assignee | ||
Comment 2•14 years ago
|
||
This passed on try server btw.
Comment 3•14 years ago
|
||
Comment on attachment 529346 [details] [diff] [review]
The fix
>diff --git a/mailnews/base/util/Makefile.in b/mailnews/base/util/Makefile.in
>--- a/mailnews/base/util/Makefile.in
>+++ b/mailnews/base/util/Makefile.in
>@@ -43,14 +43,7 @@ VPATH = @srcdir@
> include $(DEPTH)/config/autoconf.mk
>
> MODULE = msgbaseutil
>-
>-ifndef MOZ_STATIC_MAIL_BUILD
>-LIBRARY_NAME = msgbaseutil
>-EXPORT_LIBRARY = 1
>-SHORT_LIBNAME = msgbsutl
>-else
> LIBRARY_NAME = msgbsutl_s
>-endif
nit, I actually don't see a need to have msgbsutl_s anymore, and should go with msbsutil (since the _s was added as a "static" delim)
Overall this looks good, my one nit, which would require changes outside of where I applied the nit, so I don't mind if you defer that to a followup bug. I would also appreciate on addition run through try (c-c+m-c, or c-c+m-a) before this lands proper.
Attachment #529346 -
Flags: review?(bugspam.Callek) → review+
Assignee | ||
Comment 4•14 years ago
|
||
(In reply to comment #3)
> nit, I actually don't see a need to have msgbsutl_s anymore, and should go
> with msbsutil (since the _s was added as a "static" delim)
I disagree, it is still a static lib (and it would still get included in both mailnews & import if they're built separately). So I don't think this is necessary, but if you really think it is a necessary change, then we can do that in a follow-up bug.
I've now landed it: http://hg.mozilla.org/comm-central/rev/18fbc091e23f
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.4
Version: unspecified → Trunk
Comment 5•14 years ago
|
||
(In reply to comment #4)
> (In reply to comment #3)
> > nit, I actually don't see a need to have msgbsutl_s anymore, and should go
> > with msbsutil (since the _s was added as a "static" delim)
>
> I disagree,...
I don't feel that strongly, but if someone else notices this bug (who is at least a peer) and wants it, lets get said followup on file, otherwise no big deal.
Comment 6•14 years ago
|
||
Does this mean that we don't need NS_MSG_BASE any more, since all the mail modules are always linked into the same DSO (either libxul or, for highly experimental xulrunner builds, libmail)?
Assignee | ||
Comment 7•14 years ago
|
||
(In reply to comment #6)
> Does this mean that we don't need NS_MSG_BASE any more, since all the mail
> modules are always linked into the same DSO (either libxul or, for highly
> experimental xulrunner builds, libmail)?
Not quite, I think we'd need to move libimport into libmail for us to be able to do that (which IMO, I think we should consider doing).
Assignee | ||
Comment 8•14 years ago
|
||
I pushed a follow-up to remove the redundant Makefile listings in mailnews/makefiles.sh: http://hg.mozilla.org/comm-central/rev/622605d68e3f
You need to log in
before you can comment on or make changes to this bug.
Description
•