Last Comment Bug 438876 - signtool is still using static libraries.
: signtool is still using static libraries.
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Tools (show other bugs)
: 3.4
: All All
: P2 normal (vote)
: 3.12.1
Assigned To: Robert Relyea
:
Mentors:
: 263948 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-12 10:35 PDT by Robert Relyea
Modified: 2008-07-18 12:04 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Move signtool to use exported functions. Export the one missing functions it needs. (5.94 KB, patch)
2008-06-13 11:43 PDT, Robert Relyea
wtc: review+
Details | Diff | Splinter Review
fix build on window, v1 (1.16 KB, patch)
2008-06-30 13:59 PDT, Nelson Bolyard (seldom reads bugmail)
rrelyea: review+
Details | Diff | Splinter Review

Description Robert Relyea 2008-06-12 10:35:59 PDT
Signtool is really the last general tool we build today that statically links directly with NSS. (Tools like blapitest are exempted since they are unit test tools).

It should be modified to use shared libaries.
Comment 1 Nelson Bolyard (seldom reads bugmail) 2008-06-13 09:14:49 PDT
This seems like a good "first bug" for an aspiring NSS developer.
A good first step would be to remove the line that reads:

USE_STATIC_LIBS = 1

from cmd/signtool/manifest.mn and then try to build it.  If it succeeds,
the job is done.  If it fails, it would be good to note here in this bug
why it fails (what symbols are missing)..
Comment 2 Robert Relyea 2008-06-13 11:43:53 PDT
Created attachment 325002 [details] [diff] [review]
Move signtool to use exported functions. Export the one missing functions it needs.
Comment 3 Wan-Teh Chang 2008-06-20 22:43:24 PDT
Comment on attachment 325002 [details] [diff] [review]
Move signtool to use exported functions. Export the one missing functions it needs.

r=wtc.  Some comments and suggested changes below.

1. security/nss/cmd/signtool/certgen.c

The old code reads better.  The new code seems a little
roundabout.  It imports the DER cert, and then looks up
the cert by the DER cert.

2. security/nss/cmd/signtool/manifest.mn

We should remove the "USE_STATIC_LIBS = 1" line rather
than commenting it out.

3. security/nss/cmd/signtool/util.c

This is a minor problem of the original code.  It seems
more thread-safe to move

834     modules = SECMOD_GetDefaultModuleList();
835 
836     if (modules == NULL) {
837         PR_fprintf(errorFD, "%s: Can't get module list\n", PROGRAM_NAME);
838         errorCount++;
839         exit (ERRX);
840     }

after

850     SECMOD_GetReadLock (moduleLock);

We'd need to release the read lock before exit (ERRX).

4. Remove the declaration of PK11_FindCertFromDERCertItem
from pk11priv.h.
Comment 4 Slavomir Katuscak 2008-06-30 08:04:03 PDT
Bob, seems that your patch causes failures in Windows build:

cl WINNT5.2_OPT.OBJ\\signtool.obj WINNT5.2_OPT.OBJ\\certgen.obj WINNT5.2_OPT.OBJ\\javascript.obj WINNT5.2_OPT.OBJ\\list.obj WINNT5.2_OPT.OBJ\\sign.obj WINNT5.2_OPT.OBJ\\util.obj WINNT5.2_OPT.OBJ\\verify.obj WINNT5.2_OPT.OBJ\\zip.obj -FeWINNT5.2_OPT.OBJ/signtool.exe -link  ..\\..\\..\\..\\dist\\WINNT5.2_OPT.OBJ\\lib\\jar.lib ..\\..\\..\\..\\dist\\WINNT5.2_OPT.OBJ\\lib\\zlib.lib  ..\\..\\..\\..\\dist\\WINNT5.2_OPT.OBJ\\lib\\sectool.lib ..\\..\\..\\..\\dist\\WINNT5.2_OPT.OBJ\\lib\\nssutil3.lib ..\\..\\..\\..\\dist\\WINNT5.2_OPT.OBJ\\lib\\smime3.lib ..\\..\\..\\..\\dist\\WINNT5.2_OPT.OBJ\\lib\\ssl3.lib ..\\..\\..\\..\\dist\\WINNT5.2_OPT.OBJ\\lib\\nss3.lib ..\\..\\..\\..\\dist\\WINNT5.2_OPT.OBJ\\lib\\libplc4.lib ..\\..\\..\\..\\dist\\WINNT5.2_OPT.OBJ\\lib\\libplds4.lib ..\\..\\..\\..\\dist\\WINNT5.2_OPT.OBJ\\lib\\libnspr4.lib   
Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 15.00.21022.08 for 80x86
Copyright (C) Microsoft Corporation.  All rights reserved.

