342693 - Activate suiterunner on OS/2

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[Peter Weilbacher]

Seems like there isn't very much to be done to make it go. Patch upcoming.

Comment 1

[Peter Weilbacher]

Attachment: Patch without splash and DDE

The patch comments out the DDE and splash functionality present in the respective files in XPFE. Once that is needed a nsNativeAppSupportOS2.h for toolkit would be required. Not sure if the MOZ_STATIC_BUILD parts are necessary but they sure don't hurt.

Comment 2

[Peter Weilbacher]

When building I first got some problems in mailnews:

In file included from X:/trunk/mozilla/mailnews/base/build/nsMsgFactory.cpp:54:
X:/trunk/mozilla/mailnews/base/src/nsMessengerBootstrap.h:47:35: nsICommandLineHandler.h: No such file or directory

Is this something OS/2 specific that I should turn my attention to? The build continued fine once I reset MOZ_MAIL_NEWS to empty in config/autoconf.mk.

Comment 3

[[e-mail]]

(In reply to comment #2)
Currently you have to use --enable-static-mail (or --disable-mailnews).

Comment 4

[[e-mail]]

Comment on attachment 227006 [details] [diff] [review]
Patch without splash and DDE

>-RESFILE=splashos2.res
>+RESFILE = splashos2.res
>+RCFLAGS += -i $(topsrcdir)/toolkit/xre
>+ifdef BUILD_STATIC_LIBS
>+EXE_DEF_FILE = seamonkey.def
>+RCFLAGS += -DMOZ_STATIC_BUILD  -i $(DIST)/include/widget
If you're going to nit the spaces on the RESFILE line then I'm going to nit the double space on this line :-P

>+ifneq ($(OS_ARCH),WINNT)
>+SHORT_LIBNAME = suiteprf
>+endif
I don't see the point of this.

>+// #include "nsNativeAppSupportOS2.h"
I have no idea what toolkit splash screen will involve but I doubt it will use nsNativeAppSupportOS2.h so just omit the commented lines please.

sr=me with the nits fixed.

Comment 5

[Peter Weilbacher]

(In reply to comment #4)
> >+ifneq ($(OS_ARCH),WINNT)
> >+SHORT_LIBNAME = suiteprf
> >+endif
> I don't see the point of this.

OS/2 cannot have DLLs with names longer than 8 chars. Thanks for the other comments, will remove the commented lines.

And yes, --enable-static-mail helps on OS/2, too. :-)

Comment 6

[Mark Banner (:standard8)]

(In reply to comment #5)
> (In reply to comment #4)
> > >+ifneq ($(OS_ARCH),WINNT)
> > >+SHORT_LIBNAME = suiteprf
> > >+endif
> > I don't see the point of this.
> 
> OS/2 cannot have DLLs with names longer than 8 chars.

That file will probably go away soon, however according to http://developer.mozilla.org/en/docs/How_mozilla%27s_build_system_works you can just define that without the ifneq, so just add the SHORT_LIBNAME there without the ifneq.

Comment 7

[Mark Banner (:standard8)]

Comment on attachment 227006 [details] [diff] [review]
Patch without splash and DDE

+ICON 1 "..\\branding\\icons\\os2\\seamonkey.ico"

I don't see this file anywhere. Are you copying the mozos2.ico? Does this even need to be a seperate ico from the windows one - I don't know the differences between os2 and windows formats if there are any.

+++ suite\app\splashos2.rc	2006-06-25 22:36:12.000000000 +0000

I don't see much difference between this file and xpfe\bootstrap\splashos2.rc

Is it worth keeping it where it in xpfe\bootstrap (with some ifdefs) for now and moving it across later? We have other files we need to move from xpfe\bootstrap anyway and could do them all in one go (including the ico if you're going to use mozos2.ico). A couple of XXX comments would suffice for the time being.

Comment 8

[Peter Weilbacher]

(In reply to comment #6)
> (In reply to comment #5)
> > OS/2 cannot have DLLs with names longer than 8 chars.
> 
> That file will probably go away soon, however according to
> http://developer.mozilla.org/en/docs/How_mozilla%27s_build_system_works you can
> just define that without the ifneq, so just add the SHORT_LIBNAME there without
> the ifneq.

USE_SHORT_LIBNAME=1 is defined for all Windows builds, too, that's why we have always used that ifndef before, like e.g. in
http://lxr.mozilla.org/seamonkey/source/extensions/typeaheadfind/src/Makefile.in#44
http://lxr.mozilla.org/seamonkey/source/xpcom/sample/Makefile.in#61

(In reply to comment #7)
> (From update of attachment 227006 [details] [diff] [review] [edit])
> +ICON 1 "..\\branding\\icons\\os2\\seamonkey.ico"
> 
> I don't see this file anywhere. Are you copying the mozos2.ico? Does this even
> need to be a seperate ico from the windows one - I don't know the differences
> between os2 and windows formats if there are any.

Yes, the OS/2 ico format is different, and indeed I wanted to copy mozos2.ico. Sorry, I thought the "? suite/branding/icons/os2/seamonkey.ico" would make that clear.

> +++ suite\app\splashos2.rc      2006-06-25 22:36:12.000000000 +0000
> 
> I don't see much difference between this file and xpfe\bootstrap\splashos2.rc
> 
> Is it worth keeping it where it in xpfe\bootstrap (with some ifdefs) for now
> and moving it across later? We have other files we need to move from
> xpfe\bootstrap anyway and could do them all in one go (including the ico if
> you're going to use mozos2.ico). A couple of XXX comments would suffice for the
> time being.

OK, let me give that a try.


Alternatively, because there is no real use of suiterunner yet and nobody is trying to build nightlies for it on OS/2, we could also postpone this fix to a later date, when you have moved the other files.

Comment 9

[Robert Kaiser]

(In reply to comment #7)
> Is it worth keeping it where it in xpfe\bootstrap (with some ifdefs) for now
> and moving it across later? We have other files we need to move from
> xpfe\bootstrap anyway and could do them all in one go (including the ico if
> you're going to use mozos2.ico). A couple of XXX comments would suffice for the
> time being.

Copying the files now is IMHO a good idea, and we should do that for the others as well.

Comment 10

[Peter Weilbacher]

There doesn't seem to be a way to create the required RESFILE in the OBJDIR if the respective .rc file is in another directory in the source tree. Well, not without changing rules.mk in some convoluted way. So, I guess it is really better to wait with this bug until after you copied the files.

Comment 11

[Mark Banner (:standard8)]

(In reply to comment #10)
> There doesn't seem to be a way to create the required RESFILE in the OBJDIR if
> the respective .rc file is in another directory in the source tree. Well, not
> without changing rules.mk in some convoluted way. So, I guess it is really
> better to wait with this bug until after you copied the files.
> 
I've just created bug 342897 to do the copy/move. I'll try and follow it up in the next day or so with some patches.

Comment 12

[Peter Weilbacher]

Now that the copying in bug 342897 was done, it's even easier.

One copy is missing though. nsNativeAppSupportOS2.h needs to be copied from xpfe/bootstrap to toolkit/xre (I don't know why that never happened for Firefox).

Comment 13

[Peter Weilbacher]

Attachment: new patch

This patch includes an adapted version of splashos2.rc from xpfe/bootstrap. If preferred, this could also be copied first and then changed.

After we had the SHORT_LIBNAME discussion in this bug, this doesn't appear to be required any more due to FORCE_STATIC_LIB=1.

Comment 14

[Peter Weilbacher]

mkaply, I should perhaps have CC'd you earlier. Perhaps you also have any comments on this.

Comment 15

[[e-mail]]

(In reply to comment #12)
>One copy is missing though. nsNativeAppSupportOS2.h needs to be copied from
>xpfe/bootstrap to toolkit/xre (I don't know why that never happened for Firefox).
Assuming Firefox builds on OS/2 without this file, suiterunner should too?

Comment 16

[Peter Weilbacher]

(In reply to comment #15)
> Assuming Firefox builds on OS/2 without this file, suiterunner should too?

That's what I thought, too, and why it took me very long to find out why suiterunner doesn't. The Firefox build cheats and still includes the copy in xpfe/bootstrap (through LOCAL_INCLUDES).

Comment 17

[Mark Banner (:standard8)]

Comment on attachment 233402 [details] [diff] [review]
new patch

r=me assuming you're going to deal with nsNativeAppSupportOS2.h separately (as that affects toolkit).

Comment 18

[Mike Kaply [:mkaply]]

It would appear that everyone but Windows pulls nsNativeAppSupportOS2.h from xpfe?

Comment 19

[Peter Weilbacher]

Looking at http://lxr.mozilla.org/seamonkey/find?string=nsNativeAppSupport
I don't think the other platforms have a .h file for their nsNativeAppSupport, neither in xpfe/ nor in toolkit/xre/.

Comment 20

[Peter Weilbacher]

Checked into trunk.