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)

3.12.6
x86
Linux
defect
Not set
normal

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
Blocks: 527659
Attached patch NSS portion patch, v1 (obsolete) — Splinter Review
Attachment #425417 - Flags: review?(rrelyea)
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 \
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.
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
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.)
Kai: Another idea is a "nss-patches" directory in the mozilla/security directory in mozilla-central.
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.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → DUPLICATE
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 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)
Attached patch NSS portion patch, v2 (obsolete) — Splinter Review
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
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 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.

Attachment

General

Created:
Updated:
Size: