Closed Bug 72693 Opened 24 years ago Closed 23 years ago

Build PSM 2.0 for OS/2

Categories

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

1.0 Branch
x86
OS/2
defect

Tracking

(Not tracked)

VERIFIED FIXED
psm2.0

People

(Reporter: jhpedemonte, Assigned: ddrinan0264)

References

Details

Attachments

(8 files)

Changes needed to allow PSM 2.0 to build under OS/2. This bug is for any fixes needed to get this going.
Attached patch Patch file for OS/2 changes (deleted) — Splinter Review
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?
Becouse OS/2 build copies many files from dist/ to dist/OBJxxxx/ Its ugly.
Attached patch latest patch (deleted) — Splinter Review
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.
Blocks: 74459
Thanks for the patch. I'm currently reviewing it.
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.
Attached patch more callbacks (deleted) — Splinter Review
NOTE: Latest patch supercedes all previous ones. Based on PSM20_M_2_TAG.
Could we at least get a review for nssbaset.h? Once that is checked in, then the rest can go in piece by piece.
Milestone 2.0
Priority: -- → P2
Target Milestone: --- → 2.0
Attached patch PSM 2.0 only patch (deleted) — Splinter Review
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.
Depends on: 76900, 77199
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?
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.
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.
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?
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.
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?
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.
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=.
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.
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=
r=wtc.
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.
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)
> 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.
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.
PR_CALLBACK is intended for general use.
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.
well, i'm glad to give javi an ego boost! blizzard, can you super-review this?
sr=blizzard
Checked in.
Marking FIXED.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Verified per ddrinan's comment.
Status: RESOLVED → VERIFIED
Product: PSM → Core
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.

Attachment

General

Created:
Updated:
Size: