72693 - Build PSM 2.0 for OS/2

Status: VERIFIED FIXED

(Core Graveyard :: Security: UI, defect, P2)

Description

[jhp (no longer active)]

Changes needed to allow PSM 2.0 to build under OS/2.  This bug is for 
any fixes needed to get this going.

Comment 1

[jhp (no longer active)]

Attachment: Patch file for OS/2 changes

Comment 2

[jhp (no longer active)]

Attached a patch above that contains the fixes that I needed to get 
this building for OS/2, using the trunk for security/manager and the 
PSM20_M_1_5_TAG for coreconf and nss.

One question: security/ssl/src/Makefile.in looks for libssl.lib, 
libnss.lib and so on in the main Mozilla DIST, but they are not copied 
there.  So I added some lines in security/manager/Makefile.in to copy 
the required libraries from the security dist to the main dist 
directory.  Why don't the other platforms need this?  What is OS/2 
doing wrong?

Comment 3

[Dmitry Kubov]

Becouse OS/2 build copies many files from dist/ to dist/OBJxxxx/
Its ugly.

Comment 4

[jhp (no longer active)]

Attachment: latest patch

Comment 5

[jhp (no longer active)]

Added patch with better CALLBACK support.  Once again, based on 
PSM20_M_1_5_TAG for security/coreconf and security/nss, and trunk for 
security/manager.

Comment 6

[David P. Drinan]

Thanks for the patch. I'm currently reviewing it.

Comment 7

[jhp (no longer active)]

Attachment: updated patch with nelsonb's suggestions

Comment 8

[jhp (no longer active)]

Attachment: added nsprpub/pr/include/prtypes.h

Comment 9

[jhp (no longer active)]

Created a new patch incorporating nelsonb's suggestions.  Also, had to 
include nsprpub/pr/include/prtypes.h in order to properly defined 
NSS_CALLBACK_DECL in nssbaset.h.

Comment 10

[jhp (no longer active)]

Attachment: more callbacks

Comment 11

[jhp (no longer active)]

NOTE: Latest patch supercedes all previous ones.  Based on 
PSM20_M_2_TAG.

Comment 12

[jhp (no longer active)]

Could we at least get a review for nssbaset.h?  Once that is checked 
in, then the rest can go in piece by piece.

Comment 13

[John Unruh]

Milestone 2.0

Comment 14

[jhp (no longer active)]

Attachment: PSM 2.0 only patch

Comment 15

[jhp (no longer active)]

The latest patch has only PSM files.  Removed all references to NSS.  
The NSS changes are now in bug 77199.  Also, this depends on changes 
to nsinstall in bug 76900.  nsinstall.exe must be built and copied 
into the moztools directory on OS/2.

Comment 16

[jhp (no longer active)]

When building in security/manager/ssl/src, it looks for NSS libs in 
dist/lib, rather than OS2_DBG.OBJ/dist/lib.  I got around this by 
copying those libs to the main dist in security/manager/Makefile.in.  
Why don't other platforms need to do this?  Is this solution the 
correct way to do this?

Comment 17

[David P. Drinan]

I'm concerned by having to copy NSS libs to dist/lib, instead of having PSM find 
them in OS2_DBG.OBJ/dist/lib. Any idea why $(DIST) appears to be broken when 
building PSM? I'm assuming $(DIST) is set correctly for the other components 
such as necko etc.

Comment 18

[Wan-Teh Chang]

David -- you can go ahead and check in the changes for the
other files.  Just replace "NSS_CALLBACK" by "PR_CALLBACK".
The "PR_CALLBACK" changes can be committed independent of
the NSS patch.

Comment 19

[jhp (no longer active)]

drinan:
Sorry for taking so long to respond.  I have been trying to build 
security in order to find out why we need to copy over the files.  
Basically, my dist directory (mozilla/obj/dist) looks like this:
   bin
   idl
   include
   lib
   OS22.45_DBG.OBJ
   private
   public

The last three entries are created by the security build process.  The 
directory OS22.45_DBG.OBJ contains headers copied from dist/include as 
well as the security headers, and the "lib" directory, where the NSS 
libs are located.  When we come to security/manager/ssl/src, the 
Makefile there considers $(DIST) to be mozilla/obj/dist/lib, and has 
no idea about the existence of OS22.45_DBG.OBJ.  What is the best way 
to fix this problem?

Comment 20

[jhp (no longer active)]

Attachment: Patch diffed against latest code

Comment 21

[David P. Drinan]

I'm adding Javier Delgadillo to cc-list.

Javi,

Please review this patch and let me know what you think of the $(DIST) issue. 
Otherwise, the rest of the patch is good.

Comment 22

[Javier Delgadillo]

I really don't like listing all of the static libraries and then copying them
over.  I'd almost prefer a cp *.lib as a work around and then filing a bug
against NSS to let us tell it where to put the lib files.

wtc, is there another solution to Javier's problem?

Comment 23

[Wan-Teh Chang]

