Switching ppapi on nacl side to import chrome side .gyp[i] directly.

Switching ppapi on nacl side to import chrome side .gyp[i] directly.
This should eliminate the need to things using the update script.

BUG=http://code.google.com/p/nativeclient/issues/detail?id=926
TEST=None
Committed: http://src.chromium.org/viewvc/native_client?view=rev&revision=4612

[Mark Seaborn, 2011-03-22 01:17:27]

Drive-by style comments...

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/build.scons
File src/shared/ppapi/build.scons (right):

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/build.scons...
src/shared/ppapi/build.scons:15: #   # From GYP_FILE_NAME:TARGET:REGEXP
This comment would need to be removed.

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/build.scons...
src/shared/ppapi/build.scons:41: ppapi_cpp_gypi =
eval(open(ppapi_cpp_gypi_filename).read())
Doing "eval(e, {})" would be cleaner than "eval(e)" because the former avoids
allowing variables to leak through into the eval'd scope.

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/build.scons...
src/shared/ppapi/build.scons:43: ppapi_cpp_objects = [t for t in
ppapi_cpp_gypi['targets']
t -> target, perhaps?

Instead of x[0], maybe do GetOne(x) where:

def GetOne(sequence):
  assert len(sequence) == 1, sequence
  return sequence[0]

Otherwise, ordering in the Gyp file would be significant and that should
probably be commented.

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/build.scons...
src/shared/ppapi/build.scons:48: cpp_sources = ([s for s in
ppapi_cpp_objects['sources']
s -> source_file?

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/build.scons...
src/shared/ppapi/build.scons:49: if s.endswith('.cc')] +
Indentation is odd here.

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/nacl.scons
File src/shared/ppapi/nacl.scons (right):

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/nacl.scons#...
src/shared/ppapi/nacl.scons:41: ppapi_cpp_gypi =
eval(open(ppapi_cpp_gypi_filename).read())
Ditto.

Hmm, all this logic is duplicated.

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/nacl.scons#...
src/shared/ppapi/nacl.scons:87: if re.match('^cpp/[^/]*\.h$', s)] +
The '^' should be redundant in re.match(), FWIW.

http://codereview.chromium.org/6715019/diff/1007/tests/ppapi/nacl.scons
File tests/ppapi/nacl.scons (right):

http://codereview.chromium.org/6715019/diff/1007/tests/ppapi/nacl.scons#newco...
tests/ppapi/nacl.scons:20: ppapi_cpp_gypi =
eval(open(ppapi_cpp_gypi_filename).read())
Ditto.

http://codereview.chromium.org/6715019/diff/1007/tests/ppapi/nacl.scons#newco...
tests/ppapi/nacl.scons:27: if re.match('^cpp/[^/]*\.h$', s)]
Indentation is off.

[bradn, 2011-03-22 01:51:43]

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/build.scons
File src/shared/ppapi/build.scons (right):

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/build.scons...
src/shared/ppapi/build.scons:15: #   # From GYP_FILE_NAME:TARGET:REGEXP
On 2011/03/22 01:17:27, Mark Seaborn wrote:
> This comment would need to be removed.

Done.

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/build.scons...
src/shared/ppapi/build.scons:41: ppapi_cpp_gypi =
eval(open(ppapi_cpp_gypi_filename).read())
On 2011/03/22 01:17:27, Mark Seaborn wrote:
> Doing "eval(e, {})" would be cleaner than "eval(e)" because the former avoids
> allowing variables to leak through into the eval'd scope.

Done.

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/build.scons...
src/shared/ppapi/build.scons:43: ppapi_cpp_objects = [t for t in
ppapi_cpp_gypi['targets']
On 2011/03/22 01:17:27, Mark Seaborn wrote:
> t -> target, perhaps?
> 
> Instead of x[0], maybe do GetOne(x) where:
> 
> def GetOne(sequence):
>   assert len(sequence) == 1, sequence
>   return sequence[0]
> 
> Otherwise, ordering in the Gyp file would be significant and that should
> probably be commented.
Migrated to something with an assert.

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/build.scons...
src/shared/ppapi/build.scons:48: cpp_sources = ([s for s in
ppapi_cpp_objects['sources']
On 2011/03/22 01:17:27, Mark Seaborn wrote:
> s -> source_file?

Done.

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/build.scons...
src/shared/ppapi/build.scons:49: if s.endswith('.cc')] +
On 2011/03/22 01:17:27, Mark Seaborn wrote:
> Indentation is odd here.

Restructured.

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/nacl.scons
File src/shared/ppapi/nacl.scons (right):

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/nacl.scons#...
src/shared/ppapi/nacl.scons:41: ppapi_cpp_gypi =
eval(open(ppapi_cpp_gypi_filename).read())
On 2011/03/22 01:17:27, Mark Seaborn wrote:
> Ditto.
> 
> Hmm, all this logic is duplicated.

Done.

http://codereview.chromium.org/6715019/diff/1007/src/shared/ppapi/nacl.scons#...
src/shared/ppapi/nacl.scons:87: if re.match('^cpp/[^/]*\.h$', s)] +
On 2011/03/22 01:17:27, Mark Seaborn wrote:
> The '^' should be redundant in re.match(), FWIW.

Done.

[bradn, 2011-03-22 01:54:08]

Oh, wait, forgot one file... Hold on.

[bradn, 2011-03-22 01:59:18]

Ok, PTAL.

[polina, 2011-03-22 20:41:18]

I am very much onboard with the overall approach, but since I don't have
extensive experience using scons-speak, I might be the wrong person to review
the details. Jason looked into this at some point, would it make sense to
involve him?

[Mark Seaborn, 2011-03-23 18:51:12]

LGTM w.r.t. the style issues I raised.

[Mark Seaborn, 2011-03-23 18:51:32]

One nit, by the way:

http://codereview.chromium.org/6715019/diff/6010/site_scons/gyp_extract.py
File site_scons/gyp_extract.py (right):

http://codereview.chromium.org/6715019/diff/6010/site_scons/gyp_extract.py#ne...
site_scons/gyp_extract.py:2: # Copyright 2010 The Native Client Authors.  All
rights reserved.
2011?