236599 - openbsd configuration fixes for alpha, amd64, arm, i386, macppc and sparc64

Status: RESOLVED FIXED

(Firefox Build System :: General, defect, P5)

Description

[Peter Strömberg]

User-Agent:       Mozilla/5.0 (X11; U; OpenBSD macppc; en-US; rv:1.6) Gecko/20040302 Firefox/0.8
Build Identifier: Mozilla/5.0 (X11; U; OpenBSD macppc; en-US; rv:1.6) Gecko/20040302 Firefox/0.8

mozilla now works on all major platforms in openbsd
the patches are from our ports tree and I would like to get them back into mozilla

Reproducible: Always
Steps to Reproduce:

Comment 1

[Peter Strömberg]

Attachment: moz-configure patch

Comment 2

[Peter Strömberg]

Attachment: ldap lber patch

Comment 3

[Peter Strömberg]

Attachment: nspr configure patch

Comment 4

[Peter Strömberg]

Attachment: nspr openbsd.cfg

Comment 5

[Peter Strömberg]

Attachment: nspr openbsd.h patch

Comment 6

[Peter Strömberg]

Attachment: NSS openbsd patch

Comment 7

[Peter Strömberg]

Attachment: xptcall makefile patch

Comment 8

[Peter Strömberg]

Attachment: complete alpha xptcinvoke file

Comment 9

[Peter Strömberg]

Attachment: complete alpha xptcstubs file

Comment 10

[Peter Strömberg]

Attachment: complete xptcinvoke amd64 file

Comment 11

[Peter Strömberg]

Attachment: complete xptcstubs amd64 file

Comment 12

[Peter Strömberg]

Attachment: complete xptcinvoke powerpc file

Comment 13

[Peter Strömberg]

Attachment: powerpc patch

Comment 14

[Peter Strömberg]

Attachment: complete xptcinvoke powerpc file

Comment 15

[Peter Strömberg]

Attachment: complete xptcstubs asm powerpc file

Comment 16

[Peter Strömberg]

Attachment: complete xptcstubs powerpc file

Comment 17

[Peter Strömberg]

Attachment: complete xptcinvoke asm sparc64 file

Comment 18

[Peter Strömberg]

Attachment: complete xptcinvoke sparc64 file

Comment 19

[Peter Strömberg]

Attachment: complete xptcstubs sparc64 file

Comment 20

[Peter Strömberg]

Comment on attachment 143039 [details] [diff] [review]
powerpc patch

oops, duplicate of 143038

Comment 21

[Roland Mainz]

Peter:
Is it possible to make one big all-in-one patch which includes the new files, too ?

Comment 22

[cls]

What versions of OpenBSD have these patches been tested with?  Some of the
changes (like the pthread changes) look like they wouldn't work with older
versions of OpenBSD (we worked there once a long time ago).  I'd like to see a
minimally supported version in a doc somewhere.  People never tell us when
things work, only then they break so we have no idea who's using what.   Also,
can you put a more descriptive label on the patches?  Several of those listed as
'patch' are actually whole files.

Comment 23

[Peter Strömberg]

Attachment: all patches/files in one tgz file

Comment 24

[Peter Strömberg]

-pthread appeared in openbsd 2.9 (more than three years ago)

