196105 - [mach-o]sweep for XP_MAC, decide if we need to add XP_MACOX

Status: RESOLVED DUPLICATE of bug 281889

(SeaMonkey :: General, defect)

Description

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

sweep for XP_MAC, decide if we need to add XP_MACOX

for example, see #192602

mailnews has a few of these.  (others might as well, so marking browser-general)

on mach-o builds, XP_MAC is not defined (but XP_MACOSX is)

note, not all places need XP_MACOSX.

we need to audit mozilla (and the ns tree)

Comment 1

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

from simon:

The Mac build instructions have been updated to describe how
to build a Mach-O build. Now that all builds are makefile-based,
you have no excuse not to build and test on Mac!

<http://www.mozilla.org/build/mac.html>

Please note that for Mac OS X gcc builds, defines are set up as follows:

 * XP_MACOSX is defined
 * XP_UNIX is defined
 * XP_MAC is _not_ defined (XP_MAC is only defined for CFM builds)

This requires care when writing code for Unix platforms that exclude Mac:

#if defined(XP_UNIX) &&!defined(XP_MACOSX)

and for Mac only:

#if defined(XP_MAC) || defined(XP_MACOSX) 

Comment 2

[Conrad Carlen (not reading bugmail)]

Some more guidelines: On a quick look through XP_MAC #ifdefs in mailnews, many
had to do with file system characteristics, of which there are differences. On
XP_MACOSX:

1. File name length is not limited to 31 chars
2. The paths are POSIX, '/' delimited paths
3. Native charset, WRT nsIFile, is UTF-8
4. nsFileSpec (not that anybody should be using it, but...) is the XP_UNIX impl.
You can't get an FSSpec out of one.
5. Avoid using FSSpec

Comment 3

[Greg K.]

This seems pretty serious; raising severity to major.

Comment 4

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

yikes, just thought of one more place with XP_MAC, mork.

I see a few things we might have to changes in mozilla/db

(I've added MORK_MACOSX locally, but checked it in yet.)

c:\trees\trunk\mozilla\db\mork\src\morkConfig.cpp(55):#if defined(MORK_WIN) ||
defined(MORK_MAC)
c:\trees\trunk\mozilla\db\mork\src\morkConfig.h(55):#define MORK_MACOSX
c:\trees\trunk\mozilla\db\mork\src\morkConfig.h(59):#define MORK_MAC 1
c:\trees\trunk\mozilla\db\mork\src\morkConfig.h(80):#if defined(MORK_WIN) ||
defined(MORK_UNIX) || defined(MORK_MAC) || defined(MORK_BEOS) ||
defined(MORK_OS2) || defined(MORK_MACOSX)
c:\trees\trunk\mozilla\db\mork\src\morkConfig.h(135):#if defined(MORK_MAC)
c:\trees\trunk\mozilla\db\mork\src\morkConfig.h(148):#endif /* MORK_MAC */
c:\trees\trunk\mozilla\db\mork\src\morkConfig.h(161):#if defined(MORK_WIN) ||
defined(MORK_UNIX) || defined(MORK_MAC) || defined(MORK_BEOS) ||
defined(MORK_OS2) || defined(MORK_MACOSX)
c:\trees\trunk\mozilla\db\mork\src\morkFile.cpp(271):#if defined(MORK_MAC)
c:\trees\trunk\mozilla\db\mork\src\morkFile.cpp(287):#endif /* MORK_MAC */
c:\trees\trunk\mozilla\db\mork\src\morkStream.cpp(318):#if defined(MORK_MAC)
c:\trees\trunk\mozilla\db\mork\src\morkStream.cpp(338):#endif /* MORK_MAC */

Comment 5

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

Attachment: partial changes for Mac OS X to mozilla/db

Comment 6

[Simon Fraser [no longer active]]

-//#define XP_MAC 1
+#ifdef XP_MACOSX
+#define MORK_MACOSX
+#endif

Please don't break XP_MAC defines. This one may be needed, if Mork doesn't suck
in the normal tree-wide header files for CFM.

Comment 7

[Simon Fraser [no longer active]]

Here's another one that looks odd:
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/nsExternalHelperAppService.cpp#1259
since there is XP_MACOSX stuff right next to it.

Comment 8

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

-//#define XP_MAC 1

>Please don't break XP_MAC defines. This one may be needed, if Mork doesn't suck
>in the normal tree-wide header files for CFM.

confirmed with simon.  he's saying don't remove any XP_MAC stuff on purpose.

I clarified that in this case, I wasn't, and just removing commented out code.

Comment 9

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

cavin has done the work for this, so re-assign to him.

Comment 10

[Cavin Song]

Attachment: Patch for AbSync module. [Checked in: Comment 16]

Change the build hardware and os version for OS X.

Comment 11

[Cavin Song]

Attachment: Patch for converting filter rules. [Checked in: Comment 16]

This is needed when converting older version of filter rules to current version
for MacOS.

Comment 12

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

Comment on attachment 122326 [details] [diff] [review]
Patch for AbSync module.
[Checked in: Comment 16]

r/a=sspitzer, assuming that you can still sync with macho bits.

thanks cavin!

Comment 13

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

Comment on attachment 122506 [details] [diff] [review]
Patch for converting filter rules.
[Checked in: Comment 16]

r/a=sspitzer, thanks cavin.

Comment 14

[Cavin Song]

> (From update of attachment 122326 [details] [diff] [review])
> r/a=sspitzer, assuming that you can still sync with macho bits.
>
Yes.

Comment 15

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

what about this:

Index: db/mork/src/morkConfig.h
===================================================================
RCS file: /cvsroot/mozilla/db/mork/src/morkConfig.h,v
retrieving revision 1.23
diff -r1.23 morkConfig.h
54c54,56
< //#define XP_MAC 1
---
> #ifdef XP_MACOSX
> #define MORK_MACOSX
> #endif
78c80
< #if defined(MORK_WIN) || defined(MORK_UNIX) || defined(MORK_MAC) || defined(MO
RK_BEOS) || defined(MORK_OS2)
---
> #if defined(MORK_WIN) || defined(MORK_UNIX) || defined(MORK_MAC) || defined(MO
RK_BEOS) || defined(MORK_OS2) || defined(MORK_MACOSX)
158d159
< /*define MORK_USE_XP_STDLIB 1*/
160c161
< #if defined(MORK_WIN) || defined(MORK_UNIX) || defined(MORK_MAC) || defined(MO
RK_BEOS) || defined(MORK_OS2)
---
> #if defined(MORK_WIN) || defined(MORK_UNIX) || defined(MORK_MAC) || defined(MO
RK_BEOS) || defined(MORK_OS2) || defined(MORK_MACOSX)

