Change built_elsewhere flag to work with pexe mode

Change built_elsewhere flag to work with pexe mode
Instead of ignoring all dependencies for a test target, we replace the builders (CC CXX et al) except for translate with no-ops.

Also run some basic pexe mode tests on arm, and tweak the pruning process before uploading to the hw bot( and give the tarball a better name).

BUG= http://code.google.com/p/nativeclient/issues/detail?id=2609
TEST=bots

Committed: https://src.chromium.org/viewvc/native_client?view=rev&revision=8091

[(google.com) Derek Schuff, 2012-03-21 22:43:52]

PTAL

[jvoung - send to chromium..., 2012-03-21 23:15:04]

https://chromiumcodereview.appspot.com/9824001/diff/1005/SConstruct
File SConstruct (right):

https://chromiumcodereview.appspot.com/9824001/diff/1005/SConstruct#newcode1981
SConstruct:1981: env.Precious(pexe)
Could you add a comment explaining the need for env.Precious?

https://chromiumcodereview.appspot.com/9824001/diff/1005/SConstruct#newcode2314
SConstruct:2314: env.Precious(deps)
Should extra_deps be precious as well?

https://chromiumcodereview.appspot.com/9824001/diff/1005/SConstruct#newcode2884
SConstruct:2884: # Replace build commands with no-ops
I wonder if 'true' would work on Windows, but... built_elsewhere for Windows?
Never =)

https://chromiumcodereview.appspot.com/9824001/diff/1005/SConstruct#newcode2887
SConstruct:2887: # Allow emulation for testing built_elsewhere flag
Should this emulation setting be removed before committing to avoid making the
ARM hw do emulation?

https://chromiumcodereview.appspot.com/9824001/diff/1005/site_scons/site_tool...
File site_scons/site_tools/naclsdk.py (right):

https://chromiumcodereview.appspot.com/9824001/diff/1005/site_scons/site_tool...
site_scons/site_tools/naclsdk.py:332: RANLIB='true', AS='true', LD='true',
INSTALL=FakeInstall,
Do you need to stub out STRIP as well?

https://chromiumcodereview.appspot.com/9824001/diff/1005/src/trusted/service_...
File src/trusted/service_runtime/build.scons (right):

https://chromiumcodereview.appspot.com/9824001/diff/1005/src/trusted/service_...
src/trusted/service_runtime/build.scons:357: if env.Bit('linux') and not
env.CrossToolsBuild() and not env.Bit('built_elsewhere'):
80 col.  Maybe add a comment about not setting up this target for
built_elsewhere.

[(google.com) Derek Schuff, 2012-03-22 00:02:02]

https://chromiumcodereview.appspot.com/9824001/diff/1005/SConstruct
File SConstruct (right):

https://chromiumcodereview.appspot.com/9824001/diff/1005/SConstruct#newcode1981
SConstruct:1981: env.Precious(pexe)
On 2012/03/21 23:15:04, jvoung wrote:
> Could you add a comment explaining the need for env.Precious?

Done.

https://chromiumcodereview.appspot.com/9824001/diff/1005/SConstruct#newcode2314
SConstruct:2314: env.Precious(deps)
The only thing that really needs to be precious is the pexe (handled above), and
various TCB stuff (sel_ldr/nacl_bootstrap_helper) which scons randomly decides
need to be rebuilt. That stuff is handled here but the extra stuff isn't really
necessary.
On 2012/03/21 23:15:04, jvoung wrote:
> Should extra_deps be precious as well?

https://chromiumcodereview.appspot.com/9824001/diff/1005/SConstruct#newcode2884
SConstruct:2884: # Replace build commands with no-ops
well I guess it's conceivable if we want to have a windows testing bot that
doesn't build. (IIRC the gnu toolchain builds on linux only). but there would
surely be more work than just this to make that work.
On 2012/03/21 23:15:04, jvoung wrote:
> I wonder if 'true' would work on Windows, but... built_elsewhere for Windows?
> Never =)

https://chromiumcodereview.appspot.com/9824001/diff/1005/SConstruct#newcode2887
SConstruct:2887: # Allow emulation for testing built_elsewhere flag
the platform.machine() test makes sure it's only enabled on x86 hosts. 
enhanced the comment.
On 2012/03/21 23:15:04, jvoung wrote:
> Should this emulation setting be removed before committing to avoid making the
> ARM hw do emulation?