In openbsd 3.4 - were we got moz to work for the first time (see #124958), moz
worked on alpha, i386, sparc and sparc64
Now on the upcomming 3.5, powerpc and amd64 are added

Comment 25

[cls]

Comment on attachment 143027 [details] [diff] [review]
moz-configure patch

Is there a reason that the MKSHLIB_FORCE_ALL & MKSHLIB_UNFORCE_ALL vars are
being explicitly unset?    I think all of the MKSHLIB related lines can be
removed completely.  The defaults listed on line 733 of configure.in should
just work.

Comment 26

[cls]

Comment on attachment 143029 [details] [diff] [review]
nspr configure patch

Is setting LDFLAGS necessary? $_PTHREAD_LDFLAGS is already added to $OS_LIBS.

Comment 27

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 143028 [details] [diff] [review]
ldap lber patch

r+sr=dmose

Comment 28

[Peter Strömberg]

(In reply to comment #25)
> (From update of attachment 143027 [details] [diff] [review])
> Is there a reason that the MKSHLIB_FORCE_ALL & MKSHLIB_UNFORCE_ALL vars are
> being explicitly unset?    I think all of the MKSHLIB related lines can be
> removed completely.  The defaults listed on line 733 of configure.in should
> just work.

Yes, you are right, those four lines can go.

Comment 29

[Peter Strömberg]

(In reply to comment #26)
> (From update of attachment 143029 [details] [diff] [review])
> Is setting LDFLAGS necessary? $_PTHREAD_LDFLAGS is already added to $OS_LIBS.

Copy/paste from the ../configure.in patch, this line can also go away.

Comment 30

[Wan-Teh Chang]

Comment on attachment 143032 [details] [diff] [review]
NSS openbsd patch

r=wtc.

What's the difference between the outputs of 'uname -p'
and 'arch -s'?

Do we really need to pass -fPIC to the *linker* when we
build a shared library?

Comment 31

[Wan-Teh Chang]

Comment on attachment 143030 [details] [diff] [review]
nspr openbsd.cfg

r=wtc.

Is the removal of __arm32__ support intentional?

Comment 32

[Wan-Teh Chang]

Comment on attachment 143031 [details] [diff] [review]
nspr openbsd.h patch

r=wtc.

Is the removal of __arm32__ support intentional?

Do all the versions of OpenBSD that we care about
support the IPv6 socket API?

Comment 33

[Peter Strömberg]

(In reply to comment #30)
> (From update of attachment 143032 [details] [diff] [review])
> r=wtc.
> 
> What's the difference between the outputs of 'uname -p'
> and 'arch -s'?

Completely different :-)

bagheera:~ $ uname -p ; arch -s
Intel Pentium III ("GenuineIntel" 686-class, 512KB L2 cache)
i386

> 
> Do we really need to pass -fPIC to the *linker* when we
> build a shared library?

Yes
[from [e-mail]]
"You must reuse -fPIC. We've had platforms in the past where this makes a
difference:
- the compiler may have to compile extra stub code (constructors, destructors,
for instance)
- the compiler may select different library path (we've had fpic/libgcc.a)"

Comment 34

[Peter Strömberg]

(In reply to comment #31)
> (From update of attachment 143030 [details] [diff] [review])
> r=wtc.
> 
> Is the removal of __arm32__ support intentional?
> 
Yes, support in openbsd was removed after 2.8 and that's really old.

Comment 35

[Peter Strömberg]

> Do all the versions of OpenBSD that we care about
> support the IPv6 socket API?

yes

Comment 36

[cls]

Peter, can you please attach an updated patch for the configure.in changes?  Thanks.

Comment 37

[Peter Strömberg]

Attachment: moz-configure patch

Comment 38

[Peter Strömberg]

Attachment: nspr configure patch

Comment 39

[Roland Mainz]

Assigning bug to author.

Comment 40

[Wan-Teh Chang]

Comment on attachment 143368 [details] [diff] [review]
nspr configure patch

Just a minor question: do we really need to define _THREAD_SAFE?
Is that done by the -pthread compiler flag?

Comment 41

[Peter Strömberg]

(In reply to comment #40)
> Just a minor question: do we really need to define _THREAD_SAFE?
> Is that done by the -pthread compiler flag?

-pthread defines _POSIX_THREADS
If moz doesn't check on _THREAD_SAFE (as a quick glimpse seems to indicate) it
can also go.

Comment 42

[cls]

All of the patches have been checked in (including respective client branches &
trunks) with the exception of the xptcall changes and the NSS patch.

Comment 43

[cls]

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

Comment 44

[Takanori MATSUURA]

I came here from the bonsai checkin list at 03/09/2004 23:46.

Is this bug related to Bug 163013 and Bug 232742 ?

In the attachment 140309 [details] [diff] [review], amd64 platform is defined as "__x86_64__".
And a part of this attachment is merged into the trunk at 03/08/2004 19:10.

On the other hand, in the rev. 5.3 of
mozilla/directory/c-sdk/ldap/libraries/liblber/lber-int.h, 
amd64 platform is defined as "__amd64__".

Comment 45

[Wolfgang Rosenauer [:wolfiR]]

AMD64's architecture name is defined as x86-64 as comment #44 stated.
Please change this. It seems to be a race condition in check-ins ;-)

Comment 46

[Peter Strömberg]

Attachment: nspr prprf.c va_copy patch (not needed for NSPR 4.6/Mozilla 1.8 or newer)

forgot this file

Comment 47

[Wan-Teh Chang]

Comment on attachment 144400 [details] [diff] [review]
nspr prprf.c va_copy patch (not needed for NSPR 4.6/Mozilla 1.8 or newer)

r=wtc.	This patch is suitable for NSPR 4.5/Mozilla 1.7
or older.  In newer NSPR releases, I fixed this porting
problem by completely eliminating the VARARGS_ASSIGN
macro, so this patch is not needed there.

Comment 48

[Mike Shaver (:shaver emeritus)]

Comment on attachment 143034 [details] [diff] [review]
complete alpha xptcinvoke file

Other than that we probably don't want to put more code under Netscape's
copyright (and as of 1998, no less -- this bug's not _that_ old!), this looks
OK.

Please fix the copyright and date before committing.

Comment 49

[Brendan Eich [:brendan]]

What can I do to help get this bug moving again?

/be

Comment 50

[Brendan Eich [:brendan]]

Benjamin, Luser: can one of you take this and help get the patches updated and landed?

/be

Comment 51

[Martynas Venckus]

Attachment: all xptcalls for 1.8 in one tgz file + link in the Makefile, new

Brendan, please CC r? the right people...

Comment 52

[Frank Wein [:mcsmurf]]

Attachment: Xptcall files (against 1.8 branch)

Martynas Venckus provided a new patch against the 1.8 branch (trunk will follow). This ZIP file includes a patch for xpcom/reflect/xptcall/src/md/unix/Makefile.in and all the respective files for arm/sparc64/ppc/alpha/amd64.

Comment 53

[Frank Wein [:mcsmurf]]

Comment on attachment 275703 [details]
Xptcall files (against 1.8 branch)

lack of communication :)

Comment 54

[Brendan Eich [:brendan]]

One big zip file is not the way to get review. It's much better to split the patch up by module (http://www.mozilla.org/owners.html is still the source -- there's a single-field form in which you can type a relative source pathname to find which module a file belongs to) and request r? of the right people.

Honestly, I'm sorry this bug did not get action, but I'm not your patch review requesting, unzipping and splitting and reattaching slave. The owners and peers who failed you last time should probably do that, this once, to make up for inaction. But it's a two way street, so if mcsmurf or martynas could do it, even better.

Cc'ing dveditz, who can help bless 1.8 branch landings for porting changes.

/be

Comment 55

[Brendan Eich [:brendan]]

Another approach: someone with commit access just unzip, verify that only ifdef'ed or otherwise build-configured-for-openbsd code is affected, ask dveditz to bless en-masse, and commit to the branch.

Trunk should probably be done the hard way.

/be

Comment 56

[Frank Wein [:mcsmurf]]

Attachment: Makefile patch (1.8 branch)

This is the Makefile change required for building, changes xpcom/reflect/xptcall/src/md/unix/Makefile.in. If this gets checked in, all the other new files need to be checked in, too.

Comment 57

[Benjamin Smedberg]

Comment on attachment 275813 [details] [diff] [review]
Makefile patch (1.8 branch)

Boy I hate the OS_TEST usage, but it seems to be standard in this makefile... I'm pretty sure we should end up using TARGET_CPU instead.

Comment 58

[Frank Wein [:mcsmurf]]

Attachment: Xptcall files patch (1.8 branch)

This patch includes all the xptcall files needed for OpenBSD for the various platforms. I noticed the license header is missing in the amd64 files, those headers must of course be added before check-in (I didn't do it since I dunno who worked on this).
Martynas: Were you the only person working on the amd64 files?

Comment 59

[Frank Wein [:mcsmurf]]

Dan Veditz: Would it be ok to check in the xptcall files patch on the 1.8 branch? Only OpenBSD is affected here. For the trunk a real review should be done I guess, but getting someone to review this patch for the 1.8 branch? Dunno if it is worth it, IMHO rather not.

Comment 60

[Martynas Venckus]

Attachment: xptcinvoke_openbsd_amd64.cpp + license

Comment 61

[Martynas Venckus]

Attachment: xptcstubs_openbsd_amd64.cpp + license + small fixes

Comment 62

[Daniel Veditz [:dveditz]]

(In reply to comment #59)
> Dan Veditz: Would it be ok to check in the xptcall files patch on the 1.8
> branch? Only OpenBSD is affected here. For the trunk a real review should be
> done I guess, but getting someone to review this patch for the 1.8 branch?
> Dunno if it is worth it, IMHO rather not.

I don't know if getting a review is worth it to you or not, but it's not getting approved for the (more conservative) branches without one. On the other hand these aren't supported platforms so we're not going to be too conservative about untested code as long as it's been approved (reviewed) by the module owner.

Comment 63

[Daniel Veditz [:dveditz]]

Comment on attachment 275976 [details] [diff] [review]
Xptcall files patch (1.8 branch)

please get this reviewed by a module owner or peer before asking for branch approval

Comment 64

[Daniel Veditz [:dveditz]]

Comment on attachment 275976 [details] [diff] [review]
Xptcall files patch (1.8 branch)

approved for 1.8.1.7, a=dveditz for release-drivers

Comment 65

[Martynas Venckus]

Comment on attachment 275982 [details]
xptcstubs_openbsd_amd64.cpp + license + small fixes

Please don't forget to commit this too, for amd64 support.

Comment 66

[Martynas Venckus]

Comment on attachment 275980 [details]
xptcinvoke_openbsd_amd64.cpp + license

Please don't forget to commit this too, for amd64 support.

Comment 67

[Benjamin Smedberg]

Comment on attachment 275980 [details]
xptcinvoke_openbsd_amd64.cpp + license

This should be part of the patch, and it's ok to land with the existing r+a

Comment 68

[Frank Wein [:mcsmurf]]

I'll check this in soon.

Comment 69

[Frank Wein [:mcsmurf]]

Checked in on the 1.8 branch.

Comment 70

[Robert Sayre]

Is this fixed?

Comment 71

[Frank Wein [:mcsmurf]]

Resolving as fixed. It is fixed on the 1.8.1 branch, but not on trunk because the patch needs updating for trunk.
Martynas: Please open a new bug for any trunk work.