Closed Bug 114748 Opened 23 years ago Closed 22 years ago

Using LIB_PREFIX in Makefiles

Categories

(NSS :: Build, defect, P3)

3.3.1
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jhpedemonte, Assigned: nelson)

Details

Attachments

(4 files, 9 obsolete files)

3.20 KB, patch
Details | Diff | Splinter Review
16.37 KB, patch
Details | Diff | Splinter Review
941 bytes, patch
Details | Diff | Splinter Review
4.02 KB, patch
netscape
: review+
Details | Diff | Splinter Review
This is an extension of defect 58981.  We are now using the makefile 
variable LIB_PREFIX in the mozilla tree.  This defect is to use it 
appropriately in the NSS build.
Attached patch patch (obsolete) — Splinter Review
This patch includes needed changes in security/coreconf in order to build OS/2
NSS after the changes from bug 58981.  However, I went further and set about
cleaning up some of the makefiles, using LIB_PREFIX where appropriate.	Some of
these can be cleanup up further, but I didn't feel comfortable making more
changes.
Attached patch better patch (obsolete) — Splinter Review
I do want to set LIB_PREFIX=lib for OS/2 EMX/GCC builds, but not for VACPP.
Attachment #61358 - Attachment is obsolete: true
Javier, I went through your patch very quickly.  It cleans
up our makefiles and is the right thing to do.  Thanks.

We will need to carefully review it again to verify the
equivalence of the new code to the original code.
Status: NEW → ASSIGNED
Priority: -- → P3
Target Milestone: --- → 3.4
Version: unspecified → 3.3.1
Yes, this is precisely the thing we need in NSS. The changes are for the NSS 3.3
branch, and would have to be merged for the NSS 3.4 tip. NSS 3.4 makes softoken
a shared library, and renames it to softokn3. It's important because PSM is
using the shared librarie version when it moves to 3.4.
Is there a quick way to apply the patch recursively to all directories involved
at once, without having to cut the patch file into x part and going into x
directories ? GNU patch 2.5 which is what's used in the OS/2 build doesn't seem
to be able to do the trick.
Even when using the long method I described, the patches failed to apply. NSS
3.4 is still in development and there were multiple conflicts in the files that
you generated the patch for on my OS/2 tree pulled today.
I didn't realize that the patch for NSS 3.3, that certainly explains why the
patch didn't merge. FYI, the browser doesn't build right now with SET NSS_3_4=1
on OS/2 because PSM is looking for libsoftoken.lib . Once these changes are
completed to work with softoken and ported to the tip of NSS, I believe it will
all work. I'm willing to spend the time to try the changes on OS/2 in the very
short term we have left before the RTM of NSS 3.4 .
Attached patch NSS 3.4 patch (obsolete) — Splinter Review
This patch was created against NSS 3.4.  There are still some areas that could
be cleaned up, such as where we take one path for WIN32 and one for everything
else.  I wasn't quite sure what to do in those cases, so I didn't touch it.  
This patch has only been compiled on OS/2.  If I have some time tomorrow, I'll
try it on Linux.
Attachment #61364 - Attachment is obsolete: true
FYI, this patch clashes strongly with a set of patches I have to enable
coreconf to be used for cross-platform development.  I'm waiting for
our "tinderbox" (continuous repetitive) build system to be working again
before I check in that set of patches.  

Here a question about patch 67336:

Does it change (interntionally or otherwise) the names of the libs, 
dlls, dsos, or "import"/"stub" libraries on any platform?  
(any such change will create an incompatibility)

The main purpose of this patch is to make OS/2 libs NOT have "lib" prefixed to
libnames ("libname.lib" -> "name.lib").  It should also work for WIN32, if that
is what they want.  For all other operating systems, the lib names still remain
"libname.so".
Just let me know when you check in those patches, and I will refresh this one.
Since 3.4 is going to land today, what is the possibility of getting this
checked in?  OS/2 is broke without this.
Javier,

We are not planning to check in this patch before the NSS 3.4 landing.

Do you have a simpler patch that will make NSS 3.4 build on OS/2?
Comment on attachment 67336 [details] [diff] [review]
NSS 3.4 patch

