Closed Bug 25555 Opened 25 years ago Closed 24 years ago

OS/2 build enablement

Categories

(SeaMonkey :: Build Config, defect, P3)

Product:
SeaMonkey
Component:
Build Config
Platform:
All
OS/2
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: hctrinh, Assigned: hctrinh)

References

Details

(Whiteboard: [PDT-])

Attachments

(3 files)

Diffs for Makefile.in changes for OS/2 (163 files)
68.79 KB, text/plain
Details
Diffs for other OS/2 changes to *.c* and *.h* files
79.00 KB, text/plain
Details
OS/2 .c and .h diffs against the latest tree (2/22/2000)
39.35 KB, patch
Details | Diff | Splinter Review 
- Update makefile.in files to enable OS/2 build (about 150 files)
- Update source files with ifdefs for OS/2  ( another 100 files)

We would like to get these changes in prior to M14 freeze. OS/2 platform has not 
been updated since M7 and we need to get these in asap to avoid having to port 
them over again at every milestone release. We discussed these with Jim Dunn who 
was at IBM today and he suggested that we open this as "beta1" problem for 
consideration for M14, even though it is not really "beta1". We will attach 
diffs next week. 

Many of the changes to Makefile.in's are to:
- use 8.3 dll names
- override the "override" statements for shared libraries
- use $(BIN_SUFFIX), $(LIB_SUFFIX) for makefile targets, as is done on Windows.

Changes to crossplatform code will be minor ifdef changes for OS/2. Most of the 
changes are to avoid Windows specific code which is included via "ifdef XP_PC". 
We will make minor changes (to a lot of files) just to get OS/2 to build.

We will build these changes on both Linus and Windows prior to submission.
Keywords: beta1
Target Milestone: M14
When you get the diffs either create an attachment and post or
send the list to me and I will post, since we will need to 
review in order to approve.

BTW: for everyone that is cc'd on this, I was in Austin on Fri
and talked to them about how to get these checked in before M14.
Hopefully we can do this and not cause any problems.
adding mitchell
Adding waterson.  
I talked to the OS/2 team and the diffs should be available sometime this week.
Putting on PDT- radar for beta1.
Whiteboard: [PDT-]
The 2 diffs attached this morning are based on the M13 milestone release.
btw, we are currently testing the combined changes from the 2 sets of diffs on 
Linux and Windows platforms.
I was a bit premature in posting my diff. 2 lines have been corrected:

Index: nsSpecialSystemDirectory.cpp
===================================================================
RCS file: /cvsroot/mozilla/xpcom/io/nsSpecialSystemDirectory.cpp,v
retrieving revision 1.25
diff -r1.25 nsSpecialSystemDirectory.cpp
.
.
.
748c792,805
< 
---
> #ifdef XP_OS2   // <-THIS LINE CHANGED FROM PREV DIFF (DIDN'T START IN COL. 1)
>         case OS2_SystemDirectory:
>         {
>             ULONG ulBootDrive = 0;
>             char  buffer[] = " :\\OS2\\System";
>             DosQuerySysInfo( QSV_BOOT_DRIVE, QSV_BOOT_DRIVE,
>                              &ulBootDrive, sizeof ulBootDrive);
>             buffer[0] = 'A' - 1 + ulBootDrive; // duh, 1-based index...
>             *this = buffer;
> #ifdef DEBUG
>             printf( "Got OS_SystemDirectory: %s\n", buffer);
> #endif
>         }   // <- THIS LINE CHANGED FROM PREVIOUS DIFF (WAS AFTER ENDIF BELOW) 
> #endif

We are finishing up our Windows testing and it is looking good. The only other 
change to the diffs is in webshell\tests\viewer\nsViewerApp.cpp where we had 
erroneously introduced a stray ")" at line 1111. 
As we are just becoming familiar with the Mozilla open source process, when will 
we know when we are able to put these changes in before M14 freeze? Thanks.
I am going to go through the other attachment (no Makefile) today.
I sent the following out on Fri (but should have included it here too).
Anyhow here is my response from Fri.  I would like to have by this
afternoon (2/7) a plan for getting these in (via a carpool) or whatever.

--- email sent out on Fri 2/4 ---
Ok, I went through the Makefile diff that you had attached (thanks).  
Right now it looks like it won't cause 'others' any problems (for 
this one, I am more worried about Linux bustages, since this 
shouldn't hurt Windows).

Anyhow, like I said this looks good, I am hoping Mcafee,Chris and/or Leaf
might also give it a looksee.  I need at least one of them to say 'yeah' and
none of them to say NAY.  I still have to go through the other list.  Once
I do and I get people to 'approve' these we can schedule a carpool.

