27948 - Profile manager doesn't compile in a --disable-mailnews build

Status: VERIFIED FIXED

(Core Graveyard :: Profile: BackEnd, defect, P3)

Description

[tor]

Your change 1.114 makes the profile manager rely on building with mailnews.
If a --disable-mailnews build is attempted, the following errors result:

"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 61: Error: Could not
open include file "nsIMsgAccountManager.h".
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 62: Error: Could not
open include file "nsMsgBaseCID.h".
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 63: Error: Could not
open include file "nsIMsgAccount.h".
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1450: Error: Type name
expected instead of "nsIMsgAccountManager".
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1450: Error: ","
expected instead of "nsIMsgAccountManager".
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1450: Error:
nsIMsgAccountManager is not defined.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1450: Error: Illegal
value for template parameter.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1450: Error: Too many
arguments for template nsCOMPtr.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1450: Error: Too many
arguments for template nsCOMPtr.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1450: Error: Too many
arguments for template nsCOMPtr.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1450: Error:
NS_MSGACCOUNTMANAGER_PROGID is not defined.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1454: Error: Type name
expected instead of "nsIMsgIdentity".
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1454: Error: ","
expected instead of "nsIMsgIdentity".
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1454: Error:
nsIMsgIdentity is not defined.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1454: Error: Illegal
value for template parameter.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1454: Error: Too many
arguments for template nsCOMPtr.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1455: Error: Pointer
type needed instead of nsCOMPtr<int, <unknown>>.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1459: Error: Pointer
type needed instead of nsCOMPtr<int, <unknown>>.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1464: Error: Pointer
type needed instead of nsCOMPtr<int, <unknown>>.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1468: Error: Type name
expected instead of "nsIMsgIncomingServer".
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1468: Error: ","
expected instead of "nsIMsgIncomingServer".
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1468: Error:
nsIMsgIncomingServer is not defined.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1468: Error: Illegal
value for template parameter.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1468: Error: Too many
arguments for template nsCOMPtr.
"/cs/src/mozilla/mozilla/profile/src/nsProfile.cpp", line 1469: Error: Pointer
type needed instead of nsCOMPtr<int, <unknown>>.
Compilation aborted, too many messages.

Comment 1

[racham]

seth,

What are the flags that enable mailnews piece to be compiled ?
I guess what's needed here is a conditional compilation.
Do we have these disable option for all the platforms ?

Comment 2

[(not reading, please use [e-mail] instead)]

I'll take this bug.

I don't think this is a blocker, but I can see why you'd want to fix it.

changing severity to major.

Comment 3

[racham]

thanks seth.

Comment 4

[racham]

Seth,

If you get to this bug and have your fix ready, will you please wait for gayatri 
to land her i18n changes. That's expected to happen end of the day tomorrow. 

thanks.

Comment 5

[(not reading, please use [e-mail] instead)]

marking m15, I'm not going to do this for beta.

Comment 6

[tor]

*** Bug 28302 has been marked as a duplicate of this bug. ***

Comment 7

[tor]

I don't think this should be pushed off until M15.  mozilla.org shouldn't
release a milestone tarball known broken with what I venture is a fairly
popular configuration option.  The following patch, while probably not the
cleanest solution, is a low risk temporary fix for --disable-mailnews:

Index: nsProfile.cpp
===================================================================
RCS file: /cvsroot/mozilla/profile/src/nsProfile.cpp,v
retrieving revision 1.115
diff -u -r1.115 nsProfile.cpp
--- nsProfile.cpp       2000/02/18 03:37:54     1.115
+++ nsProfile.cpp       2000/02/18 18:28:13
@@ -58,9 +58,12 @@
 #include "nsIModule.h"
 #include "nsIGenericFactory.h"
 #include "nsICookieService.h"
+
+#ifdef MOZ_MAIL_NEWS
 #include "nsIMsgAccountManager.h"
 #include "nsMsgBaseCID.h"
 #include "nsIMsgAccount.h"
+#endif
 
 #if defined (XP_UNIX)
 #elif defined (XP_MAC)
@@ -1454,6 +1457,7 @@
                                if ( (PL_strlen(serverName) == 0) ||
(PL_strlen(serverType) == 0) )
                                        return NS_ERROR_FAILURE;
 
+#ifdef MOZ_MAIL_NEWS
                                NS_WITH_SERVICE(nsIMsgAccountManager,
accountManager, NS_MSGACCOUNTMANAGER_PROGID, &rv);
                                NS_ENSURE_SUCCESS(rv, rv);
 
@@ -1494,7 +1498,7 @@
                                if (NS_FAILED(rv)) return rv;
                                rv = account->AddIdentity(identity);
                                if (NS_FAILED(rv)) return rv;
-
+#endif
                                CRTFREEIF(serverName);
                                CRTFREEIF(serverType);
                        }

Comment 8

[(not reading, please use [e-mail] instead)]

your patch looks fine.  does the rest of the tree build with disable-mailnews?

I've marked it beta1 and m14, and re-assigned it to you.

(I can check in for you if you get approval.)

Comment 9

[tor]

Yes, the rest of the tree builds fine with --disable-mailnews.

Comment 10

[tor]

Patch depends on MOZ_MAIL_NEWS being defined for an ordinary build, which
doesn't seem to be the case on Win32 or Mac.

Comment 11

[tor]

We have exchanged some email regarding this problem.  It seems that the
consensus is that --disable-mailnews builds should run the export phase
over mailnews to generate the necessary interface headers.  The code should
then check at runtime if mailnews is installed and do the appropriate thing.

