Add build target for nacl_io from the NaCl SDK.

Add build target for nacl_io from the NaCl SDK.

BUG=316353
R=noelallen
CC=bradnelson

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=235053

[Peter Lundblad, 2013-11-08 18:20:52]

Hi,

This is my attempt of supporting nacl_io in the chrome build system.  A few
notes:

- It's not hooked up to anything. I have another CL that compiles liblouis and
depends on this one, but I split it up to keep things managable.  I've been
sending this to the trybots with the new target hooked up in all.gyp (see
earlier patchsets).

- I had a conflict between mount.cc and syscalls/mount.c since the .o files end
up in the same directory. If you have better suggestions than renaming one of
the files, I am all ears.

- I had to add #ifdef guards around _GNU_SOURCE in  syscalls/fdopen.c to work
around a duplicate define error. I don't think this file's content is actually
used in newlib, but it is included in the sources list in the dsc file. Do you
think I should rather special case and remove in in the gyp file?

[Peter Lundblad, 2013-11-12 02:07:26]

Friendly ping. Any opinion on this CL?

[binji, 2013-11-12 19:41:42]

Sorry for the delay, we were pushing for the pnacl launch. :)

https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/native_...
File native_client_sdk/native_client_sdk_untrusted.gyp (right):

https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/native_...
native_client_sdk/native_client_sdk_untrusted.gyp:7:
'..//build/common_untrusted.gypi',
is // correct or a typo?

https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/bui...
File native_client_sdk/src/build_tools/dsc_info.py (right):

https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/bui...
native_client_sdk/src/build_tools/dsc_info.py:15: print >> sys.stderr,
'dsc_info: msg'
'dsc_info: %s' % msg

https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/bui...
native_client_sdk/src/build_tools/dsc_info.py:17: 
nit: two lines between top-level functions

https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/bui...
native_client_sdk/src/build_tools/dsc_info.py:32: def DoMain(argv):
we usually just have:

def main(args):

and pass args[1:] to parse_args