My one comment is that it seems like you are doing a lot of extra work and
I would think there has to be a simpler way of doing this.  I am worried that
the maintainence of this will be a nightmare!  I went through this 'type' of
change in Dec and tried to make sure that all libs that are required to 
'resolve' symbols at link time were being included and it is too bad we 
can't leverage that work.

For example you have lots of the following changes...
Well the library name is LIBRARY_NAME already (there should be
a way in config/rules.mk to map OS2_LIBRARYNAME to LIBRARYNAME
if IS_COMPONENT is set.  Does OS2_IMPLIB match IS_COMPONENT?
if so, another mapping.  OS2_LIBS seem to match to EXTRA_DSO_LDOPTS
(except for the -l) too bad we couldn't script them clean (again in rules.mk).

(using the followin as an example)
Index: db/mork/build/Makefile.in
===================================================================
RCS file: /cvsroot/mozilla/db/mork/build/Makefile.in,v
retrieving revision 1.10
diff -r1.10 Makefile.in
39a40
> ifneq ($(MOZ_WIDGET_TOOLKIT),os2)
50a52,57
> else
> OS2_LIBRARYNAME = mork
> OS2_IMPLIB = 1
> OS2_LIBS = libmsgmork_s xpcom
> OS_LIBS += $(NSPR_LIBS)
> endif

adding pavlov and Chris S.
Chris S. I am going to ask you to review the Makefile diffs if you
don't mine.
r= jdunn@netscape.com for the following.
      mozilla/config/pathsub.c
      mozilla/editor/base/nsEditor.cpp
      mozilla/include/xp_core.h
      mozilla/gfx/src/os2/*
      mozilla/include/os2file.h, os2sock.h
      mozilla/intl/locale/src/os2/*
      mozilla/intl/uconv/src/nsOS2Charset.cpp
      mozilla/widget/src/os2/*
      mozilla/xpcom/io/nsFileSpecOS2.cpp
r= mscott@netscape.com
    mozilla/webshell/src/nsWebShell.cpp
    mozilla/mailnews/compose/src/nsSmtpProtocol.cpp
r= beard@netscape.com
    modules/plugin/nglsrc/nsPluginInstancePeer.cpp
    mozilla/modules/plugin/public/nsplugindefs.h
r= beard@netscape.com
    mozilla/modules/plugin/nglsrc/nsPluginHostImpl.cpp
r= nhotta@netscape.com 
   intl/locale/src/nsLocaleFactory.cpp  (also opened post-M14 bug 26962 too)
   intl/locale/src/nsLocaleFactory.h       
   intl/locale/src/nsLocaleService.cpp       

r= waterson
   rdf/base/public/rdf.h 

r= sspitzer
   modules/libpref/src/nsPref.cpp       
   modules/libpref/src/prefapi.c
Blocks: 26962
Well, about a 1/3 of the Makefile.ins patched had failed hunks so I can't verify
that they do not break the existing build.  I'd like to have as many of the OS2_
variables converted to the generic variables that already exist (as jdunn
pointed out).  This would move almost all of the OS/2 specific ifdefs into
rules.mk...except that I do not see any diffs to rules.mk nor config.mk.

I have taken the liberty of inserting email that I received from
Henry Sobotka.  ChrisS. I am going to try to dig through this but
if you have anytime to offer suggestions that would be great!

Henry wrote:
------------
There's nothing to be done with IS_COMPONENT as we already rely on it
just like the other platforms using the autoconf build. Nor do I see how
AC_DEFINES can do the same job as our OS/2 blocks in the makefiles. Most
involve four variables:

OS2_IMPLIB: This originated at a time when the other platforms were
doing a jumble of static and dynamic linkage, and we wanted DLLS. So for
us, it means "build a dll". In one case (xpt), not getting the DLL
immediately into LIBPATH was causing problems, so we opted for static
linkage instead of a shared library like the other platforms. In other
words, it gives us a freedom of choice to which I believe every
operating system or platform is entitled. As a sidenote, when
NO_STATIC_LIB and NO_SHARED_LIB were introduced last spring, they broke
us entirely: gmake went through the motions but for some reason we
literally ended up building nothing; so we undefined them for OS2 in
rules.mk and everything was back to normal. 

OS2_LIBRARYNAME: As I've pointed out, this is essential for getting the
8.3 DLL names OS/2 requires. Having a regexp or Perl script do that is
unsatisfactory because it would separate the names from the modules as
well as introduce the risk of duplicate names. If I happen to forget the
name of a DLL associated with a module to put a breakpoint in it, now I
need only glance at the Makefile.in, which is in the same directory as
the one whose code I'm looking at. I don't want to have to do mental
parsing based on LIBRARY_NAME, or dig up a separate list in /config or
/build, or scan a directory with 100+ DLLs looking for what is probably
the right one.

OS2_LIBS: The names of our import libraries have to match those of the
DLLs. I've also noticed several cases where our linkage differs from the
other platforms; either we don't need some libraries they use or we need
others they don't. Having the bare name there also makes it easy to
generate linkage strings in the two formats we need (-lfoo for gcc, and
and [path]/foo.lib for icc) by means of a single line in OS2.mk.

OS_LIBS: Essential for NSPR linkage.

Conversely, EXTRA_DSO_LDOPTS sometimes works for emx+gcc, but elsewhere
causes problems because lib gets prepended so we end up getting -llibfoo
and break with "no such library". In the case of VAC++, it's a killer so
we would have to wrap it everywhere anyhow. From a maintenance
standpoint, it's far simpler to have a separate little OS2 block for
both our builds than to have one sometimes tangled in EXTRA_DSO_LDOPTS
and at others steering clear.
I'm shooting for conformity.  The idea would be to have the near 800
Makefile.ins in the build to be as generic as possible and have all of
the platform specific switches in configure.in, config.mk & rules.mk.
We're still a ways away from that goal but we're slowly making progress.
The platform specific ifdefs should only be added where necessary.

> OS2_IMPLIB: This originated at a time when the other platforms were
> doing a jumble of static and dynamic linkage, and we wanted DLLS. So for
> us, it means "build a dll". 

So, OS2_IMPLIB is the equivalent of NO_SHARED_LIB= which is the default.

> In one case (xpt), not getting the DLL
> immediately into LIBPATH was causing problems, so we opted for static
> linkage instead of a shared library like the other platforms. 

Currently, xpt is built static only.

>In other words, it gives us a freedom of choice to which I believe every
> operating system or platform is entitled.

Which is fine and dandy. However, you're duplicating functionality that
is already in place.

> As a sidenote, when
> NO_STATIC_LIB and NO_SHARED_LIB were introduced last spring, they broke
> us entirely: gmake went through the motions but for some reason we
> literally ended up building nothing; so we undefined them for OS2 in
> rules.mk and everything was back to normal. 

Where any bugs opened about this?  Was anyone outside of mozilla.os2
aware of/informed about the breakage?

> OS2_LIBRARYNAME: As I've pointed out, this is essential for getting the
> 8.3 DLL names OS/2 requires. 

Rather than using OS/2 specific ifdefs, I'd rather have a define,
NEEDS_8_3_FILENAMES, that is set in configure.in or config.mk.  Then using
that define, set LIBRARY_NAME to its 8.3 shorted name if necessary.
Someone (besides me) was working on a mingw port and this would help
them out as well.

> OS2_LIBS: The names of our import libraries have to match those of the
> DLLs. I've also noticed several cases where our linkage differs from the
> other platforms; either we don't need some libraries they use or we need
> others they don't. 

Outside of OS specific libs (which should be set in OS_LIBS), I'm curious
as to why the link line would differ.  Afaik, we are not relying upon
shared library dependencies when we link.  Oh, could it be that the
non-gcc compiler requires you to resolve all symbols even when linking
a shared library, like AIX's does?

> Having the bare name there also makes it easy to
> generate linkage strings in the two formats we need (-lfoo for gcc, and
> and [path]/foo.lib for icc) by means of a single line in OS2.mk.

This is something that others using the build could benefit from.  Again,
I'm thinking about the mingw build.  The first problem that comes into
mind is that you now need the 8.3 ifdefs when setting LIBS as well.
Eventually, this may become unnecessary if we can start implementing the
mod.mks as described in the "Recursive Make Considered Harmful" paper
(thinking aloud now).

> Conversely, EXTRA_DSO_LDOPTS sometimes works for emx+gcc, but elsewhere
> causes problems because lib gets prepended so we end up getting -llibfoo
> and break with "no such library". In the case of VAC++, it's a killer so
> we would have to wrap it everywhere anyhow. From a maintenance
> standpoint, it's far simpler to have a separate little OS2 block for
> both our builds than to have one sometimes tangled in EXTRA_DSO_LDOPTS
> and at others steering clear.

I think we can fix this w/o having to put the OS/2 specific ifdefs inside
each Makefile.in.


> Where any bugs opened about this?  Was anyone outside of mozilla.os2
> aware of/informed about the breakage?

No. I didn't see any point in a filing a bug affecting code not in the tree.

> set LIBRARY_NAME to its 8.3 shorted name

In other words, have a SHORT_LIBRARY_NAME wherever LIBRARY_NAME > 8?

> could it be that the non-gcc compiler requires you to resolve all symbols
> even when linking a shared library, like AIX's does?

Both our compilers (gcc and icc) require all symbols to be resolved. That's 
also one of the reasons I moved our linkage to the install cycle.

> EXTRA_DSO_LDOPTS

I'd be very happy if it were generic enough for us to use. The main reason we 
opted for OS/2 blocks was because we didn't want to risk breaking other 
platforms.

OK, I think we need to have a little more history here to make the situation 
more clear.

We are basically moving from an M7 to M13. Our goal is to start getting the OS/2 
changes in so we can stay up with the tip. The merge effort is very difficult. 

While we realize that the changes right now are not the most elegant, we would 
like to get them in so we can starting having different parts of the team work 
on different things, as well as ease distribution of the OS/2 version.

We have been testing our changes on Linux and Windows and so far they have not 
broken. This has been a huge focus of our team.

What we are trying to do is beg, plead, bargain, etc. to at least get what we 
have in the tree so we can make progress without remerging.

So that's our story.

We ask for sympathy, a great deal of sympathy. And hopefully permission to check 
the files in :)

Thanks

Mike Kaply
> In other words, have a SHORT_LIBRARY_NAME wherever LIBRARY_NAME > 8?

Yes, that could work as well.

> Both our compilers (gcc and icc) require all symbols to be resolved. That's 
> also one of the reasons I moved our linkage to the install cycle.

I know at one point jdunn was working on adding logic to the build to resolve
all of the link symbols when necessary for the AIX build.  It seems like there
should be a very generic solution for this problem.


I have fixed all of the unresolved symbols at link time with
changes to EXTRA_DSO_LDOPTS.  
I introduced 
MOZ_COMPONENT_LIBS = MOX_COMPONENT_XPCOM_LIBS + MOZ_COMPONENT_NSPR_LIBS
(set in configure.in) and propogated to config/autoconf.mk
These are used throughout the makefiles but are essential for IS_COMPONENT
things.  
So for pure components: 
EXTRA_DSO_LDOPTS = $(MOZ_COMPONENT_LIBS)
or
EXTRA_DSO_LDOPTS = $(MOZ_COMPONENT_XPCOM_LIBS)

depending on requirements.
If you need to 'tweak' MOZ_COMPONENT_LIBS you can do that in
configure.in under YOUR os/2 section.
Then in no IS_COMPONENT things, I use these when needed and also
made sure that EXTRA_DSO_LDOPTS contained all the libs required.
So all unresolved symbols should be covered, if not then we need
to look into how these rules are being translated on os/2.
Cool. I've been going at it from the other end of removing OS2_IMPLIB and 
OS2_LIBRARY name and tweaking rules.mk. I'm using SHORT_LIBNAME instead of 
SHORT_LIBRARY_NAME to fit into the existing neat columns in the Makefile.ins.

Does MOZ_COMPONENT_LIBS include things like mozjs (cf. -lmozjs among 
EXTRA_DSO_LDOPTS in caps/src)?

No, MOZ_COMPONENT_LIBS is just a macro to include xpcom and NSPR_LIBS.
It was just put in place to cover the main libs (xpcom and NSPR_LIBS)
that are required by all Components (IS_COMPONENT=1).  I believe that 
all IS_COMPONENT=1 libs have a line like 
EXTRA_DSO_LDOPTS += $(MOZ_COMPONENT_LIBS)

EXTRA_DSO_LDOPTS is a macro used in config/rules.mk
EXTRA_DSO_LDOPTS will have all of the libs required to link against

SHARED_LIBRARY_LIBS will include all of the 'sub libraries' that 
are basically of the folm lib<blah>_s.a and there is the rule that
extracts all of the .o's out of the SHARED_LIBRARY_LIBS and creates
a SUB_LOBJS that is then linked against (maybe not for OS/2 at this moment)

Chris, how would you feel about a solution like this:

1) Delete OS/2 block entirely;
2) Add to Makefile.in (using webshell/src as example), where necessary:

SHORT_LIBNAME	= webshell
EXTRA_DSO_LIBS	= jsdom docshell

3) Change EXTRA_DSO_LIBS to:

EXTRA_DSO_LDOPTS = \
		$(MOZ_TIMER_LIBS) \
		-L$(DIST)/bin \
		$(EXTRA_DSO_LIBS) \
		$(MOZ_COMPONENT_LIBS) \
		$(NULL)

4) Add to rules make:

ifeq ($(MOZ_OS2_TOOLS),VACPP)          # or any other platform that needs this
EXTRA_DSO_LIBS		:= $(addsuffix .$(LIB_SUFFIX),$(addprefix 
$(DIST)/lib/,$(EXTRA_DSO_LIBS)))
else
EXTRA_DSO_LIBS		:= $(addprefix -l,$(EXTRA_DSO_LIBS))
endif

We would also use the SHORT_LIBNAME slot in a case like uconv, where we need 
another name such as mozuconv.dll to avoid a conflict with the OS/2 system 
uconv.dll.

Henry, that looks good to me.
Status: NEW → ASSIGNED
Most of these changes are now in the tree. We still have lots of Makefiles to 
get in, but those will be done under a different bug.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: