691133 - xpcom: AURORA_BASE_20110927 fails to build darwin/x11

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Jeremy Huddleston]

6dc468c41136 (20110820) built fine, but the AURORA_BASE_20110927 tag fails with various issues in xpcom (probably because someone assumed darwin == cocoa):

In file included from ./nsLocalFile.h:76:
./nsLocalFileUnix.h:57:10: fatal error: 'nsILocalFileMac.h' file not found
#include "nsILocalFileMac.h"
         ^

/opt/local/var/macports/build/_Volumes_Home_jeremy_src_macports_trunk_dports_www_firefox-x11-devel/firefox-x11-devel/work/mozilla-central/xpcom/io/nsAppFileLocationProvider.cpp:600:12: error: unknown type name 'SInt32'
    static SInt32 version = 0;
           ^
/opt/local/var/macports/build/_Volumes_Home_jeremy_src_macports_trunk_dports_www_firefox-x11-devel/firefox-x11-devel/work/mozilla-central/xpcom/io/nsAppFileLocationProvider.cpp:603:9: error: use of undeclared identifier 'OSErr'
        OSErr err = ::Gestalt(gestaltSystemVersion, &version);
        ^

Comment 1

[Jeremy Huddleston]

Attachment: build fix for xpcom

Comment 2

[Hanspeter Niederstrasser]

Attachment: xpcom patch for m-c

Comment 3

[Hanspeter Niederstrasser]

Comment on attachment 564544 [details] [diff] [review]
xpcom patch for m-c

grrr.  Anyway, here's the same patch modified to go cleanly against m-c.  Patching cleanly vs aurora got caught in the recent PRBool to bool transition.

I had previously tried to fix build failure this by changing xpcom/io/Makefile.in to generate nsILocalFileMac.h on Darwin (not just Cocoa), but that led me to a massive amount of patching then needed in the ipc area.  I'm not very clear in to what nsILocalFileMac.h ends up completely providing so I can't guess as to which method is better.  The patch below is several weeks old for reference.

diff --git a/xpcom/io/Makefile.in b/xpcom/io/Makefile.in
--- a/xpcom/io/Makefile.in
+++ b/xpcom/io/Makefile.in
@@ -152,19 +152,19 @@ XPIDLSRCS	= \
 		nsIIOUtil.idl \
 		nsISafeOutputStream.idl \
 		nsIScriptableBase64Encoder.idl \
 		$(NULL)
 
 ifeq ($(MOZ_WIDGET_TOOLKIT),os2)
 XPIDLSRCS	+= nsILocalFileOS2.idl
 else
-ifeq ($(MOZ_WIDGET_TOOLKIT),cocoa)
+ifeq ($(OS_ARCH),Darwin)
 XPIDLSRCS	+= nsILocalFileMac.idl
-endif # cocoa
+endif # Darwin
 endif # os2
 
 SDK_XPIDLSRCS   = \
 		nsIDirectoryService.idl \
 		nsIFile.idl \
 		nsILocalFile.idl \
 		nsIInputStream.idl \
 		nsIOutputStream.idl \

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 564034 [details] [diff] [review]
build fix for xpcom

As best I can tell this patch is obsoleted by your later patch.

Comment 5

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 564544 [details] [diff] [review]
xpcom patch for m-c

I can't figure out why you need this patch:  As best I can tell XP_MACOSX and MOZ_WIDGET_COCOA are synonyms -- I can't figure out how one would get set without the other.  I suspect you've made other changes to the Mozilla build environment that you haven't told us about.

That said, I haven't tried to build on Darwin (as opposed to OS X), so I can't be 100% sure this patch isn't needed.  And in any case I'm quite sure it's harmless.  So I'm willing to r+ it.

Comment 6

[Jeremy Huddleston]

MOZ_WIDGET_COCOA is for building against the Cocoa frameworks.

XP_MACOSX is for building on darwin (which can be Cocoa, GTK, ...)

Comment 7

[Steven Michaud [:smichaud] (Retired)]

> MOZ_WIDGET_COCOA is for building against the Cocoa frameworks.
>
> XP_MACOSX is for building on darwin (which can be Cocoa, GTK, ...)

OK, that makes sense as	a description of how these *should* be used.

But I still can't figure out how, without making any changes to
current trunk code, there could ever be a build environment in which
one of these would be set without the other being set.

Comment 8

[Steven Michaud [:smichaud] (Retired)]

Take a look at the top level configure.in (where both of these are set or not set) and see what you think.

Comment 9

[Hanspeter Niederstrasser]

--enable-default-toolkit=cairo-gtk2 on Darwin (or even normal OS X) sets XP_MACOSX but not MOZ_WIDGET_COCOA.  Picking cairo-qt would have the same effect.

Comment 10

[Steven Michaud [:smichaud] (Retired)]

OK, yes.  I was assuming that you weren't explicitly doing anything to change either of these settings.  And I (now) can see why someone might want to cairo-gtk2 or cairo-qt on either Darwin or OS X.

Comment 11

[Steven Michaud [:smichaud] (Retired)]

> XP_MACOSX is for building on darwin (which can be Cocoa, GTK, ...)

Actually, since there's also an XP_DARWIN that can be defined in configure.in, XP_MACOSX should probably be for building on "normal" OS X.

Comment 12

[Steven Michaud [:smichaud] (Retired)]

>> XP_MACOSX is for building on darwin (which can be Cocoa, GTK, ...)
>
> Actually, since there's also an XP_DARWIN that can be defined in
> configure.in, XP_MACOSX should probably be for building on "normal"
> OS X.

But the actual use of XP_DARWIN in the tree seems to be as a superset
of XP_MACOSX and XP_IOS.

Comment 13

[Steven Michaud [:smichaud] (Retired)]

In any case, I can now better understand the reason for your patch.  It's not just harmless, it actually makes sense.

Comment 14

[Steven Michaud [:smichaud] (Retired)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/124239b3d717

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/124239b3d717