544450 - NSS softoken fails to link in latest mozilla-central build environment

Status: RESOLVED DUPLICATE of bug 519550

(NSS :: Libraries, defect)

Description

[Kai Engert (:KaiE:)]

I've described the problem in detail in bug 527659 comment 38.

I don't see a solution how the problem could be overridden without having to change softoken files.

I initially tried to make the build variable check within security/nss/lib/softoken/config.mk but it didn't work, at time of link execution the variable was empty.

Therefore I moved the variable check and definition to file security/coreconf/location.mk

Comment 1

[Kai Engert (:KaiE:)]

Attachment: NSS portion patch, v1

Comment 2

[Kai Engert (:KaiE:)]

Comment on attachment 425417 [details] [diff] [review]
NSS portion patch, v1

I should have ran the diff inside a NSS sandbox, not in a mozilla sandbox, the "old" lines in NSS 3.12.6 no longer contain $(SQLITE), but rather -sqlite3

My proposed change to softoken/config.mk diffed against NSS looks like this:


Index: config.mk
===================================================================
RCS file: /cvsroot/mozilla/security/nss/lib/softoken/config.mk,v
retrieving revision 1.29
diff -u -u -r1.29 config.mk
--- config.mk   11 Jun 2009 00:55:48 -0000      1.29
+++ config.mk   5 Feb 2010 08:55:28 -0000
@@ -57,7 +57,7 @@
        -L$(DIST)/lib \
        -L$(NSSUTIL_LIB_DIR) \
        -lnssutil3 \
-       -lsqlite3 \
+       -l$(SQLITE_LIB_NAME) \
        -L$(NSPR_LIB_DIR) \
        -lplc4 \
        -lplds4 \
@@ -66,7 +66,7 @@
 else # ! NS_USE_GCC

 EXTRA_SHARED_LIBS += \
-       $(DIST)/lib/sqlite3.lib \
+       $(DIST)/lib/$(SQLITE_LIB_NAME).lib \
        $(DIST)/lib/nssutil3.lib \
        $(NSPR_LIB_DIR)/$(NSPR31_LIB_PREFIX)plc4.lib \
        $(NSPR_LIB_DIR)/$(NSPR31_LIB_PREFIX)plds4.lib \
@@ -83,7 +83,7 @@
        -L$(DIST)/lib \
        -L$(NSSUTIL_LIB_DIR) \
        -lnssutil3 \
-       -lsqlite3 \
+       -l$(SQLITE_LIB_NAME) \
        -L$(NSPR_LIB_DIR) \
        -lplc4 \
        -lplds4 \

Comment 3

[Wan-Teh Chang]

The SQLITE makefile variable is the patch that Ted "luser" checked
in to mozilla-central with the NSS team's permission.  When we
update the NSS in mozilla-central, we need to re-apply Ted's patch.

I do agree using a better variable name than SQLITE.  I had the
same idea as your patch before, so I support using SQLITE_LIB_NAME
(I actually considered using the longer SQLITE_LIBRARY_NAME
because it correspods to the existing LIBRARY_NAME variable, see
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/sqlite/manifest.mn&rev=1.4&mark=41#37
) and
excluding the -l flag from the value of this variable.

Comment 4

[Robert Relyea]

I agree with this variable name. Trunk check-in still needs to be coordinated with or FIPS review with the lab.

This bug is high priority to have the lab sign-off on.

bob

Comment 5

[Wan-Teh Chang]

Kai: let's create a convention for storing Mozilla-specific
patches for the NSS in mozilla-central, so that we can easily
re-apply them when we update NSS in mozilla-central.

I suggest a "patches" or "mozilla-patches" directory in the
mozilla/security/nss directory.

Re: variable name: let's go with SQLITE_LIB_NAME that you use.
(I found that LIBRARY_NAME doesn't include the version
number, so it's pointless to try to match it.)

Comment 6

[Wan-Teh Chang]

Kai: Another idea is a "nss-patches" directory in the mozilla/security
directory in mozilla-central.

Comment 7

[Kai Engert (:KaiE:)]

I apologize, I didn't know about bug 519550, thanks for making me aware. I didn't know that we have already applied a patch on top of NSS in mozilla-central.

I understand your desire to improve the name of the variable, currently SQLITE, proposed SQLITE_LIB_NAME.

But we aren't introducing this variable, it's already there. It's already being used in cmd/platlibs.mk

Do you really want me to use a patch, which does renaming of existing build variables?

I'd prefer, at least for this initial landing, to go the simple path, keep the existing patch in PSM, keep the existing variables as is in platlibs.mk, and merge the patch from bug 519550.

Comment 9

[Kai Engert (:KaiE:)]

Wan-Teh, thanks for your idea to keep the necessary patch in cvs or hg.

Keeping the patch inside NSS-cvs might not be ideal, because different mozilla branches may require different patches? Therefore I prefer the idea to keep the patches (or infomation) in the respective mozilla branch.

However, I'm worried that it won't help us that much. The patches will often have to be merged when upgrading to a newer NSS snapshot. That means merging and patch updating.

Therefore I'd like to propose a simpler approach:

What about
  mozilla/security/README.NSS-patches

A simple text file that gives information about the currently imported NSS snapshot, and a list of all bug numbers + patch names (or direct links to attachment in bugzilla) of the patches applied on top of it.

Comment 10

[Robert Relyea]

Comment on attachment 425417 [details] [diff] [review]
NSS portion patch, v1

This patch looks right, but as commented earlier should not be applied to the NSS trunk (at least not yet).

Cancelling the review. If you need one for wherever the mozilla patches go, I'll be happy to provide it.

bob

Comment 11

[Wan-Teh Chang]

Attachment: NSS portion patch, v2

This patch includes the changes to nss/cmd/platlibs.mk, too.

The changes to coreconf/location.mk and nss/cmd/platlibs.mk can be
checked in first, before the changes to nss/lib/softoken/config.mk
are checked in.

Comment 12

[Wan-Teh Chang]

Kai: I suggest creating a
  mozilla/security/nss-patches
directory, and add a README file in there to describe the patches.

It'll be easy to maintain the patches because there should not
be many.

Comment 13

[Wan-Teh Chang]

Comment on attachment 426415 [details] [diff] [review]
NSS portion patch, v2

I improved this patch and moved it to bug 519550.