Closed
Bug 544450
Opened 15 years ago
Closed 15 years ago
NSS softoken fails to link in latest mozilla-central build environment
Categories
(NSS :: Libraries, defect)
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 519550
People
(Reporter: KaiE, Unassigned)
References
Details
Attachments
(2 obsolete files)
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
Reporter | ||
Comment 1•15 years ago
|
||
Attachment #425417 -
Flags: review?(rrelyea)
Reporter | ||
Comment 2•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
Kai: Another idea is a "nss-patches" directory in the mozilla/security
directory in mozilla-central.
Reporter | ||
Comment 7•15 years ago
|
||
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.
Reporter | ||
Updated•15 years ago
|
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → DUPLICATE
Reporter | ||
Comment 9•15 years ago
|
||
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•15 years ago
|
||
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
Attachment #425417 -
Flags: review?(rrelyea)
Comment 11•15 years ago
|
||
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.
Attachment #425417 -
Attachment is obsolete: true
Comment 12•15 years ago
|
||
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•15 years ago
|
||
Comment on attachment 426415 [details] [diff] [review]
NSS portion patch, v2
I improved this patch and moved it to bug 519550.
Attachment #426415 -
Attachment is obsolete: true
You need to log in
before you can comment on or make changes to this bug.
Description
•