If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Build PSM 2.0 for OS/2

VERIFIED FIXED in psm2.0

Status

Core Graveyard
Security: UI
P2
normal
VERIFIED FIXED
17 years ago
a year ago

People

(Reporter: jhp (no longer active), Assigned: David P. Drinan)

Tracking

1.0 Branch
psm2.0
x86
OS/2
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(8 attachments)

(Reporter)

Description

17 years ago
Changes needed to allow PSM 2.0 to build under OS/2.  This bug is for 
any fixes needed to get this going.
(Reporter)

Comment 1

17 years ago
Created attachment 28279 [details] [diff] [review]
Patch file for OS/2 changes
(Reporter)

Comment 2

17 years ago
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

17 years ago
Becouse OS/2 build copies many files from dist/ to dist/OBJxxxx/
Its ugly.
(Reporter)

Comment 4

17 years ago
Created attachment 28933 [details] [diff] [review]
latest patch
(Reporter)

Comment 5

17 years ago
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.
(Reporter)

Updated

17 years ago
Blocks: 74459
(Assignee)

Comment 6

17 years ago
Thanks for the patch. I'm currently reviewing it.
(Reporter)

Comment 7

17 years ago
Created attachment 30193 [details] [diff] [review]
updated patch with nelsonb's suggestions
(Reporter)

Comment 8

17 years ago
Created attachment 30274 [details] [diff] [review]
added nsprpub/pr/include/prtypes.h
(Reporter)

Comment 9

17 years ago
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.
(Reporter)

Comment 10

17 years ago
Created attachment 30280 [details] [diff] [review]
more callbacks
(Reporter)

Comment 11

17 years ago
NOTE: Latest patch supercedes all previous ones.  Based on 
PSM20_M_2_TAG.
(Reporter)

Comment 12

17 years ago
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

17 years ago
Milestone 2.0
Priority: -- → P2
Target Milestone: --- → 2.0
(Reporter)

Comment 14

17 years ago
Created attachment 31858 [details] [diff] [review]
PSM 2.0 only patch
(Reporter)

Comment 15

17 years ago
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.
(Reporter)

Updated

17 years ago
Depends on: 76900, 77199
(Reporter)

Comment 16

17 years ago
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?
(Assignee)

Comment 17

17 years ago
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

17 years ago
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.
(Reporter)

Comment 19

17 years ago
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?
(Reporter)

Comment 20

17 years ago
Created attachment 35506 [details] [diff] [review]
Patch diffed against latest code
(Assignee)

Comment 21

17 years ago
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

17 years ago
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

17 years ago
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.
(Assignee)

Comment 24

17 years ago
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

17 years ago
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.
(Assignee)

Comment 26

17 years ago
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=
(Assignee)

Comment 27

17 years ago
Created attachment 35623 [details] [diff] [review]
Updated patch. Removed makefile changes and use PR_CALLBACK instead of NSS_CALLBACK.

Comment 28

17 years ago
r=wtc.

Comment 29

17 years ago
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

17 years ago
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

17 years ago
> 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.
(Reporter)

Comment 32

17 years ago
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

17 years ago
PR_CALLBACK is intended for general use.

Comment 34

17 years ago
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
blizzard@mozilla.org, scc@mozilla.org, or brendan@mozilla.org

Feel free to forward the sr request to one of them.
(Reporter)

Comment 35

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

blizzard, can you super-review this?
sr=blizzard
(Assignee)

Comment 37

17 years ago
Checked in.
(Assignee)

Comment 38

17 years ago
Marking FIXED.
Status: NEW → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 39

17 years ago
Verified per ddrinan's comment.
Status: RESOLVED → VERIFIED

Updated

13 years ago
Component: Security: UI → Security: UI
Product: PSM → Core

Updated

9 years ago
Version: psm2.0 → 1.0 Branch
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.