https://chromiumcodereview.appspot.com/9824001/diff/1005/site_scons/site_tool...
File site_scons/site_tools/naclsdk.py (right):

https://chromiumcodereview.appspot.com/9824001/diff/1005/site_scons/site_tool...
site_scons/site_tools/naclsdk.py:332: RANLIB='true', AS='true', LD='true',
INSTALL=FakeInstall,
Apparently not, since it works without it? couldn't hurt though, scons seems to
randomly decide stuff needs rebuilding so it could pop up sometime
On 2012/03/21 23:15:04, jvoung wrote:
> Do you need to stub out STRIP as well?

[jvoung - send to chromium..., 2012-03-22 00:17:45]

https://chromiumcodereview.appspot.com/9824001/diff/1005/SConstruct
File SConstruct (right):

https://chromiumcodereview.appspot.com/9824001/diff/1005/SConstruct#newcode2887
SConstruct:2887: # Allow emulation for testing built_elsewhere flag
On 2012/03/22 00:02:02, Derek Schuff wrote:
> the platform.machine() test makes sure it's only enabled on x86 hosts. 
> enhanced the comment.
> On 2012/03/21 23:15:04, jvoung wrote:
> > Should this emulation setting be removed before committing to avoid making
the
> > ARM hw do emulation?
> 

Ah of course.

https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
File site_scons/site_tools/naclsdk.py (right):

https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
site_scons/site_tools/naclsdk.py:372: env.Replace(CC='true', CXX='true',
ASPP='true', LINK='true', LD='true')
Should this be done earlier?  Mainly concerned with how this interacts with the
pnacl_generate_pexe check above (with an early return).

https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
site_scons/site_tools/naclsdk.py:399: CXX='NO-NATIVE-CXX-INVOCATION-ALLOWED')
I assume this worked too since I think toolchain tests should include something
that used PNaClGetNNaClEnv ?

[robertm, 2012-03-22 14:09:37]

drive by comments below

https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
File site_scons/site_tools/naclsdk.py (right):