Quote (">" is me, the rest is alecf):
-----------------------------------------------------
> Ok, I experimented with a --disable-mailnews build with a make
> phase in the mailnews directory and using nsProfile.cpp from the
> tip.  It compiled fine and seems to be functional.
> 
  
yes, that's the idea, and one of the primary advantages to XPCOM.

XPCOM is a little different than the traditional
link-in-all-modules-at-build-time in that it's possible, at runtime, to
detect the presence of a particular component, and act appropriately.  
basically if the NS_WITH_SERVICE you saw failed, then mail is probably not
installed... 

I happened to look at the code in nsProfile.cpp and it's NOT behaving
correctly if mail is not installed, but I have talked to the maintainer to
have him fix this.

...

due to the dynamic nature of XPCOM, this isn't a real "dependancy" unless
the code trying to use mail is written wrong. We're moving towards a
runtime dependancy model instead of build-time dependancy.. this does make
dependancies a little tricky, but I think it's worth it in the long run.

So the general rule that I'm trying to educate people on is this: mail
might not be there. Feel free to use our services and interfaces, but make
your code behave appropriately if our component is not there.
-----------------------------------------------------

Comment 12

[leger]

Putting on PDT- radar for beta1.

Comment 13

[cls]

Ugh. I think I understand why you would want to always export your interface to
the rest of the build but it could cause some complications in the future.  At
some point before the final release (hopefully in the next milestone), the
export & libs phases will be consolidated once the idl build order is figured
out (bug #17525).  When this happens, I guess we'll have to move the
MOZ_MAIL_NEWS ifdef into the mailnews Makefiles. Just something to be aware of.

Comment 14

[Christine Begle]

*** Bug 28599 has been marked as a duplicate of this bug. ***

Comment 15

[tor]

Reassigning bug to alecf since he seems to have strong feeling about which way
this should be fixed (see mozilla.unix).

Comment 16

[Mike Shaver (:shaver emeritus)]

I think we want to provide some more-generic way for things to get called from
the profile manager, because things other than mailnews might want to interact
with it, and see new profiles or whatever.

(I also wonder what fresh hell brought the Netcenter ureg stuff into the Mozilla
codebase, but that's a different bug.)

Comment 17

[Alec Flett]

I have a scheme for this, but it really needs to wait for beta1. Basically, it
involves taking registration information and creating a category of registration
handlers so that things like Mail and AIM can handle them in their own sweet
way.

Comment 18

[cls]

This really needs to be fixed before M14 releases. Since it can't be done
properly, here's the kludge: Move the Makefile ifdef MOZ_MAIL_NEWS logic from
the toplevel Makefile into the mailnews module's Makefiles.

Comment 19

[cls]

Attachment: Kludge to move MOZ_MAIL_NEWS ifdef

Comment 20

[Alec Flett]

This hack will work, but why does this need to be fixed before m14? Please do
not change target milestones if the bug is not assigned to you. It's just common
bugzilla courtesy.

Why can't we just release note this? --disable-editor, etc are also broken but I
don't see anyone complaining about that. it adds about 10% to the build time in
order to build mailnews.

Comment 21

[Alec Flett]

by the way, I have talked to racham about moving this stuff out of nsProfile,
and we WILL do it, I promise you that. However, it's not going to happen until
M15.

Comment 22

[cls]

Sorry about the milestone change.  The difference is that --disable-editor was
terminally broken due to a design change not just some random breakage.  It was
also removed sometime ago.  This is breakage occurred the day the tree was
closed for M14.  Considering that a number of people use this option, is there
any reason *not* to fix it before M14?  

Comment 23

[Alec Flett]

basically because backing out that change looks difficult, (and we will need to
back it out after M14) and it doesn't hurt anyone right now... the build works
fine with a simple gmake export in the mailnews directory.

How about a simpler change, were we do the export phase in mailnews if
--disable-mailnews is on?

Comment 24

[cls]

Attachment: Second kludge that only runs make export in mailnews

Comment 25

[Alec Flett]

this seems much more acceptable to me.. . Thanks for the effort chris. We'll
definately fix the general problem after M14/beta1.. r=alecf.

reassigning to chris so that he can check this in himself, and CC'ing leaf for
approval.

Comment 26

[cls]

Ok, the temp fix has been checked in.  Handing the bug back to alecf so he can
coordinate the real fix with racham.

Comment 27

[Alec Flett]

thanks chris!

Comment 28

[Mike Shaver (:shaver emeritus)]

What's the status here?  I am -very- tempted to just rip the ureg stuff out,
because it raises my blood pressure, and .security does a good enough job of
that as it is.

Is there any sound reason to keep it in there?  People who need to refer to it
for constructing the proprietary UREG dynamic profile loveliness (hi, alecf!)
can just pull from another version, right?

Comment 29

[Alec Flett]

Ok, I tell you what. Racham and I will have a plan and schedule for you by
monday, April 3rd. We will definately have this out by Netscape's beta2 initial
tree closure.
note that thanks to cls, this bug doesn't quite match the description, because
it now does compile w/--disable-mailnews

Comment 30

[Alec Flett]

this is really all bhuvan at this point
bhuvan - I'm staying on the CC, but you can mark this fixed when you move the
mail stuff out of the mozilla tree and into the ns tree.

Comment 31

[racham]

Mozilla profile manager is no more dependent on having some mailnews header 
files. PReg code has been moved to ns tree. Marking fixed.

Comment 32

[Grace Bush]

reviewed code fix