https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/bui...
native_client_sdk/src/build_tools/dsc_info.py:38: parser.add_option('-t',
'--target',
use positional args for this instead, and have a optional arg for the lib_dir.

https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/bui...
native_client_sdk/src/build_tools/dsc_info.py:42: parser.error('Wrong numer of
arguments')
sp: number

https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/bui...
native_client_sdk/src/build_tools/dsc_info.py:55: sys.exit(main(sys.argv))
for consistency, please add KeyboardInterrupt exception check (like build_sdk,
test_sdk, etc)

https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/lib...
File native_client_sdk/src/libraries/nacl_io/library.dsc (right):

https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/lib...
native_client_sdk/src/libraries/nacl_io/library.dsc:98:
"syscalls/syscall_mount.c",
comment on why this needs to be renamed?

[plundblad, 2013-11-12 23:17:58]

Hi, ptal.

binji@chromium.org writes:
> Sorry for the delay, we were pushing for the pnacl launch. :)

No worries, congrats on the launch!


>
https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/native_...
> File native_client_sdk/native_client_sdk_untrusted.gyp (right):
> 
>
https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/native_...
> native_client_sdk/native_client_sdk_untrusted.gyp:7:
> '..//build/common_untrusted.gypi',
> is // correct or a typo?

typo: done

>
https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/bui...
> File native_client_sdk/src/build_tools/dsc_info.py (right):
> 
>
https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/bui...
> native_client_sdk/src/build_tools/dsc_info.py:15: print >> sys.stderr,
> 'dsc_info: msg'
> 'dsc_info: %s' % msg

Thanks!

>
https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/bui...
> native_client_sdk/src/build_tools/dsc_info.py:17:
> nit: two lines between top-level functions

Done.

>
https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/bui...
> native_client_sdk/src/build_tools/dsc_info.py:32: def DoMain(argv):
> we usually just have:
> 
> def main(args):
> 
> and pass args[1:] to parse_args

I left DoMain and got rid of main( so that it still works inside gyp's
p_do_main construct.  If you feel stronlgy, I can change it and just invoke
python from within gyp instead.
DoMain gets the arg vector without the executable name, which is why it passes
the
whole array to the opt parser.

>
https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/bui...
> native_client_sdk/src/build_tools/dsc_info.py:38:
> parser.add_option('-t', '--target',
> use positional args for this instead, and have a optional arg for the
> lib_dir.

Changed.

>
https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/bui...
> native_client_sdk/src/build_tools/dsc_info.py:42: parser.error('Wrong
> numer of arguments')
> sp: number

Clarified message.

>
https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/bui...
> native_client_sdk/src/build_tools/dsc_info.py:55:
> sys.exit(main(sys.argv))
> for consistency, please add KeyboardInterrupt exception check (like
> build_sdk, test_sdk, etc)

Added.

>
https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/lib...
> File native_client_sdk/src/libraries/nacl_io/library.dsc (right):
> 
>
https://codereview.chromium.org/61763014/diff/10008/native_client_sdk/src/lib...
> native_client_sdk/src/libraries/nacl_io/library.dsc:98:
> "syscalls/syscall_mount.c",
> comment on why this needs to be renamed?

Done, assuming this file format allows python style comments.

> https://codereview.chromium.org/61763014/

-- 

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[binji, 2013-11-13 00:58:28]

https://codereview.chromium.org/61763014/diff/1280001/native_client_sdk/nativ...
File native_client_sdk/native_client_sdk_untrusted.gyp (right):

https://codereview.chromium.org/61763014/diff/1280001/native_client_sdk/nativ...
native_client_sdk/native_client_sdk_untrusted.gyp:13: 'target_name':
'nacl_io_untrusted',
this new gyp file isn't referenced anywhere -- doesn't it need to be added to
all.gyp, or something?

Ah -- I see you removed it from all.gyp. Why?

[binji, 2013-11-13 01:00:31]

On 2013/11/13 00:58:28, binji wrote:
>
https://codereview.chromium.org/61763014/diff/1280001/native_client_sdk/nativ...
> File native_client_sdk/native_client_sdk_untrusted.gyp (right):
> 
>
https://codereview.chromium.org/61763014/diff/1280001/native_client_sdk/nativ...
> native_client_sdk/native_client_sdk_untrusted.gyp:13: 'target_name':
> 'nacl_io_untrusted',
> this new gyp file isn't referenced anywhere -- doesn't it need to be added to
> all.gyp, or something?
> 
> Ah -- I see you removed it from all.gyp. Why?

Sorry, I had a comment about DoMain that was elided; you should do it the way
you had before -- I didn't realize at first that you needed DoMain so it could
be used by gyp. Just add a comment so other readers understand why there is an
extra function there. :)

[Peter Lundblad, 2013-11-13 01:03:21]

binji@chromium.org writes:
> 
>
https://codereview.chromium.org/61763014/diff/1280001/native_client_sdk/nativ...
> File native_client_sdk/native_client_sdk_untrusted.gyp (right):
> 
>
https://codereview.chromium.org/61763014/diff/1280001/native_client_sdk/nativ...
> native_client_sdk/native_client_sdk_untrusted.gyp:13: 'target_name':
> 'nacl_io_untrusted',
> this new gyp file isn't referenced anywhere -- doesn't it need to be
> added to all.gyp, or something?
> 
> Ah -- I see you removed it from all.gyp. Why?

I have another cl that builds the libluis braille translator and depends
on this (see my initial comments when I sent out this cl).  I've been including
it in all.gyp for the try bots, but left it out since it will be
a dependency after my next cl.  Should I also add it to all.gyp?

Thanks,
//Peter

> https://codereview.chromium.org/61763014/

-- 

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[binji, 2013-11-13 19:10:34]

On 2013/11/13 01:03:21, Peter Lundblad wrote:
> mailto:binji@chromium.org writes:
> > 
> >
>
https://codereview.chromium.org/61763014/diff/1280001/native_client_sdk/nativ...
> > File native_client_sdk/native_client_sdk_untrusted.gyp (right):
> > 
> >
>
https://codereview.chromium.org/61763014/diff/1280001/native_client_sdk/nativ...
> > native_client_sdk/native_client_sdk_untrusted.gyp:13: 'target_name':
> > 'nacl_io_untrusted',
> > this new gyp file isn't referenced anywhere -- doesn't it need to be
> > added to all.gyp, or something?
> > 
> > Ah -- I see you removed it from all.gyp. Why?
> 
> I have another cl that builds the libluis braille translator and depends
> on this (see my initial comments when I sent out this cl).  I've been
including
> it in all.gyp for the try bots, but left it out since it will be
> a dependency after my next cl.  Should I also add it to all.gyp?
> 
> Thanks,
> //Peter
> 
> > https://codereview.chromium.org/61763014/
> 
> -- 
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

Sorry, more email fails. :-)

Here was my response:
It might be useful, so the trybots will show that it compiles properly via gyp.

[Peter Lundblad, 2013-11-13 19:27:35]

PTAL

I added the new targets in all.gyp and switched dsc_info.py back
to having a main() function as well.  DoMain already has function documentation
that explains its purpose;)

Thanks,
//Peter


binji@chromium.org writes:
> On 2013/11/13 01:03:21, Peter Lundblad wrote:
> > mailto:binji@chromium.org writes:
> > >
> > >
> 
>
https://codereview.chromium.org/61763014/diff/1280001/native_client_sdk/nativ...
> > > File native_client_sdk/native_client_sdk_untrusted.gyp (right):
> > >
> > >
> 
>
https://codereview.chromium.org/61763014/diff/1280001/native_client_sdk/nativ...
> > > native_client_sdk/native_client_sdk_untrusted.gyp:13: 'target_name':
> > > 'nacl_io_untrusted',
> > > this new gyp file isn't referenced anywhere -- doesn't it need to be
> > > added to all.gyp, or something?
> > >
> > > Ah -- I see you removed it from all.gyp. Why?
> 
> > I have another cl that builds the libluis braille translator and depends
> > on this (see my initial comments when I sent out this cl).  I've been
> including
> > it in all.gyp for the try bots, but left it out since it will be
> > a dependency after my next cl.  Should I also add it to all.gyp?
> 
> > Thanks,
> > //Peter
> 
> > > https://codereview.chromium.org/61763014/
> 
> > --
> 
> > To unsubscribe from this group and stop receiving emails from it, send an
> email
> > to mailto:chromium-reviews+unsubscribe@chromium.org.
> 
> Sorry, more email fails. :-)
> 
> Here was my response:
> It might be useful, so the trybots will show that it compiles properly via  
> gyp.
> 
> https://codereview.chromium.org/61763014/