I don't agree that fixing the OS/2 build problem should wait until after the
3.4 landing seeing as they did do the legwork to get it in prior to the 3.4
landing.  However, the patch doesn't build on linux.  It's looking for a
dbm.lib instead of a dbm.a.  I have a feeling that most of the hardcoded .lib
instances should be $(LIB_SUFFIX)
Attached patch NSS 3.4 patch, builds on Linux (obsolete) — Splinter Review
This patch properly builds on Linux and OS/2 for NSS 3.4
Attachment #67336 - Attachment is obsolete: true
Attached patch updated patch (obsolete) — Splinter Review
Updated patch to trunk following landing of NSS 3.4.  Can someone review this?
Attachment #68034 - Attachment is obsolete: true
I am reviewing your changes now, and have some comments and questions.
I'll email you my questions.  
Attached patch patch with nelsonb's suggestions (obsolete) — Splinter Review
This patch includes changes as suggested by nelsonb.  Have not tested it on
anything other than OS/2, yet.
Attachment #68426 - Attachment is obsolete: true
IINM, there are still several places in the new patch where names of 
libraries to be linked end in .lib rather than in .$(LIB_SUFFIX)
e.g., in smime/config.mk

 SHARED_LIBRARY_LIBS = \
-       $(DIST)/lib/libpkcs12.${LIB_SUFFIX} \
-       $(DIST)/lib/libpkcs7.${LIB_SUFFIX} \
+       $(DIST)/lib/$(LIB_PREFIX)pkcs12.lib \
+       $(DIST)/lib/$(LIB_PREFIX)pkcs7.lib \
        $(NULL)

Now that SHARED_LIBRARY_LIBS are common to all builds, rather than ifdeffed
by platform, they need to all use $(LIB_SUFFIX)

Will it be possible for you to test this on any **ix platform, e.g. Linux
or AIX?
Attached patch fix LIB_SUFFIX issue (obsolete) — Splinter Review
This corrected patch builds on OS/2 and Linux.	Thanks, Nelson...
Attachment #68561 - Attachment is obsolete: true
Comment on attachment 68631 [details] [diff] [review]
fix LIB_SUFFIX issue

1. security/coreconf/ruleset.mk

>+	LIBRARY        = $(OBJDIR)/$(LIB_PREFIX)$(LIBRARY_NAME).$(LIB_SUFFIX)
>+	SHARED_LIBRARY = $(OBJDIR)/$(LIB_PREFIX)$(LIBRARY_NAME)$(LIBRARY_VERSION)$(JDK_DEBUG_SUFFIX).$(DLL_SUFFIX)

The SHARED_LIBRARY variable should use $(DLL_PREFIX) instead of $(LIB_PREFIX).

>+	ifeq (,$(filter-out WIN%,$(OS_TARGET)))
> 		SHARED_LIBRARY = $(OBJDIR)/$(LIBRARY_NAME)$(LIBRARY_VERSION)32$(JDK_DEBUG_SUFFIX).dll
> 		IMPORT_LIBRARY = $(OBJDIR)/$(LIBRARY_NAME)$(LIBRARY_VERSION)32$(JDK_DEBUG_SUFFIX).lib

Nelson, we should take this opportunity to get rid of the "32" in our
Windows DLL and import library names.  They don't apply to WIN16 and WINCE
and both NSS and JSS override the default DLL and import library names
anyway.  (DBM, the only other user of coreconf, does not have a DLL.)

>+	ifeq ($(OS_CONFIG), AIX4.1)
>+		SHARED_LIBRARY = $(OBJDIR)/lib$(LIBRARY_NAME)$(LIBRARY_VERSION)_shr$(JDK_DEBUG_SUFFIX).a
>+	endif

I would replace $(OS_CONFIG) by $(OS_TARGET)$(OS_RELEASE).  I would like to
eventually get rid of the OS_CONFIG make variable.

2. security/nss/lib/fortcrypt/swfort/pkcs11/Makefile

>-LIBCI_JAR               = $(OBJDIR)/lib$(LIBRARY_NAME).jar
>+LIBCI_JAR               = $(OBJDIR)/$(LIB_PREFIX)$(LIBRARY_NAME).jar

This change should be removed.	This is the name of a jar file, not a
static library.