Javier (of Netscape): I am testing a fix that makes it unnecessary
to copy the static libs over.  I understand why Javier (of IBM) needs
to do that on OS/2.

David: You can go ahead and review the non-makefile changes.  I'd
suggest that you hold on doing anything with the makefile.

Comment 24

[David P. Drinan]

OK, r=ddrinan for all the non-makefile changes. I've applied this patch to my 
tree and will check in once this bug has sr=.

Comment 25

[Wan-Teh Chang]

David,

My patch for bug #81181 
(http://bugzilla.mozilla.org/showattachment.cgi?attach_id=35573)
should address the issue that Javier Pedemonte's patch for
security/manager/Makefile.in attempts to fix, so you can safely
ignore that part of his patch.

Comment 26

[David P. Drinan]

wtc,
Good. I'll just check in the non-makefile changes. 

Javier (Pedemont),
Who is doing super-review on this bug? I can't check in until we get sr=

Comment 27

[David P. Drinan]

Attachment: Updated patch. Removed makefile changes and use PR_CALLBACK instead of NSS_CALLBACK.

Comment 28

[Wan-Teh Chang]

r=wtc.

Comment 29

[Mike Kaply [:mkaply]]

I don't understand why you guys changed all the NSS_CALLBACKs to PR_CALLBACKs 
(including wtc's NSS that is already checked in)

if you have this:

#define NSS_IMPLEMENT      PR_IMPLEMENT(DUMMY)
#define NSS_EXTERN_DATA    PR_EXTERN_DATA(DUMMY)
#define NSS_IMPLEMENT_DATA PR_IMPLEMENT_DATA(DUMMY)

why wouldn't you have this:

#define NSS_CALLBACK       PR_CALLBACK

Security is its own component/module and should have its own callback that can 
be set independent of what NSPRs callback is set to, similar to Javascript 
having JS_DLL_CALLBACK

I don't think the calling convention in NSS should be dependent on the calling 
convention in NSPR.

This becomes especially important when we try to switch calling conventions. We 
could actually leave NSS alone (only change the header from mapping to 
PR_CALLBACK to mapping to a calling convention explicitly) and only change NSPR 
to a new calling convention.

Comment 30

[Wan-Teh Chang]

http://lxr.mozilla.org/seamonkey/search?string=PR_CALLBACK

PR_CALLBACK is used all over the places.  Are you planning to
fix them all?

Javascript has JS_DLL_CALLBACK because it has the requirement
that it can be built without NSPR.  NSS does not have that
requirement.

These NSS_ macros were added because the PR_XXX(type) form
breaks some Unix source code browsing tools such as ctags,
which don't like to see the function return type in parentheses:
#define NSS_IMPLEMENT      PR_IMPLEMENT(DUMMY)
#define NSS_EXTERN_DATA    PR_EXTERN_DATA(DUMMY)
#define NSS_IMPLEMENT_DATA PR_IMPLEMENT_DATA(DUMMY)

Comment 31

[Mike Kaply [:mkaply]]

> PR_CALLBACK is used all over the places.  Are you planning to
> fix them all?

We're trying :)

We're looking at different places in the code and changing it where it makes 
sense. For instance PNG, MNG, and JPEG have there own way to set calling 
convention, so they shouldn't use PR_CALLBACK - they should use the one in their 
own header.

We also changed tons of PR_CALLBACKS to JS_DLL_CALLBACKS and vice versa.

So we are trying to clean it up a bit. I don't know if there is any "right" way 
to do it.

For instance, there is a PrefChangedCallback in prefs that uses PR_CALLBACK - 
I'm not sure what it should be. Should there be a generic NS_CALLBACK or a 
PREF_CALLBACK? NS_QuickSort is another good example.

I think the original purpose of PR_CALLBACK was for function callbacks to NSPR, 
and people (especially OS/2) just used it because it was there. So we're doing 
what we can to make it a little easier for components to exist standalone.

But if NSS is totally dependendent on NSPR, then that's OK with me. I trust you 
guys on this. You've had the code a lot longer than me :)

So let's just get this stuff in and I'll stop worrying about it.

Comment 32

[jhp (no longer active)]

wtc, drinan:
Yes, I build fine with the patch from 81181.  No other changes 
necessary to security/manager/Makefile.in.

I think Javier Delgadillo is the sr for psm bugs.

Comment 33

[Wan-Teh Chang]

PR_CALLBACK is intended for general use.

Comment 34

[Javier Delgadillo]

heh. I have fooled people into thinking I have power ;)

I can give r= for PSM bugs, but our super reviews have been going to
[e-mail], [e-mail], or [e-mail]

Feel free to forward the sr request to one of them.

Comment 35

[jhp (no longer active)]

well, i'm glad to give javi an ego boost!

blizzard, can you super-review this?

Comment 36

[Christopher Blizzard (:blizzard)]

sr=blizzard

Comment 37

[David P. Drinan]

Checked in.

Comment 38

[David P. Drinan]

Marking FIXED.

Comment 39

[John Unruh]

Verified per ddrinan's comment.