Add gyp files to build NSS in Dart

Add gyp files to build NSS in Dart

The change builds static versions of the cross-platform secure networking libraries NSS and NSPR from Mozilla, on Linux, MacOS, and Windows.  These libraries are linked with the standalone Dart executable in a future changelist to support HTTPS and SSL networking in dart:io.

The built libraries will be tested by that future changelist.

Patchlist 19 was committed: https://code.google.com/p/dart/source/detail?r=14524

Reverted in r=14529

Pachlist 21 was committed: https://code.google.com/p/dart/source/detail?r=14684

Reverted in r=14688

Committed: https://code.google.com/p/dart/source/detail?r=14730

[Bill Hesse, 2012-09-27 13:41:31]

Just barely working on all three platforms, building static libraries, that work
when linked with Dart.  I will move the modified gyp files to a good place
outside third_party, and see if I can make them work there.

[Bill Hesse, 2012-09-27 13:43:46]

Not ready for review at all - the modifications to tools/gyp should be moved out
into the third party gyp files, and reduced in number and scope.  Some patch
files are obsolete.

On 2012/09/27 13:41:31, Bill Hesse wrote:
> Just barely working on all three platforms, building static libraries, that
work
> when linked with Dart.  I will move the modified gyp files to a good place
> outside third_party, and see if I can make them work there.

[Bill Hesse, 2012-10-30 15:54:18]

Ready for review - it is reduced from 20 files down to 5 files.

[Mads Ager (google), 2012-10-31 09:01:01]

http://codereview.chromium.org/10909189/diff/28002/runtime/bin/net/nss.gyp
File runtime/bin/net/nss.gyp (right):

http://codereview.chromium.org/10909189/diff/28002/runtime/bin/net/nss.gyp#ne...
runtime/bin/net/nss.gyp:1: # Copyright (c) 2012 The Chromium Authors. All rights
reserved.
Could you indicate which parts of this file have been changed and where it is
coming from. We should have a comment at the top spelling that out.

It would be nice to figure out if we can avoid copying this file and use some
gyp include machinery to create a wrapper around the gyp files in the
third_party directory.

For now, let's just make it very clear what the differences are with good
comments.

http://codereview.chromium.org/10909189/diff/28002/runtime/bin/net/sqlite.gyp
File runtime/bin/net/sqlite.gyp (right):

http://codereview.chromium.org/10909189/diff/28002/runtime/bin/net/sqlite.gyp...
runtime/bin/net/sqlite.gyp:1: # Copyright (c) 2012 The Chromium Authors. All
rights reserved.
Same comment here. Let's make it clear where this file is coming from and what
the actual changes are. Also for the next files.

http://codereview.chromium.org/10909189/diff/28002/runtime/tools/gyp/third_pa...
File runtime/tools/gyp/third_party_configurations.gypi (right):

http://codereview.chromium.org/10909189/diff/28002/runtime/tools/gyp/third_pa...
runtime/tools/gyp/third_party_configurations.gypi:4: 
Could we call this file nss_configurations.gypi to make it clearer what it is
used for? I find the third_party name a bit too generic.

Let's add a comment here instead of in the variables section describing what
this file is used for. Also, add a comment that this file should only be used
for compiling C and not C++ code.