Microsoft (R) Incremental Linker Version 9.00.21022.08
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:WINNT5.2_OPT.OBJ/signtool.exe 
..\..\..\..\dist\WINNT5.2_OPT.OBJ\lib\jar.lib 
..\..\..\..\dist\WINNT5.2_OPT.OBJ\lib\zlib.lib 
..\..\..\..\dist\WINNT5.2_OPT.OBJ\lib\sectool.lib 
..\..\..\..\dist\WINNT5.2_OPT.OBJ\lib\nssutil3.lib 
..\..\..\..\dist\WINNT5.2_OPT.OBJ\lib\smime3.lib 
..\..\..\..\dist\WINNT5.2_OPT.OBJ\lib\ssl3.lib 
..\..\..\..\dist\WINNT5.2_OPT.OBJ\lib\nss3.lib 
..\..\..\..\dist\WINNT5.2_OPT.OBJ\lib\libplc4.lib 
..\..\..\..\dist\WINNT5.2_OPT.OBJ\lib\libplds4.lib 
..\..\..\..\dist\WINNT5.2_OPT.OBJ\lib\libnspr4.lib 
WINNT5.2_OPT.OBJ\signtool.obj 
WINNT5.2_OPT.OBJ\certgen.obj 
WINNT5.2_OPT.OBJ\javascript.obj 
WINNT5.2_OPT.OBJ\list.obj 
WINNT5.2_OPT.OBJ\sign.obj 
WINNT5.2_OPT.OBJ\util.obj 
WINNT5.2_OPT.OBJ\verify.obj 
WINNT5.2_OPT.OBJ\zip.obj 
certgen.obj : error LNK2019: unresolved external symbol _CERT_CertificateTemplate referenced in function _sign_cert
WINNT5.2_OPT.OBJ/signtool.exe : fatal error LNK1120: 1 unresolved externals
make[3]: *** [WINNT5.2_OPT.OBJ/signtool.exe] Error 2

Please back out, or fix it ASAP. Thanks.
Comment 5 Nelson Bolyard (seldom reads bugmail) 2008-06-30 12:22:13 PDT
Slavo, How would Bob know?
Tinderbox on Windows has not been running since June 20 at 06:38 PDT.
That's 10 days ago.  Please get it running again ASAP.
Comment 6 Nelson Bolyard (seldom reads bugmail) 2008-06-30 13:35:04 PDT
I restored USE_STATIC_LIBS = 1 to manifest.mn to get this building
on Windows again, until this can be fixed properly.
Checking in manifest.mn; new revision: 1.7; previous revision: 1.6
Comment 7 Nelson Bolyard (seldom reads bugmail) 2008-06-30 13:59:12 PDT
Created attachment 327473 [details] [diff] [review]
fix build on window, v1

Bob, please review.
Comment 8 Robert Relyea 2008-06-30 14:09:48 PDT
Comment on attachment 327473 [details] [diff] [review]
fix build on window, v1

r+ rrelyea
!@#!@$! took all morning to get my windows machine to get a build...

This is the correct fix.
Comment 9 Nelson Bolyard (seldom reads bugmail) 2008-06-30 14:28:11 PDT
Checking in certgen.c;   new revision: 1.12; previous revision: 1.11
Checking in manifest.mn; new revision: 1.8; previous revision: 1.7
Comment 10 Julien Pierre 2008-07-18 12:04:24 PDT
*** Bug 263948 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.