-- 

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[binji, 2013-11-13 19:34:06]

Apologies, I didn't notice the comment before. lgtm

[commit-bot: I haz the power, 2013-11-14 01:38:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/plundblad@chromium.org/61763014/1490001

[commit-bot: I haz the power, 2013-11-14 02:40:46]

Retried try job too often on mac for step(s) compile
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac&number...

[commit-bot: I haz the power, 2013-11-14 04:23:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/plundblad@chromium.org/61763014/1490001

[commit-bot: I haz the power, 2013-11-14 06:10:20]

Change committed as 235053

[Alexander Potapenko, 2013-11-14 10:01:59]

Hi all,

This broke the CrOS ASan x86 bot, see
http://chromegw.corp.google.com/i/chromium.memory/builders/Chromium%20OS%20%2...:

chromeos-chrome-33.0.1710.0_alpha-r1: ________ running '/usr/bin/python2.7
src/build/gyp_chromium' in '/home/chrome-bot/chrome_root'
chromeos-chrome-33.0.1710.0_alpha-r1: Updating projects from gyp files...
chromeos-chrome-33.0.1710.0_alpha-r1: gyp: Unable to find targets in build file
/home/chrome-bot/chrome_root/src/native_client_sdk/native_client_sdk_untrusted.gyp
chromeos-chrome-33.0.1710.0_alpha-r1: Error: Command /usr/bin/python2.7
src/build/gyp_chromium returned non-zero exit status 1 in
/home/chrome-bot/chrome_root
chromeos-chrome-33.0.1710.0_alpha-r1: Hook '/usr/bin/python2.7
src/build/gyp_chromium' took 33.86 secs

[binji, 2013-11-14 10:04:42]

On 2013/11/14 10:01:59, Alexander Potapenko wrote:
> Hi all,
> 
> This broke the CrOS ASan x86 bot, see
>
http://chromegw.corp.google.com/i/chromium.memory/builders/Chromium%2520OS%25...:
> 
> chromeos-chrome-33.0.1710.0_alpha-r1: ________ running '/usr/bin/python2.7
> src/build/gyp_chromium' in '/home/chrome-bot/chrome_root'
> chromeos-chrome-33.0.1710.0_alpha-r1: Updating projects from gyp files...
> chromeos-chrome-33.0.1710.0_alpha-r1: gyp: Unable to find targets in build
file
>
/home/chrome-bot/chrome_root/src/native_client_sdk/native_client_sdk_untrusted.gyp
> chromeos-chrome-33.0.1710.0_alpha-r1: Error: Command /usr/bin/python2.7
> src/build/gyp_chromium returned non-zero exit status 1 in
> /home/chrome-bot/chrome_root
> chromeos-chrome-33.0.1710.0_alpha-r1: Hook '/usr/bin/python2.7
> src/build/gyp_chromium' took 33.86 secs

Please see https://codereview.chromium.org/66163013/ for the fix.

[Alexander Potapenko, 2013-11-14 11:10:00]

On 2013/11/14 10:01:59, Alexander Potapenko wrote:
> Hi all,
> 
> This broke the CrOS ASan x86 bot, see
>
http://chromegw.corp.google.com/i/chromium.memory/builders/Chromium%2520OS%25...:
> 
> chromeos-chrome-33.0.1710.0_alpha-r1: ________ running '/usr/bin/python2.7
> src/build/gyp_chromium' in '/home/chrome-bot/chrome_root'
> chromeos-chrome-33.0.1710.0_alpha-r1: Updating projects from gyp files...
> chromeos-chrome-33.0.1710.0_alpha-r1: gyp: Unable to find targets in build
file
>
/home/chrome-bot/chrome_root/src/native_client_sdk/native_client_sdk_untrusted.gyp
> chromeos-chrome-33.0.1710.0_alpha-r1: Error: Command /usr/bin/python2.7
> src/build/gyp_chromium returned non-zero exit status 1 in
> /home/chrome-bot/chrome_root
> chromeos-chrome-33.0.1710.0_alpha-r1: Hook '/usr/bin/python2.7
> src/build/gyp_chromium' took 33.86 secs

Fixed by joaodasilva@ in r235114. Yay!