Linux: add option to use system nspr.

base/base.gypi

 # Copyright (c) 2012 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 {
   'target_defaults': {
     'variables': {
       'base_target': 0,
     },
     'target_conditions': [
       # This part is shared between the targets defined below.
       ['base_target==1', {
         'sources': [
           '../build/build_config.h',
           'third_party/dmg_fp/dmg_fp.h',
           'third_party/dmg_fp/g_fmt.cc',
           'third_party/dmg_fp/dtoa_wrapper.cc',
           'third_party/icu/icu_utf.cc',
           'third_party/icu/icu_utf.h',
           'third_party/nspr/prtime.cc',
           'third_party/nspr/prtime.h',
           'third_party/nspr/prcpucfg_linux.h',

[wtc, 2013/01/04 16:55:28]

Please add prtypes.h and prcpucfg.h.

I wonder if we should also add the other prcpucfg_xxx.h files.
It seems wrong to only list prcpucfg_linux.h.

[Paweł Hajdan Jr., 2013/01/04 18:01:31]

Thanks, done.

           'third_party/xdg_mime/xdgmime.h',
           'allocator/allocator_extension.cc',
           'allocator/allocator_extension.h',
           'allocator/type_profiler_control.cc',
           'allocator/type_profiler_control.h',
           'android/base_jni_registrar.cc',
           'android/base_jni_registrar.h',
           'android/build_info.cc',
           'android/build_info.h',
           'android/scoped_java_ref.cc',
@@ skipping 687 matching lines @@
           ['<(chromeos)!=1 or >(nacl_untrusted_build)==1', {
             'sources/': [
               ['exclude', '^chromeos/'],
             ],
           }],
           # Remove all unnecessary files for build_nexe.py to avoid exceeding
           # command-line-string limitation when building NaCl on Windows.
           ['OS == "win" and >(nacl_untrusted_build)==1', {
               'sources/': [ ['exclude', '\\.h$'] ],
           }],
+          ['<(use_system_nspr)==1', {

[Ryan Sleevi, 2013/01/03 18:51:35]

You're in a conditions block, do not use this variable syntax, since it's
implicit. (The same applies to the os_bsd and chromeos of lines 710/720,
respectively).

['use_system_nspr == 1', [
  [ ... ]
]

Also, why do you not need to check that >(nacl_untrusted_build) == 0? When
inside a NACL untrusted build, you surely don't intend to link in all of NSPR,
do you? If so, that should definitely get nacl & security-team review.

[Paweł Hajdan Jr., 2013/01/04 18:01:31]

Nope. If I don't do that build/gyp_chromium bails out with:

gyp: name 'use_system_nspr' is not defined while evaluating condition
'use_system_nspr==1' in
/usr/local/google/home/phajdan/chromium/src/base/base.gyp

I may be doing something wrong, in which case I'm happy to learn. However, these
two are clearly *not* equivalent (which may be a bug).
Note this conditional section excludes things, so it's not possible to drag more
things when excluding things.

By the way, linking to "outside world" is not possible or at least not easy in
NACL untrusted build. This change certainly doesn't do that, and continues to
use bundled prtime.cc for NaCl untrusted part.

[Ryan Sleevi, 2013/01/07 00:04:10]

Ah, it's because it's within a target_conditions block, which is forcing a
deferral of the context of the variables. That's still likely a bug in GYP,
since they're supposed to be equivalent.
I just think it's a bad idea not to be checking the >(nacl_untrusted_build) bit
here when removing. You can see in base_untrusted.gyp that it gets re-added
back, which I think is confusing compared to having the source files fully
enumerated within this GYP. 

If your condition was:
<(use_system_nspr)==1 and >(nacl_untrusted_build)==0

Then you wouldn't have to re-add the source back in base_untrusted

+            'sources/': [
+              ['exclude', '^third_party/nspr/'],

[wtc, 2013/01/04 16:55:28]

This will exclude prtime.cc and use the PR_ParseTimeString
function from the system nspr library instead, right?

If so, our bundled prtime.h needs to declare the
PR_ParseTimeString function with extern "C" to avoid
potential name mangling issues. I suggest copying over
the PR_BEGIN_EXTERN_C and PR_END_EXTERN_C from upstream
prtime.h:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/nsprpub/pr/include/prtime...

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/nsprpub/pr/include/prtime...

[Paweł Hajdan Jr., 2013/01/04 18:01:31]

Yep. We're using system headers though. That's the whole trick here: when using
system libs, we should always use system headers, and when using bundled libs we
should be using bundled headers.

The conclusion is that no change is necessary here, since we always use matching
headers, _and_ this is confirmed by the change working both for
use_system_nspr=0 and 1. Otherwise it would fail to link.

[wtc, 2013/01/05 16:23:35]

You're right. After I wrote my previous comment, I realized
my suggestion was wrong because we're always using matching
headers. Unfortunately I could not send you a new comment
until now.
Some compilers may not require extern "C" when it should be
used, so in general a successful build under one compiler
is not sufficient to confirm extern "C" is used correctly.
But I remember gcc requires proper use of extern "C".

+            ],
+          }],
         ],
       }],
     ],
   },
 }