Split the nssckbi target off from the 'nss' target.

nss.gyp

Index: nss.gyp
diff --git a/nss.gyp b/nss.gyp
index a24c6cef25b0d88b229565d9a6c27b8a6ac0ecd4..71c9ad3891269003d0a59f0823a1ff7cd96df043 100644
--- a/nss.gyp
+++ b/nss.gyp
@@ -245,10 +245,16 @@
             'HAVE_BSD_FLOCK',
             'HAVE_CRT_EXTERNS_H',
             'HAVE_DLADDR',
-            'HAVE_LCHOWN',

[wtc, 2012/05/15 18:03:47]

I would add this macro back. This is a faithful imitation
of the upstream NSPR build system.  HAVE_LCHOWN is tested
by a file not used by Chromium.

             'HAVE_SOCKLEN_T',
             'HAVE_STRERROR',
           ],
+          'direct_dependent_settings': {
+            'defines': [
+              'XP_UNIX',
+              'DARWIN',
+              'XP_MACOSX',

[Ryan Sleevi, 2012/05/15 04:23:39]

Since these all appear in (indirectly included) .h files, explicitly specifying
them here.

Previously this was handled transitively by each of the targets.

[wtc, 2012/05/15 18:03:47]

Please remove these direct_dependent_settings blocks for
NSPR.

A design goal of NSPR is that one should not need to define
any macro in order to include NSPR headers.  This is why
public NSPR headers are only allowed to test
compiler-predefined macros or macros defined in prcpucfg.h.

[Ryan Sleevi, 2012/05/15 19:38:47]

Ah, I missed that we had specially patched prcpucfg.h to work around the
configure step auto-generating.

[wtc, 2012/05/15 23:47:11]

prcpucfg.h is not generated by the configure script.
It comes from one of the _xxxos.cfg files in the
mozilla/nsprpub/pr/include/md directory:
http://mxr.mozilla.org/nspr/source/nsprpub/pr/include/md/

The macros from the configure script are defined on the
compiler command line and are not tested by NSPR public
header files.

[Ryan Sleevi, 2012/05/15 23:54:12]

I think we may be miscommunicating.

prcpucfg.h is not checked in to NSPR public. It's copied by the Makefile-based
compile step from one of the .cfg files - see 
http://mxr.mozilla.org/nspr/source/nsprpub/pr/include/md/Makefile.in

In our tree, we've checked in prcpucfg.h that just directly includes one of the
_xxxos.cfg files, for the platforms we support compiling NSS statically (OS X &
Win)

For other third-party packages, such as those based on config.h-and-friends,
we've done things like set the defines in the .GYP files, and just include an
empty config.h, which is why I missed that we'd modified prcpucfg.h

+            ],
+          },
           'sources/': [
             ['exclude', '^mozilla/nsprpub/pr/src/md/windows/'],
             ['exclude', '^mozilla/nsprpub/pr/src/threads/combined/'],
@@ -284,6 +290,13 @@
             '_CRT_SECURE_NO_WARNINGS',
             '_CRT_NONSTDC_NO_WARNINGS',
           ],
+          'direct_dependent_settings': {
+            'defines': [
+              'XP_PC',
+              'WIN32',
+              'WIN95',
+            ],
+          },

[Ryan Sleevi, 2012/05/15 04:23:39]