Comment 16

[Cavin Song]

> Created an attachment (id=122326)
> Patch for AbSync module.
Fix checked in.

> Created an attachment (id=122506)
> Patch for converting filter rules.
Fix checked in.

Leaving the bug open for other similar issues/changes.

Comment 17

[Serge Gautherie (:sgautherie)]

(In reply to comment #1)
> and for Mac only:
> #if defined(XP_MAC) || defined(XP_MACOSX) 

It seems that we can now (2005-02) remove the |defined(XP_MAC)|...

Comment 18

[Serge Gautherie (:sgautherie)]

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

Comment 19

[Serge Gautherie (:sgautherie)]

Attachment: (Dv1) <xpfe/*> [Moved to bug 281889]

I have no compiler: Could you compile/test/(super-)review/check in this patch ?
Thanks.

{{
Index: mozilla/xpfe/components/autocomplete/src/nsLDAPAutoCompleteSession.cpp
-// build system.  The MOZ_LDAP_XPCOM preprocessor symbol is only 
-// defined on Mac because noone else needs this weirdness; thus 
}}
I wonder if |MOZ_LDAP_XPCOM| could be removed too (other bug) then ?

Comment 20

[Simon Fraser [no longer active]]

Comment on attachment 173538 [details] [diff] [review]
(Dv1) <xpfe/*>
[Moved to bug 281889]

Why is there no XP_MACOSX code in showOSAlert.cpp?

Comment 21

[Serge Gautherie (:sgautherie)]

(In reply to comment #20)
> (From update of attachment 173538 [details] [diff] [review] [edit])
> Why is there no XP_MACOSX code in showOSAlert.cpp?

I would have no idea: I'm not a Mac user, and I assume that the current code
works as it is/was.
Should these XP_MAC be relaced by XP_MACOSX, instead of removed ?
Any hint are/will welcomed: "helpwanted".

Comment 22

[Josh Aas]

see 281889, comment #24

*** This bug has been marked as a duplicate of 281889 ***

Comment 23

[Serge Gautherie (:sgautherie)]

Comment on attachment 173538 [details] [diff] [review]
(Dv1) <xpfe/*>
[Moved to bug 281889]

Moving (updated to current Trunk) patch to bug 281889 per Josh request.