http://codereview.chromium.org/10909189/diff/28002/runtime/tools/gyp/third_pa...
runtime/tools/gyp/third_party_configurations.gypi:25: 'cflags!': [
I would like some more comments here. There is one line that has a comment. I
got confused by -fno-exceptions being removed until I realized that this is for
compiling C code and not C++.

[Søren Gjesse, 2012-10-31 11:27:04]

Please coordinate on adding zlib with ajohnsen@, see
http://codereview.chromium.org/11275035/.

[Bill Hesse, 2012-11-02 12:28:53]

All comments addressed.

The changes to the gyp files from the Chromium versions have been minimized, and
documented.  It seems like they could be upstreamed in conditions sections,
depending on a flag, but that would put what looks like a linux build in
Chromium, that doesn't actually build in Chromium, and other extraneous parts
that aren't relevant to Chromium builds in them.

Tested on all platforms.

[Mads Ager (google), 2012-11-02 13:56:35]

This is getting there. Final round of comments for the first version from me.

Longer term it would be nice if we could move closer to sharing gyp files with
upstream chromium.

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/net/nss.gyp
File runtime/bin/net/nss.gyp (right):

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/net/nss.gyp#ne...
runtime/bin/net/nss.gyp:12: 'includes': [  # Added by Dart.
Please move all of these comments to the line above what they are describing.

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/net/os_Linux.S
File runtime/bin/net/os_Linux.S (right):

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/net/os_Linux.S...
runtime/bin/net/os_Linux.S:1: // Copyright (c) 2012, the Dart project authors. 
Please see the AUTHORS file
Please rename to os_linux.S since we are adding this in the Dart repo we should
use dart naming.

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/net/sqlite.gyp
File runtime/bin/net/sqlite.gyp (right):

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/net/sqlite.gyp...
runtime/bin/net/sqlite.gyp:17: 'sqlite_directory':
'../../../third_party/sqlite',  # Added by Dart.
Please move to above the lines that the comments describe to make them stand out
more clearly.

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/net/ssl.gyp
File runtime/bin/net/ssl.gyp (right):

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/net/ssl.gyp#ne...
runtime/bin/net/ssl.gyp:18: 'includes': [  # Added by Dart.
Ditto

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/net/zlib.gyp
File runtime/bin/net/zlib.gyp (right):

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/net/zlib.gyp#n...
runtime/bin/net/zlib.gyp:12: 'includes': [  # Added by Dart.
Ditto

http://codereview.chromium.org/10909189/diff/46001/runtime/tools/gyp/nss_conf...
File runtime/tools/gyp/nss_configurations.gypi (right):

http://codereview.chromium.org/10909189/diff/46001/runtime/tools/gyp/nss_conf...
runtime/tools/gyp/nss_configurations.gypi:10: {
Could you add a blank line after the comment?

http://codereview.chromium.org/10909189/diff/46001/runtime/tools/gyp/nss_conf...
runtime/tools/gyp/nss_configurations.gypi:30: # Removes these flags from the
list cflags.
Could you group these flags and comment on the individual ones? -fno-exceptions
is removed because it is C code and not C++ code, right? However, it is not
under the comment that says that the flag is not for C.

[Ivan Posva, 2012-11-02 15:58:51]

Since the newly added gyp files are messing with the build configurations, I
would like to see if there is a difference in compiler command lines for the VM
sources before and after applying this patch.

Thanks,
-Ivan

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/bin.gypi
File runtime/bin/bin.gypi (right):

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/bin.gypi#newco...
runtime/bin/bin.gypi:318: 'export_dependent_settings': [
Can you briefly explain what this does and why it was needed?

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/net/nss.gyp
File runtime/bin/net/nss.gyp (right):

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/net/nss.gyp#ne...
runtime/bin/net/nss.gyp:186:
'<(nss_directory)/mozilla/nsprpub/pr/src/md/unix/linux.c',  # Added.
Is this also "Added by Dart"?

[Bill Hesse, 2012-11-02 16:08:15]

I will check out whether the compilation lines for existing targets have changed
(except to include the new include directories and linked libraries, where
appropriate) on all platforms.  Since we didn't change any of the global gyp
files, they should not have.

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/bin.gypi
File runtime/bin/bin.gypi (right):

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/bin.gypi#newco...
runtime/bin/bin.gypi:318: 'export_dependent_settings': [
On 2012/11/02 15:58:52, Ivan Posva wrote:
> Can you briefly explain what this does and why it was needed?

The targets like nss and zlib include direct-dependent settings for include
directories and defines, and to pass these up to the libdart_builtin, that is
actually compiling against the libraries and the header files, we need to pass
them through this intermediate target.  That is what export_dependent_settings
does.

We could avoid this by just putting the targets as dependencies in
libdart_builtin, and removing this target.  Do you think we should do this?

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/net/nss.gyp
File runtime/bin/net/nss.gyp (right):

http://codereview.chromium.org/10909189/diff/46001/runtime/bin/net/nss.gyp#ne...
runtime/bin/net/nss.gyp:186:
'<(nss_directory)/mozilla/nsprpub/pr/src/md/unix/linux.c',  # Added.
On 2012/11/02 15:58:52, Ivan Posva wrote:
> Is this also "Added by Dart"?

Yes.  I'll change it.  It didn't fit as an end-of-line comment.

[Bill Hesse, 2012-11-05 10:39:27]

All comments addressed.

The compilation commands issued when building via tools/build.py runtime have
been examined, and the only difference is that more include directories and
defines have been added to the compilation of files in the target
libdart_builtin.  All the commands for all other targets, including the VM,
tests, and the Dart executable, are unchanged.  Commands to compile the NSS and
NSPR libraries have been added, of course.

[Søren Gjesse, 2012-11-05 11:24:51]

lgtm

[Mads Ager (google), 2012-11-05 12:42:32]

LGTM

https://codereview.chromium.org/10909189/diff/49002/runtime/tools/gyp/nss_con...
File runtime/tools/gyp/nss_configurations.gypi (right):

https://codereview.chromium.org/10909189/diff/49002/runtime/tools/gyp/nss_con...
runtime/tools/gyp/nss_configurations.gypi:37: '-fno-exceptions',
Could you add two sections here? One for disabled warnings and one for flags
that only make sense for C++:

# disable warning flags
'-Werrror',
...
# remove C++ flags
'-fno-exceptions',
...

[Ivan Posva, 2012-11-09 23:00:20]

https://chromiumcodereview.appspot.com/10909189/diff/44010/runtime/bin/bin.gypi
File runtime/bin/bin.gypi (right):

https://chromiumcodereview.appspot.com/10909189/diff/44010/runtime/bin/bin.gy...
runtime/bin/bin.gypi:310: 'nss',
Can you list the different nss dependencies here? I would hope that this would
remove the need for 'export_dependent_settings' from below.