Get rid of assumptions about ".nexe" extension.

Get rid of assumptions about ".nexe" extension.

This is a step towards having PNaCl generate bitcode "pexe"
files instead of pretending to be a native toolchain.
Committed: http://src.chromium.org/viewvc/native_client?view=rev&revision=5795

[Nick Bray, 2011-06-23 18:32:24]

... you monster.

Seriously, if there is any way to split up this CL - please do it!

http://codereview.chromium.org/7242011/diff/6001/SConstruct
File SConstruct (right):

http://codereview.chromium.org/7242011/diff/6001/SConstruct#newcode1605
SConstruct:1605: if args is None:
if args is not None:
  command = [nexe] + args

(Watch the spacing around brackets, too.)

http://codereview.chromium.org/7242011/diff/6001/src/trusted/service_runtime/...
File src/trusted/service_runtime/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/src/trusted/service_runtime/...
src/trusted/service_runtime/nacl.scons:161: nexe_name = base_name
Inline.

http://codereview.chromium.org/7242011/diff/6001/tests/barebones/nacl.scons
File tests/barebones/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/barebones/nacl.scons#n...
tests/barebones/nacl.scons:78: # a test that the validador can handle an empty
segment after the last
Spelling

http://codereview.chromium.org/7242011/diff/6001/tests/barebones/nacl.scons#n...
tests/barebones/nacl.scons:81: nexe =
env.File('${SCONSTRUCT_DIR}/tests/prebuilt/arm/gold-barebones_hello_world.nexe')
Line length.

http://codereview.chromium.org/7242011/diff/6001/tests/browser_dynamic_librar...
File tests/browser_dynamic_library/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/browser_dynamic_librar...
tests/browser_dynamic_library/nacl.scons:36: prog_suffix)
Why not interpolate prog_suffix like ${STAGING_DIR} ?

Also, using .get here is awful.  Prefer [].  Or interpolate TARGET_FULLARCH like
everything else.

http://codereview.chromium.org/7242011/diff/6001/tests/browser_startup_time/n...
File tests/browser_startup_time/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/browser_startup_time/n...
tests/browser_startup_time/nacl.scons:14: nexe = 'browser_startup_time_%s' %
env.get('TARGET_FULLARCH')
Why leave this named nexe?

http://codereview.chromium.org/7242011/diff/6001/tests/bundle_size/nacl.scons
File tests/bundle_size/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/bundle_size/nacl.scons...
tests/bundle_size/nacl.scons:8: nexe = env.ComponentProgram('bundle_size',
Is this nexe?  Or prog?  Or something else?  Pick a name and standardize it.

http://codereview.chromium.org/7242011/diff/6001/tests/chrome_extension/nacl....
File tests/chrome_extension/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/chrome_extension/nacl....
tests/chrome_extension/nacl.scons:15: nexe =
'ppapi_test_example_%s${PROGSUFFIX}' % env.get('TARGET_FULLARCH')
For cross-references like this, interpolate all the variables the same way.

http://codereview.chromium.org/7242011/diff/6001/tests/dup/nacl.scons
File tests/dup/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/dup/nacl.scons#newcode12
tests/dup/nacl.scons:12: nexe = env.ComponentProgram('dup_test', 'dup_test.c')
Again, what should the local name be?

http://codereview.chromium.org/7242011/diff/6001/tests/egyptian_cotton/nacl.s...
File tests/egyptian_cotton/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/egyptian_cotton/nacl.s...
tests/egyptian_cotton/nacl.scons:123: args = args)
No spaces around keyword args.

args = args -> args=args

http://codereview.chromium.org/7242011/diff/6001/tests/fake_browser_ppapi/nac...
File tests/fake_browser_ppapi/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/fake_browser_ppapi/nac...
tests/fake_browser_ppapi/nacl.scons:76: basic_object_target =
env.File('$STAGING_DIR/ppapi_basic_object_%s${PROGSUFFIX}' %
Line length.

http://codereview.chromium.org/7242011/diff/6001/tests/inbrowser_test_runner/...
File tests/inbrowser_test_runner/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/inbrowser_test_runner/...
tests/inbrowser_test_runner/nacl.scons:24:
env.File('$STAGING_DIR/dynamic_load_test%s${PROGSUFFIX}' % arch_suffix),
Interpolate both the same way?

http://codereview.chromium.org/7242011/diff/6001/tests/ppapi_browser/ppb_grap...
File tests/ppapi_browser/ppb_graphics2d/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/ppapi_browser/ppb_grap...
tests/ppapi_browser/ppb_graphics2d/nacl.scons:14: [
'$STAGING_DIR/%s${PROGSUFFIX}' % nexe ])
Bracket spacing wrong.

http://codereview.chromium.org/7242011/diff/6001/tests/ppapi_ppb_scrollbar/na...
File tests/ppapi_ppb_scrollbar/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/ppapi_ppb_scrollbar/na...
tests/ppapi_ppb_scrollbar/nacl.scons:12: [ '$STAGING_DIR/%s${PROGSUFFIX}' % nexe
])
Spacing.

http://codereview.chromium.org/7242011/diff/6001/tests/syscalls/nacl.scons
File tests/syscalls/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/syscalls/nacl.scons#ne...
tests/syscalls/nacl.scons:103: 'semaphore_tests',
Wrong indentation level.

4 spaces if using this line breaking style.

http://codereview.chromium.org/7242011/diff/6001/tests/syscalls/nacl.scons#ne...
tests/syscalls/nacl.scons:140: 'timefuncs_test',
Bad spacing.

http://codereview.chromium.org/7242011/diff/6001/tests/syscalls/nacl.scons#ne...
tests/syscalls/nacl.scons:155: 'raw_timefuncs_test',
Bad spacing

http://codereview.chromium.org/7242011/diff/6001/tests/threads/nacl.scons
File tests/threads/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/threads/nacl.scons#new...
tests/threads/nacl.scons:26: 'thread_test',
Bad spacing

http://codereview.chromium.org/7242011/diff/6001/tests/threads/nacl.scons#new...
tests/threads/nacl.scons:51: 'simple_thread_test',
Bad spacing.  I won't note it any more.  Fix it all.

[pdox, 2011-06-23 20:42:57]

http://codereview.chromium.org/7242011/diff/6001/SConstruct
File SConstruct (right):

http://codereview.chromium.org/7242011/diff/6001/SConstruct#newcode1605
SConstruct:1605: if args is None:
On 2011/06/23 18:32:24, Nick Bray wrote:
> if args is not None:
>   command = [nexe] + args
> 
> (Watch the spacing around brackets, too.)

Done.

http://codereview.chromium.org/7242011/diff/6001/src/trusted/service_runtime/...
File src/trusted/service_runtime/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/src/trusted/service_runtime/...
src/trusted/service_runtime/nacl.scons:161: nexe_name = base_name
On 2011/06/23 18:32:24, Nick Bray wrote:
> Inline.

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/barebones/nacl.scons
File tests/barebones/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/barebones/nacl.scons#n...
tests/barebones/nacl.scons:78: # a test that the validador can handle an empty
segment after the last
On 2011/06/23 18:32:24, Nick Bray wrote:
> Spelling

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/barebones/nacl.scons#n...
tests/barebones/nacl.scons:81: nexe =
env.File('${SCONSTRUCT_DIR}/tests/prebuilt/arm/gold-barebones_hello_world.nexe')
On 2011/06/23 18:32:24, Nick Bray wrote:
> Line length.

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/browser_dynamic_librar...
File tests/browser_dynamic_library/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/browser_dynamic_librar...
tests/browser_dynamic_library/nacl.scons:36: prog_suffix)
On 2011/06/23 18:32:24, Nick Bray wrote:
> Why not interpolate prog_suffix like ${STAGING_DIR} ?
> 
> Also, using .get here is awful.  Prefer [].  Or interpolate TARGET_FULLARCH
like
> everything else.

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/browser_startup_time/n...
File tests/browser_startup_time/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/browser_startup_time/n...
tests/browser_startup_time/nacl.scons:14: nexe = 'browser_startup_time_%s' %
env.get('TARGET_FULLARCH')
On 2011/06/23 18:32:24, Nick Bray wrote:
> Why leave this named nexe?

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/bundle_size/nacl.scons
File tests/bundle_size/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/bundle_size/nacl.scons...
tests/bundle_size/nacl.scons:8: nexe = env.ComponentProgram('bundle_size',
Nick,

I left existing code alone, so if there's "prog" in there, it's from a previous
CL.

I've been using "nexe" if there is only a single one in the scons file.
Otherwise, I've been using test_name_nexe.

http://codereview.chromium.org/7242011/diff/6001/tests/chrome_extension/nacl....
File tests/chrome_extension/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/chrome_extension/nacl....
tests/chrome_extension/nacl.scons:15: nexe =
'ppapi_test_example_%s${PROGSUFFIX}' % env.get('TARGET_FULLARCH')
On 2011/06/23 18:32:24, Nick Bray wrote:
> For cross-references like this, interpolate all the variables the same way.

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/dup/nacl.scons
File tests/dup/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/dup/nacl.scons#newcode12
tests/dup/nacl.scons:12: nexe = env.ComponentProgram('dup_test', 'dup_test.c')
I don't understand your objection. Is there anything wrong with calling it nexe?
Even if it turns out to be a pexe, it still represents a nexe ("native client
executable") in the abstract.

http://codereview.chromium.org/7242011/diff/6001/tests/egyptian_cotton/nacl.s...
File tests/egyptian_cotton/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/egyptian_cotton/nacl.s...
tests/egyptian_cotton/nacl.scons:123: args = args)
On 2011/06/23 18:32:24, Nick Bray wrote:
> No spaces around keyword args.
> 
> args = args -> args=args

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/fake_browser_ppapi/nac...
File tests/fake_browser_ppapi/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/fake_browser_ppapi/nac...
tests/fake_browser_ppapi/nacl.scons:76: basic_object_target =
env.File('$STAGING_DIR/ppapi_basic_object_%s${PROGSUFFIX}' %
On 2011/06/23 18:32:24, Nick Bray wrote:
> Line length.

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/inbrowser_test_runner/...
File tests/inbrowser_test_runner/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/inbrowser_test_runner/...
tests/inbrowser_test_runner/nacl.scons:24:
env.File('$STAGING_DIR/dynamic_load_test%s${PROGSUFFIX}' % arch_suffix),
On 2011/06/23 18:32:24, Nick Bray wrote:
> Interpolate both the same way?

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/ppapi_browser/ppb_grap...
File tests/ppapi_browser/ppb_graphics2d/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/ppapi_browser/ppb_grap...
tests/ppapi_browser/ppb_graphics2d/nacl.scons:14: [
'$STAGING_DIR/%s${PROGSUFFIX}' % nexe ])
On 2011/06/23 18:32:24, Nick Bray wrote:
> Bracket spacing wrong.

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/ppapi_ppb_scrollbar/na...
File tests/ppapi_ppb_scrollbar/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/ppapi_ppb_scrollbar/na...
tests/ppapi_ppb_scrollbar/nacl.scons:12: [ '$STAGING_DIR/%s${PROGSUFFIX}' % nexe
])
On 2011/06/23 18:32:24, Nick Bray wrote:
> Spacing.

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/syscalls/nacl.scons
File tests/syscalls/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/syscalls/nacl.scons#ne...
tests/syscalls/nacl.scons:103: 'semaphore_tests',
On 2011/06/23 18:32:24, Nick Bray wrote:
> Wrong indentation level.
> 
> 4 spaces if using this line breaking style.

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/syscalls/nacl.scons#ne...
tests/syscalls/nacl.scons:140: 'timefuncs_test',
On 2011/06/23 18:32:24, Nick Bray wrote:
> Bad spacing.

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/syscalls/nacl.scons#ne...
tests/syscalls/nacl.scons:155: 'raw_timefuncs_test',
On 2011/06/23 18:32:24, Nick Bray wrote:
> Bad spacing

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/threads/nacl.scons
File tests/threads/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/6001/tests/threads/nacl.scons#new...
tests/threads/nacl.scons:26: 'thread_test',
On 2011/06/23 18:32:24, Nick Bray wrote:
> Bad spacing

Done.

http://codereview.chromium.org/7242011/diff/6001/tests/threads/nacl.scons#new...
tests/threads/nacl.scons:51: 'simple_thread_test',
On 2011/06/23 18:32:24, Nick Bray wrote:
> Bad spacing.  I won't note it any more.  Fix it all.

Done.

[pdox, 2011-06-23 21:41:21]

trybots are happy.
PTAL.

[jvoung - send to chromium..., 2011-06-23 21:55:23]

Would it be useful to have a check in ComponentProgram that things are being
named with ".nexe"?

http://codereview.chromium.org/7242011/diff/9095/tests/barebones/nacl.scons
File tests/barebones/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/9095/tests/barebones/nacl.scons#n...
tests/barebones/nacl.scons:70: out, nexe,
nit: does this fit all on one line now?

[pdox, 2011-06-23 22:03:31]

Added check, and two more files.

http://codereview.chromium.org/7242011/diff/9095/tests/barebones/nacl.scons
File tests/barebones/nacl.scons (right):

http://codereview.chromium.org/7242011/diff/9095/tests/barebones/nacl.scons#n...
tests/barebones/nacl.scons:70: out, nexe,
On 2011/06/23 21:55:23, jvoung wrote:
> nit: does this fit all on one line now?

Done.

[pdox, 2011-06-23 23:09:51]

trybots appear happy again, PTAL

[jvoung - send to chromium..., 2011-06-24 19:58:41]

LGTM