https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
site_scons/site_tools/naclsdk.py:331: env.Replace(CC='true', CXX='true',
LINK='true', AR='true',
if it is not too much work it be nice to make the faked out
command more verbose, e.g.

 'Skip CC', dest
'Skip CXX', dest

etc

https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
site_scons/site_tools/naclsdk.py:372: env.Replace(CC='true', CXX='true',
ASPP='true', LINK='true', LD='true')
if this ends up staying around you could unify this with the similar statement
above.

On 2012/03/22 00:17:45, jvoung wrote:
> Should this be done earlier?  Mainly concerned with how this interacts with
the
> pnacl_generate_pexe check above (with an early return).

https://chromiumcodereview.appspot.com/9824001/diff/10001/src/trusted/service...
File src/trusted/service_runtime/build.scons (right):

https://chromiumcodereview.appspot.com/9824001/diff/10001/src/trusted/service...
src/trusted/service_runtime/build.scons:357: if env.Bit('linux') and not
env.CrossToolsBuild() and not env.Bit('built_elsewhere'):
line length

[(google.com) Derek Schuff, 2012-03-22 16:10:25]

https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
File site_scons/site_tools/naclsdk.py (right):

https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
site_scons/site_tools/naclsdk.py:331: env.Replace(CC='true', CXX='true',
LINK='true', AR='true',
How would that work? Will it work if I just pass a python function here instead
of a shell command?

On 2012/03/22 14:09:37, robertm wrote:
> if it is not too much work it be nice to make the faked out
> command more verbose, e.g.
> 
>  'Skip CC', dest
> 'Skip CXX', dest
> 
> etc

https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
site_scons/site_tools/naclsdk.py:372: env.Replace(CC='true', CXX='true',
ASPP='true', LINK='true', LD='true')
The change above is for the nacl environment and applies in both pexe and
non-pexe mode. This change only applies in non-pexe mode since this function
should not be called otherwise (hence the check above). This change is just
necessary because the change to the way built_elsewhere is implemented affects
both pexe and non-pexe mode.

On 2012/03/22 00:17:45, jvoung wrote:
> Should this be done earlier?  Mainly concerned with how this interacts with
the
> pnacl_generate_pexe check above (with an early return).

https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
site_scons/site_tools/naclsdk.py:399: CXX='NO-NATIVE-CXX-INVOCATION-ALLOWED')
It did work when I tested, but I added a case here as well just to be sure.
On 2012/03/22 00:17:45, jvoung wrote:
> I assume this worked too since I think toolchain tests should include
something
> that used PNaClGetNNaClEnv ?

https://chromiumcodereview.appspot.com/9824001/diff/10001/src/trusted/service...
File src/trusted/service_runtime/build.scons (right):

https://chromiumcodereview.appspot.com/9824001/diff/10001/src/trusted/service...
src/trusted/service_runtime/build.scons:357: if env.Bit('linux') and not
env.CrossToolsBuild() and not env.Bit('built_elsewhere'):
On 2012/03/22 14:09:37, robertm wrote:
> line length 

Done.

[robertm, 2012-03-22 16:18:43]

LGTM

(assuming trybots are happy)

https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
File site_scons/site_tools/naclsdk.py (right):

https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
site_scons/site_tools/naclsdk.py:331: env.Replace(CC='true', CXX='true',
LINK='true', AR='true',
not sure. 
another option could be CC='echo CC #' where the purpose of the '#' is to ignore
the command line.
if the simple stuff does not work don't bother

On 2012/03/22 16:10:25, Derek Schuff wrote:
> How would that work? Will it work if I just pass a python function here
instead
> of a shell command?
> 
> On 2012/03/22 14:09:37, robertm wrote:
> > if it is not too much work it be nice to make the faked out
> > command more verbose, e.g.
> > 
> >  'Skip CC', dest
> > 'Skip CXX', dest
> > 
> > etc
>

[jvoung - send to chromium..., 2012-03-22 17:44:08]

lgtm too in case you were waiting

On 2012/03/22 16:18:43, robertm wrote:
> LGTM
> 
> (assuming trybots are happy)
> 
>
https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
> File site_scons/site_tools/naclsdk.py (right):
> 
>
https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
> site_scons/site_tools/naclsdk.py:331: env.Replace(CC='true', CXX='true',
> LINK='true', AR='true',
> not sure. 
> another option could be CC='echo CC #' where the purpose of the '#' is to
ignore
> the command line.
> if the simple stuff does not work don't bother
> 
> On 2012/03/22 16:10:25, Derek Schuff wrote:
> > How would that work? Will it work if I just pass a python function here
> instead
> > of a shell command?
> > 
> > On 2012/03/22 14:09:37, robertm wrote:
> > > if it is not too much work it be nice to make the faked out
> > > command more verbose, e.g.
> > > 
> > >  'Skip CC', dest
> > > 'Skip CXX', dest
> > > 
> > > etc
> >

[(google.com) Derek Schuff, 2012-03-22 18:01:18]

was mostly just wating for the tree to open up again :)
commited r8091

On Thu, Mar 22, 2012 at 10:44 AM,  <jvoung@google.com> wrote:
> lgtm too in case you were waiting
>
>
> On 2012/03/22 16:18:43, robertm wrote:
>>
>> LGTM
>
>
>> (assuming trybots are happy)
>
>
>
>
https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
>>
>> File site_scons/site_tools/naclsdk.py (right):
>
>
>
>
https://chromiumcodereview.appspot.com/9824001/diff/10001/site_scons/site_too...
>>
>> site_scons/site_tools/naclsdk.py:331: env.Replace(CC='true', CXX='true',
>> LINK='true', AR='true',
>> not sure.
>> another option could be CC='echo CC #' where the purpose of the '#' is to
>
> ignore
>>
>> the command line.
>> if the simple stuff does not work don't bother
>
>
>> On 2012/03/22 16:10:25, Derek Schuff wrote:
>> > How would that work? Will it work if I just pass a python function here
>> instead
>> > of a shell command?
>> >
>> > On 2012/03/22 14:09:37, robertm wrote:
>> > > if it is not too much work it be nice to make the faked out
>> > > command more verbose, e.g.
>> > >
>> > >  'Skip CC', dest
>> > > 'Skip CXX', dest
>> > >
>> > > etc
>> >
>
>
>
>
> https://chromiumcodereview.appspot.com/9824001/

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To post to this group, send email to native-client-reviews@googlegroups.com.
To unsubscribe from this group, send email to
native-client-reviews+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/native-client-reviews?hl=en.