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

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

BUG=123666
TEST=GYP_DEFINES=exclude_nss_root_certs=0 ./build/gyp_chromium   && compile
R=wtc

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

wtc: PTAL.

As I begin to work on https://code.google.com/p/chromium/issues/detail?id=128134
/ http://code.google.com/p/chromium/issues/detail?id=123666, I suspect that ckfw
(the NSS bootstrap portion) will need to be separated out from nssckbi (the NSS
built in trust list), so that we can build ckfw+osx without having to build
nssckbi.

This is just a first step to keep 'exclude_nss_root_certs' building.

http://codereview.chromium.org/10392098/diff/1/nss.gyp
File nss.gyp (left):

http://codereview.chromium.org/10392098/diff/1/nss.gyp#oldcode971
nss.gyp:971: 'HAVE_BSD_FLOCK',
HAVE_STRERROR and HAVE_BSD_FLOCK are only used by NSPR, and the other two are
now exported as dependent settings by NSPR.

http://codereview.chromium.org/10392098/diff/1/nss.gyp
File nss.gyp (right):

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode255
nss.gyp:255: 'XP_MACOSX',
Since these all appear in (indirectly included) .h files, explicitly specifying
them here.

Previously this was handled transitively by each of the targets.

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode299
nss.gyp:299: },
Same here

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode352
nss.gyp:352: 'mozilla/security/nss/lib/nss/nssver.c',
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)

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode936
nss.gyp:936: 'nspr',
Since NSS public headers include NSPR headers, forcing this dependency to be
declared.

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode986
nss.gyp:986: 'NSS_ENABLE_ECC',
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:46]

Patch set 1 LGTM.  Please make the changes I suggested below.

http://codereview.chromium.org/10392098/diff/1/nss.gyp
File nss.gyp (left):

http://codereview.chromium.org/10392098/diff/1/nss.gyp#oldcode248
nss.gyp:248: 'HAVE_LCHOWN',

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.

http://codereview.chromium.org/10392098/diff/1/nss.gyp#oldcode865
nss.gyp:865: 'SOFTOKEN_SHLIB_VERSION=\"3\"',

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

http://codereview.chromium.org/10392098/diff/1/nss.gyp#oldcode971
nss.gyp:971: 'HAVE_BSD_FLOCK',

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.

http://codereview.chromium.org/10392098/diff/1/nss.gyp#oldcode1008
nss.gyp:1008: 'WIN95',

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

http://codereview.chromium.org/10392098/diff/1/nss.gyp
File nss.gyp (right):

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode255
nss.gyp:255: 'XP_MACOSX',

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.

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode360
nss.gyp:360: 'nssckbi',

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.

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode985
nss.gyp:985: 'NO_NSPR_10_SUPPORT',

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

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode986
nss.gyp:986: 'NSS_ENABLE_ECC',

Yes, this is a good change.  Thanks.

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode1073
nss.gyp:1073: '_WINDOWS',

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.

[Ryan Sleevi, 2012-05-15 19:38:18]

Review feedback

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

http://codereview.chromium.org/10392098/diff/1/nss.gyp
File nss.gyp (right):

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode255
nss.gyp:255: 'XP_MACOSX',
On 2012/05/15 18:03:47, wtc wrote:
> 
> 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.

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

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode360
nss.gyp:360: 'nssckbi',
On 2012/05/15 18:03:47, wtc wrote:
> 
> 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.

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"

[Ryan Sleevi, 2012-05-15 20:20:56]

Add back HAVE_LCHOWN

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

http://codereview.chromium.org/10392098/diff/1/nss.gyp
File nss.gyp (right):

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode255
nss.gyp:255: 'XP_MACOSX',

On 2012/05/15 19:38:47, Ryan Sleevi wrote:
> 
> Ah, I missed that we had specially patched prcpucfg.h to work around the
> configure step auto-generating.

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.

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode360
nss.gyp:360: 'nssckbi',

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]

http://codereview.chromium.org/10392098/diff/1/nss.gyp
File nss.gyp (right):

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode255
nss.gyp:255: 'XP_MACOSX',
On 2012/05/15 23:47:11, wtc wrote:
> 
> On 2012/05/15 19:38:47, Ryan Sleevi wrote:
> > 
> > Ah, I missed that we had specially patched prcpucfg.h to work around the
> > configure step auto-generating.
> 
> 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.

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

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode360
nss.gyp:360: 'nssckbi',
On 2012/05/15 23:47:11, wtc wrote:
> 
> 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.)

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:39]

http://codereview.chromium.org/10392098/diff/1/nss.gyp
File nss.gyp (right):

http://codereview.chromium.org/10392098/diff/1/nss.gyp#newcode360
nss.gyp:360: 'nssckbi',

On 2012/05/15 23:54:12, Ryan Sleevi wrote:
>
> 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.

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