Route the buiding of libnosys, libpthread through NaClSdkLibrary.

Route the buiding of libnosys, libpthread through NaClSdkLibrary.

This is more systematic and allows us to build those libraries in a shared form for pnacl.
Added a comment to src/untrusted/irt_stub/nacl.scons
explaining why we do not follow the pattern there.
Committed: https://src.chromium.org/viewvc/native_client?view=rev&revision=8687

[Robert Muth (chromium), 2012-05-21 20:29:46]

PTAL

[Roland McGrath, 2012-05-21 20:35:45]

https://chromiumcodereview.appspot.com/10416014/diff/4002/src/untrusted/irt_s...
File src/untrusted/irt_stub/nacl.scons (right):

https://chromiumcodereview.appspot.com/10416014/diff/4002/src/untrusted/irt_s...
src/untrusted/irt_stub/nacl.scons:8: libppapi_stub = env.NaClSdkLibrary(
It was an explicit choice not to build libppapi_stub as a shared library.  This
change gets it built as shared for the existing nacl-glibc toolchain, which we
do not want.  Nothing will actually use the shared libppapi_stub.so anyway
unless you were to change libppapi.so (the source file).

[Robert Muth (chromium), 2012-05-21 21:33:27]

https://chromiumcodereview.appspot.com/10416014/diff/4002/src/untrusted/irt_s...
File src/untrusted/irt_stub/nacl.scons (right):

https://chromiumcodereview.appspot.com/10416014/diff/4002/src/untrusted/irt_s...
src/untrusted/irt_stub/nacl.scons:8: libppapi_stub = env.NaClSdkLibrary(
It wont be built unless env.Bit('pnacl_shared_newlib') is true.

For PNaCl it would be nice to have a least the option to build
as many libraries as possible shared, especially the large ones.
This one is pretty small, I can take it out if you are worried 
about interference.

On 2012/05/21 20:35:45, Roland McGrath wrote:
> It was an explicit choice not to build libppapi_stub as a shared library. 
This
> change gets it built as shared for the existing nacl-glibc toolchain, which we
> do not want.  Nothing will actually use the shared libppapi_stub.so anyway
> unless you were to change libppapi.so (the source file).

[Roland McGrath, 2012-05-21 22:08:26]

https://chromiumcodereview.appspot.com/10416014/diff/4002/src/untrusted/irt_s...
File src/untrusted/irt_stub/nacl.scons (right):

https://chromiumcodereview.appspot.com/10416014/diff/4002/src/untrusted/irt_s...
src/untrusted/irt_stub/nacl.scons:8: libppapi_stub = env.NaClSdkLibrary(
On 2012/05/21 21:33:27, Robert Muth (chromium) wrote:
> It wont be built unless env.Bit('pnacl_shared_newlib') is true.

That's not true.  NaClSdkLibrary always affects --nacl_glibc mode.

> For PNaCl it would be nice to have a least the option to build
> as many libraries as possible shared, especially the large ones.
> This one is pretty small, I can take it out if you are worried 
> about interference.

The preference for making libraries shared is exactly the same for the
existing nacl-glibc case.  This is one we previously decided for good
reason was better left as always static.

[robertm, 2012-05-22 13:55:16]

https://chromiumcodereview.appspot.com/10416014/diff/4002/src/untrusted/irt_s...
File src/untrusted/irt_stub/nacl.scons (right):

https://chromiumcodereview.appspot.com/10416014/diff/4002/src/untrusted/irt_s...
src/untrusted/irt_stub/nacl.scons:8: libppapi_stub = env.NaClSdkLibrary(
On 2012/05/21 22:08:26, Roland McGrath wrote:
> On 2012/05/21 21:33:27, Robert Muth (chromium) wrote:
> > It wont be built unless env.Bit('pnacl_shared_newlib') is true.
> 
> That's not true.  NaClSdkLibrary always affects --nacl_glibc mode.


The keyword arg "no_shared_lib" was supposed to preserve the original behavior.
For libppapi_stub this is a moot point now since I reverted my changes and added
a comment.


> 
> > For PNaCl it would be nice to have a least the option to build
> > as many libraries as possible shared, especially the large ones.
> > This one is pretty small, I can take it out if you are worried 
> > about interference.
> 
> The preference for making libraries shared is exactly the same for the
> existing nacl-glibc case.  This is one we previously decided for good
> reason was better left as always static.

see above

[jvoung - send to chromium..., 2012-05-22 20:49:40]

https://chromiumcodereview.appspot.com/10416014/diff/14006/SConstruct
File SConstruct (right):

https://chromiumcodereview.appspot.com/10416014/diff/14006/SConstruct#newcode...
SConstruct:3421: if kwargs['no_shared_lib']:
Can't you still add a arg with a default value, instead of using kwargs, or is
that too confusing, e.g.,:

def foo(env, lib_name, no_shared_lib=False, *name, **kwargs):

https://chromiumcodereview.appspot.com/10416014/diff/14006/src/untrusted/nosy...
File src/untrusted/nosys/nacl.scons (right):

https://chromiumcodereview.appspot.com/10416014/diff/14006/src/untrusted/nosy...
src/untrusted/nosys/nacl.scons:92: no_shared_lib=not
env.Bit('pnacl_shared_newlib'))
Should probably leave a comment that this isn't generated for glibc -- same for
pthread.

[Robert Muth (chromium), 2012-05-22 21:16:36]

https://chromiumcodereview.appspot.com/10416014/diff/14006/SConstruct
File SConstruct (right):

https://chromiumcodereview.appspot.com/10416014/diff/14006/SConstruct#newcode...
SConstruct:3421: if kwargs['no_shared_lib']:
On 2012/05/22 20:49:40, jvoung wrote:
> Can't you still add a arg with a default value, instead of using kwargs, or is
> that too confusing, e.g.,:
> 
> def foo(env, lib_name, no_shared_lib=False, *name, **kwargs):
> 

Sadly you cannot mix args that way in python
The only clean alternative would be to add a real argument. But that would
require changing all NaClSdkLibrary() uses

https://chromiumcodereview.appspot.com/10416014/diff/14006/src/untrusted/nosy...
File src/untrusted/nosys/nacl.scons (right):

https://chromiumcodereview.appspot.com/10416014/diff/14006/src/untrusted/nosy...
src/untrusted/nosys/nacl.scons:92: no_shared_lib=not
env.Bit('pnacl_shared_newlib'))
On 2012/05/22 20:49:40, jvoung wrote:
> Should probably leave a comment that this isn't generated for glibc -- same
for
> pthread.

Done.

[jvoung - send to chromium..., 2012-05-22 23:22:12]

lgtm

https://chromiumcodereview.appspot.com/10416014/diff/14006/SConstruct
File SConstruct (right):

https://chromiumcodereview.appspot.com/10416014/diff/14006/SConstruct#newcode...
SConstruct:3421: if kwargs['no_shared_lib']:
On 2012/05/22 21:16:36, Robert Muth (chromium) wrote:
> On 2012/05/22 20:49:40, jvoung wrote:
> > Can't you still add a arg with a default value, instead of using kwargs, or
is
> > that too confusing, e.g.,:
> > 
> > def foo(env, lib_name, no_shared_lib=False, *name, **kwargs):
> > 
> 
> Sadly you cannot mix args that way in python
> The only clean alternative would be to add a real argument. But that would
> require changing all NaClSdkLibrary() uses

Well it worked on a simple example I tried, but there is probably some subtle
python rule where it would break so... yeah let's not mix them.

https://chromiumcodereview.appspot.com/10416014/diff/7007/src/untrusted/nosys...
File src/untrusted/nosys/nacl.scons (right):

https://chromiumcodereview.appspot.com/10416014/diff/7007/src/untrusted/nosys...
src/untrusted/nosys/nacl.scons:93: no_shared_lib=not
env.Bit('pnacl_shared_newlib'))
nit: A little bit odd having negation on both sides.
no_... = not ...

[robertm, 2012-05-22 23:26:37]

https://chromiumcodereview.appspot.com/10416014/diff/7007/src/untrusted/nosys...
File src/untrusted/nosys/nacl.scons (right):

https://chromiumcodereview.appspot.com/10416014/diff/7007/src/untrusted/nosys...
src/untrusted/nosys/nacl.scons:93: no_shared_lib=not
env.Bit('pnacl_shared_newlib'))
On 2012/05/22 23:22:13, jvoung wrote:
> nit: A little bit odd having negation on both sides.
> no_... = not ...

yeah I do not like it either but make the flag positive, e.g. do_build_shared
does not really work unless I change all instances of NaClSdkLibrary()