Same here

           'sources/': [
             ['exclude', '^mozilla/nsprpub/pr/src/md/unix/'],
             ['exclude', '^mozilla/nsprpub/pr/src/pthreads/'],
@@ -329,6 +342,108 @@
       'target_name': 'nss',
       'product_name': 'crnss',
       'type': '<(component)',
+      'dependencies': [
+        'nss_static',
+      ],
+      'export_dependent_settings': [
+        'nss_static',
+      ],
+      'sources': [
+        'mozilla/security/nss/lib/nss/nssver.c',

[Ryan Sleevi, 2012/05/15 04:23:39]

A bit of a hack to ensure at least one symbol is linked in to the target. MSVC
gets grumpy if a target has no linkables but tries to produce a DLL (line 344)

+      ],
+      'conditions': [
+        ['exclude_nss_root_certs==0', {
+          'dependencies': [
+            'nssckbi',
+          ],
+          'export_dependent_settings': [
+            'nssckbi',

[wtc, 2012/05/15 18:03:47]

This export_dependent_settings block may not be necessary
because nssckbi should not be used directly.  It has just
a public header nssckbi.h, which doesn't test any macros.

[Ryan Sleevi, 2012/05/15 19:38:47]

If "nssckbi.h" is part of the public interface of NSS (and it does seem to be),
then generally we should be exporting the settings for it.

That said, nssckbi.h lives in lib/ckfw/builtins/, whereas the 'nssckbi' target
is exporting as an include dir lib/ckfw. I've fixed it such that the proper
builtins/ dir is exported (which wasn't happening in the old code - line 913-930
of old), which at least ensures that callers can #include "nssckbi.h"

[wtc, 2012/05/15 23:47:11]

nssckbi.h is an NSS public header, but Chromium doesn't use
it and I think Chromium should not use it.  So I suggest not
exposing nssckbi.h to NSS consumers in the Chromium source
tree.

(Mozilla uses nssckbi.h because Mozilla loads nssckbi.dll
using a different method and needs to know the version of
nssckbi.dll.)

[Ryan Sleevi, 2012/05/15 23:54:12]

If it's all the same, I'd rather keep the static-vs-dynamic linkage opaque to
the dependents, and provide as much as possible the same interfaces as upstream.
It also keeps consistency between Linux/ChromeOS and Mac/Win targets in terms of
what is visible header wise.

[wtc, 2012/05/16 00:07:40]

Yes, it is fine to export the dependent settings of nssckbi.

+          ],
+        }],
+        ['OS == "mac" and component == "shared_library"', {
+          'xcode_settings': {
+            'GCC_SYMBOLS_PRIVATE_EXTERN': 'NO',  # no -fvisibility=hidden
+          },
+        }],
+        ['OS == "win" and component == "shared_library"', {
+          'sources': [
+            'mozilla/security/nss/exports_win.def',
+          ],
+        }],
+      ],
+    },
+    {
+      # This is really more of a pseudo-target to work around the fact that
+      # a single static_library target cannot contain two object files of the
+      # same name (hash.o / hash.obj). Logically, this is part of the
+      # 'nss_static' target. By separating it out, it creates a possible
+      # circular dependency between 'nss_static' and 'nssckbi' when
+      # 'exclude_nss_root_certs' is not specified, as 'nss_static' depends on
+      # the 'builtinsC_GetFunctionList' exported by this target. This is an
+      # artifact of how NSS is being statically built, which is not an
+      # officially supported configuration - normally, 'nssckbi.dll/so' would
+      # depend on libnss3.dll/so, and the higher layer caller would instruct
+      # libnss3.dll to dynamically load nssckbi.dll, breaking the circle.
+      #
+      # TODO(rsleevi): http://crbug.com/128134 - Break the circular dependency
+      # without requiring nssckbi to be built as a shared library.
+      'target_name': 'nssckbi',
+      'product_name': 'crnssckbi',
+      'type': 'static_library',
+      'sources': [
+        'mozilla/security/nss/lib/ckfw/builtins/anchor.c',
+        'mozilla/security/nss/lib/ckfw/builtins/bfind.c',
+        'mozilla/security/nss/lib/ckfw/builtins/binst.c',
+        'mozilla/security/nss/lib/ckfw/builtins/bobject.c',
+        'mozilla/security/nss/lib/ckfw/builtins/bsession.c',
+        'mozilla/security/nss/lib/ckfw/builtins/bslot.c',
+        'mozilla/security/nss/lib/ckfw/builtins/btoken.c',
+        'mozilla/security/nss/lib/ckfw/builtins/builtins.h',
+        'mozilla/security/nss/lib/ckfw/builtins/certdata.c',
+        'mozilla/security/nss/lib/ckfw/builtins/ckbiver.c',
+        'mozilla/security/nss/lib/ckfw/builtins/constants.c',
+        'mozilla/security/nss/lib/ckfw/builtins/nssckbi.h',
+        'mozilla/security/nss/lib/ckfw/ck.h',
+        'mozilla/security/nss/lib/ckfw/ckfw.h',
+        'mozilla/security/nss/lib/ckfw/ckfwm.h',
+        'mozilla/security/nss/lib/ckfw/ckfwtm.h',
+        'mozilla/security/nss/lib/ckfw/ckmd.h',
+        'mozilla/security/nss/lib/ckfw/ckt.h',
+        'mozilla/security/nss/lib/ckfw/crypto.c',
+        'mozilla/security/nss/lib/ckfw/find.c',
+        'mozilla/security/nss/lib/ckfw/hash.c',
+        'mozilla/security/nss/lib/ckfw/instance.c',
+        'mozilla/security/nss/lib/ckfw/mechanism.c',
+        'mozilla/security/nss/lib/ckfw/mutex.c',
+        'mozilla/security/nss/lib/ckfw/nssck.api',
+        'mozilla/security/nss/lib/ckfw/nssckepv.h',
+        'mozilla/security/nss/lib/ckfw/nssckft.h',
+        'mozilla/security/nss/lib/ckfw/nssckfw.h',
+        'mozilla/security/nss/lib/ckfw/nssckfwc.h',
+        'mozilla/security/nss/lib/ckfw/nssckfwt.h',
+        'mozilla/security/nss/lib/ckfw/nssckg.h',
+        'mozilla/security/nss/lib/ckfw/nssckmdt.h',
+        'mozilla/security/nss/lib/ckfw/nssckt.h',
+        'mozilla/security/nss/lib/ckfw/object.c',
+        'mozilla/security/nss/lib/ckfw/session.c',
+        'mozilla/security/nss/lib/ckfw/sessobj.c',
+        'mozilla/security/nss/lib/ckfw/slot.c',
+        'mozilla/security/nss/lib/ckfw/token.c',
+        'mozilla/security/nss/lib/ckfw/wrap.c',
+      ],
+      'dependencies': [
+        'nss_static',
+      ],
+      'export_dependent_settings': [
+        'nss_static',
+      ],
+      'include_dirs': [
+        'mozilla/security/nss/lib/ckfw',
+      ],
+    },
+    {
+      'target_name': 'nss_static',
+      'type': 'static_library',
       'sources': [
         'mozilla/security/nss/lib/base/arena.c',
         'mozilla/security/nss/lib/base/base.h',
@@ -376,45 +491,6 @@
         'mozilla/security/nss/lib/certhigh/ocspt.h',
         'mozilla/security/nss/lib/certhigh/ocspti.h',
         'mozilla/security/nss/lib/certhigh/xcrldist.c',
-        'mozilla/security/nss/lib/ckfw/builtins/anchor.c',
-        'mozilla/security/nss/lib/ckfw/builtins/bfind.c',
-        'mozilla/security/nss/lib/ckfw/builtins/binst.c',
-        'mozilla/security/nss/lib/ckfw/builtins/bobject.c',
-        'mozilla/security/nss/lib/ckfw/builtins/bsession.c',
-        'mozilla/security/nss/lib/ckfw/builtins/bslot.c',
-        'mozilla/security/nss/lib/ckfw/builtins/btoken.c',
-        'mozilla/security/nss/lib/ckfw/builtins/builtins.h',
-        'mozilla/security/nss/lib/ckfw/builtins/certdata.c',
-        'mozilla/security/nss/lib/ckfw/builtins/ckbiver.c',
-        'mozilla/security/nss/lib/ckfw/builtins/constants.c',
-        'mozilla/security/nss/lib/ckfw/builtins/nssckbi.h',
-        'mozilla/security/nss/lib/ckfw/ck.h',
-        'mozilla/security/nss/lib/ckfw/ckfw.h',
-        'mozilla/security/nss/lib/ckfw/ckfwm.h',
-        'mozilla/security/nss/lib/ckfw/ckfwtm.h',
-        'mozilla/security/nss/lib/ckfw/ckmd.h',
-        'mozilla/security/nss/lib/ckfw/ckt.h',
-        'mozilla/security/nss/lib/ckfw/crypto.c',
-        'mozilla/security/nss/lib/ckfw/find.c',
-        'mozilla/security/nss/lib/ckfw/hash.c',
-        'mozilla/security/nss/lib/ckfw/instance.c',
-        'mozilla/security/nss/lib/ckfw/mechanism.c',
-        'mozilla/security/nss/lib/ckfw/mutex.c',
-        'mozilla/security/nss/lib/ckfw/nssck.api',
-        'mozilla/security/nss/lib/ckfw/nssckepv.h',
-        'mozilla/security/nss/lib/ckfw/nssckft.h',
-        'mozilla/security/nss/lib/ckfw/nssckfw.h',
-        'mozilla/security/nss/lib/ckfw/nssckfwc.h',
-        'mozilla/security/nss/lib/ckfw/nssckfwt.h',
-        'mozilla/security/nss/lib/ckfw/nssckg.h',
-        'mozilla/security/nss/lib/ckfw/nssckmdt.h',
-        'mozilla/security/nss/lib/ckfw/nssckt.h',
-        'mozilla/security/nss/lib/ckfw/object.c',
-        'mozilla/security/nss/lib/ckfw/session.c',
-        'mozilla/security/nss/lib/ckfw/sessobj.c',
-        'mozilla/security/nss/lib/ckfw/slot.c',
-        'mozilla/security/nss/lib/ckfw/token.c',
-        'mozilla/security/nss/lib/ckfw/wrap.c',
         'mozilla/security/nss/lib/cryptohi/cryptohi.h',
         'mozilla/security/nss/lib/cryptohi/cryptoht.h',
         'mozilla/security/nss/lib/cryptohi/dsautil.c',
@@ -683,7 +759,6 @@
         'mozilla/security/nss/lib/nss/nss.h',
         'mozilla/security/nss/lib/nss/nssinit.c',
         'mozilla/security/nss/lib/nss/nssrenam.h',
-        'mozilla/security/nss/lib/nss/nssver.c',
         'mozilla/security/nss/lib/nss/utilwrap.c',
         'mozilla/security/nss/lib/pk11wrap/debug_module.c',
         'mozilla/security/nss/lib/pk11wrap/dev3hack.c',
@@ -853,16 +928,23 @@
         # debug_module.c is included by pk11load.c.
         'mozilla/security/nss/lib/pk11wrap/debug_module.c',
       ],
+      'dependencies': [
+        'nspr',
+        '../sqlite/sqlite.gyp:sqlite',
+      ],
+      'export_dependent_settings': [
+        'nspr',

[Ryan Sleevi, 2012/05/15 04:23:39]

Since NSS public headers include NSPR headers, forcing this dependency to be
declared.

+      ],
       'defines': [
-        'NSS_ENABLE_ECC',
-        'USE_UTIL_DIRECTLY',
         'MP_API_COMPATIBLE',
-        'RIJNDAEL_INCLUDE_TABLES',
         'NSS_DISABLE_DBM',
+        'NSS_ENABLE_ECC',
         'NSS_STATIC',
         'NSS_USE_STATIC_LIBS',
+        'RIJNDAEL_INCLUDE_TABLES',
         'SHLIB_VERSION=\"3\"',
         'SOFTOKEN_SHLIB_VERSION=\"3\"',

[wtc, 2012/05/15 18:03:47]

The original order of these macros is a faithful imitation
of the upstream NSS build system.  Sorting the macros is
fine.

+        'USE_UTIL_DIRECTLY',
       ],
       'defines!': [
         # Regrettably, NSS can't be compiled with NO_NSPR_10_SUPPORT yet.
@@ -872,7 +954,6 @@
         'mozilla/security/nss/lib/base',
         'mozilla/security/nss/lib/certdb',
         'mozilla/security/nss/lib/certhigh',
-        'mozilla/security/nss/lib/ckfw',
         'mozilla/security/nss/lib/cryptohi',
         'mozilla/security/nss/lib/dev',
         'mozilla/security/nss/lib/freebl',
@@ -899,13 +980,10 @@
         'mozilla/security/nss/lib/ssl',
         'mozilla/security/nss/lib/util',
       ],
-      'dependencies': [
-        'nspr',
-        '../sqlite/sqlite.gyp:sqlite',
-      ],
       'direct_dependent_settings': {
         'defines': [
           'NO_NSPR_10_SUPPORT',

[wtc, 2012/05/15 18:03:47]

I believe NO_NSPR_10_SUPPORT can be removed because you
added the export_dependent_settings block for nspr.

+          'NSS_ENABLE_ECC',

[Ryan Sleevi, 2012/05/15 04:23:39]

Similar to the above XP_UNIX-and-friends above in nspr, this worked because
libssl in Chromium's local tree was also defining this.

Since this affects the public API, force this as a #define.

[wtc, 2012/05/15 18:03:47]

Yes, this is a good change.  Thanks.

           'NSS_STATIC',
           'NSS_USE_STATIC_LIBS',
           'USE_UTIL_DIRECTLY',
@@ -936,12 +1014,6 @@
           'defines': [
             'NSS_DISABLE_ROOT_CERTS',
           ],
-          'sources/': [
-            ['exclude', '^mozilla/security/nss/lib/ckfw/'],
-          ],
-          'include_dirs!': [
-            'mozilla/security/nss/lib/ckfw',
-          ],
         }],
         ['exclude_nss_libpkix==1', {
           'defines': [
@@ -965,10 +1037,6 @@
         }],
         ['OS=="mac"', {
           'defines': [
-            'XP_UNIX',
-            'DARWIN',
-            'HAVE_STRERROR',
-            'HAVE_BSD_FLOCK',

[Ryan Sleevi, 2012/05/15 04:23:39]

HAVE_STRERROR and HAVE_BSD_FLOCK are only used by NSPR, and the other two are
now exported as dependent settings by NSPR.

[wtc, 2012/05/15 18:03:47]

Please add these four macros back (especially the first two).
These are a faithful imitation of the upstream NSS build
system.

I will have HAVE_STRERROR and HAVE_BSD_FLOCK removed from
the upstream NSS build system.

             'SHLIB_SUFFIX=\"dylib\"',
             'SHLIB_PREFIX=\"lib\"',
             'SOFTOKEN_LIB_NAME=\"libsoftokn3.dylib\"',
@@ -992,24 +1060,19 @@
               'NSS_X64',
             ],
           },
-          'conditions': [
-            ['component == "shared_library"', {
-              'xcode_settings': {
-                'GCC_SYMBOLS_PRIVATE_EXTERN': 'NO',  # no -fvisibility=hidden
-              },
-            }],
-          ],
         }],
         ['OS=="win"', {
           'defines': [
-            'XP_PC',
-            'WIN32',
             '_WINDOWS',
-            'WIN95',

[wtc, 2012/05/15 18:03:47]

Please add these three macros back.  These are a faithful
imitation of the upstream NSS build system.

             'SHLIB_SUFFIX=\"dll\"',
             'SHLIB_PREFIX=\"\"',
             'SOFTOKEN_LIB_NAME=\"softokn3.dll\"',
           ],
+          'direct_dependent_settings': {
+            'defines': [
+              '_WINDOWS',

[wtc, 2012/05/15 18:03:47]

This direct_dependent_settings block is correct because the
public NSS header secport.h tests the _WINDOWS macro.  We
probably should change secport.h in NSS upstream to not
test _WINDOWS.  I believe the testing of _WINDOWS is a mistake
because _WINDOWS means a windows application (as opposed to
_CONSOLE, a console application) rather than the Windows
operating system.

+            ],
+          },
           'conditions': [
             ['target_arch=="ia32"', {
               'defines': [
@@ -1023,11 +1086,6 @@
                 'MP_NO_MP_WORD',
               ],
             }],
-            ['component == "shared_library"', {
-              'sources': [
-                'mozilla/security/nss/exports_win.def',
-              ],
-            }],
           ],
         }],
         ['clang==1', {