>-#$(SHARED_LIBRARY): $(SWCILIB)

I would leave all the commented-out SWCILIB stuff alone as documentation.

All the other changes look fine, although I have not reviewed them very
carefully.

Nelson, you also need to make similar changes to nss/cmd/platlibs.mk.
The Mozillla client only builds nss/lib, which is why Javier's patch
does not change nss/cmd/platlibs.mk.
Attachment #68631 - Flags: needs-work+
Assigned the bug to Nelson because he will merge this patch
with his cross-compilation patches.
Assignee: wtc → nelsonb
Status: ASSIGNED → NEW
Nelson, I attached my patch for nss/cmd/platlibs.mk for your
reference.
Javier's patch included modifications to mozilla/config/config.mk and 
mozilla/security/manager/Makefile.in.  Those two files are not part of 
NSS, and are subject to different checkin controls than are the NSS files
in mozilla/security/coreconf and mozilla/security/nss, so I did not checkin
any changes to those two files.  

I took Javier's two patches above (minus the two files not in NSS), 
and edited them per Wan-Teh's suggestions, and applied them to a fresh
workarea.  I built that workarea using IRIX and AIX, and ran the test
script on IRIX.  When it passed, I checked in those changes.  
I am watching NSS's tinderbox now.  

Wan-Teh suggested some additional changes, such as getting rid of the 
suffix "32" from Windows library builds (now that we no longer support
16-bit windows), and changing all $(OS_CONFIG) to $(OS_TARGET)$(OS_RELEASE).
I will incorporate those changes into my upcoming big set of coreconf 
changes for cross-platform building.
All the tinderbox builds have gone green except NT 4.0 and Linux 2.4.
The build system for Linux 2.4 is apparently dead suince 13:30 today.
The NT system has now not completed a build in almost 3 hours, which 
is over twice as long as it normally takes, so I suspect it is dead too.
I have succesfully built the tip on my own NT system, so I consider 
that platform green too.

Someone else should checkin the non-NSS parts of Javier's patch.
Status: NEW → ASSIGNED
These are the remaining two files to check in.	They are not in the NSS
component.  Who handles this checkin?
Attachment #68631 - Attachment is obsolete: true
Comment on attachment 68904 [details] [diff] [review]
config/config.mk & security/manager/Makefile.in : remaining files

I just updated NSS_CLIENT_TAG, so the Mozilla client's version of NSS
has Javier's LIB_PREFIX changes now.

I was planning to check in Javier's change for the remaining, non-NSS
files, but I found that mozilla/config/config.mk has changed underneath
us.  It seems that if you are not doing static builds on OS/2, you just
need to apply Javier's patch to mozilla/security/manager/Makefile.in.
Please let me know if that works.

Until then, I believe the OS/2 Mozilla client build will break.  Sorry.
This is the updated version of Javier's patch.	Both
files have changed since Javier generated his patch
(attachment 68904 [details] [diff] [review]).
Attachment #68904 - Attachment is obsolete: true
Chris, could you review attachment 69311 [details] [diff] [review]?  Thanks.
Comment on attachment 69311 [details] [diff] [review]
config/config.mk & security/manager/Makefile.in: remaining files

Why are we changing the non-NSS_3_4 section?  It should be removed all together
or left alone since NSS wasn't fixed prior to 3.4.  

Also, is this going to conflict with the proper fix for bug 124211 ?
wtc:  Attachment 69311 [details] [diff] builds and works on OS/2.  

Regarding seawood's comments, the only conflict I see with 124211 is it's use of
NSS_LIB_PREFIX.  Once this gets checked in, we should change that patch.  
Chris,

Good catch.  As you pointed out, security/manager/ssl/src/Makefile.in
is also using NSS_LIB_PREFIX and needs to be fixed as well.

I deleted the non-NSS 3.4 section in config/config.mk.	Please review
the new patch.	Thanks.
Attachment #69311 - Attachment is obsolete: true
Comment on attachment 69372 [details] [diff] [review]
Remaining files: remove NSS_LIB_PREFIX

r=cls
Attachment #69372 - Flags: review+
The patch for the remaining files has been checked in.
The OS/2 tinderbox is green now.  Marked the bug fixed.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: