cr: Add support for GN build file generation

cr: Add support for GN build file generation

To use, invoke cr init with --generator gn:

$ cr init -o out_linux/Debug --generator gn
$ cr build ...
$ cr run ...
$ etc.

You can also pass in additional arguments:

$ cr init -o out_linux/Debug --generator gn -s GN_ARG_foo=bar

Note that gn args works normally, although you need to specify the out directory manually:

$ gn args out_linux/Debug

Committed: https://crrev.com/1c0cfffc940e5b4dfed4a528e2f8ee3d8157bc56
Cr-Commit-Position: refs/heads/master@{#325664}

[Sami, 2015-04-16 17:30:10]

skyostil@chromium.org changed reviewers:
+ primiano@chromium.org

[Sami, 2015-04-17 15:55:43]

skyostil@chromium.org changed reviewers:
+ petrcermak@chromium.org

[petrcermak, 2015-04-17 16:14:54]

Looks good overall. Just a couple of comments.

Petr

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/actions/gn.py
File tools/cr/cr/actions/gn.py (right):

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/actions/gn....
tools/cr/cr/actions/gn.py:15: """A prepare action that runs gyp whenever you
select an output directory."""
s/gyp/gn/ :-)

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/actions/gn....
tools/cr/cr/actions/gn.py:58: if key in args:
As discussed offline, it would probably be better to do:

  if key not in args:
    arg_lines.append(line.strip())

If the key is in args, then it will be added in the next loop on line 66.

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/actions/gn....
tools/cr/cr/actions/gn.py:59: arg_lines.append('%s = %s' % (key, value))
s/value/args[key]/

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/base/androi...
File tools/cr/cr/base/android.py (right):

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/base/androi...
tools/cr/cr/base/android.py:42: GN_ARG_os='"android"'
Shouldn't it be GN_ARG_OS (capital "OS")?

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/commands/pr...
File tools/cr/cr/commands/prepare.py (right):

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/commands/pr...
tools/cr/cr/commands/prepare.py:56: help='Sets the build file generator to use.
Overrides ' + cls.SELECTOR
nit: Should probably add a full stop at the end of the help string. Perhaps use:
'[...]. Overrides %s.' % cls.Selector

[Sami, 2015-04-17 16:20:34]

Thanks, all addressed.

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/actions/gn.py
File tools/cr/cr/actions/gn.py (right):

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/actions/gn....
tools/cr/cr/actions/gn.py:15: """A prepare action that runs gyp whenever you
select an output directory."""
On 2015/04/17 16:14:54, petrcermak wrote:
> s/gyp/gn/ :-)

Gah, missed one, fixed :)

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/actions/gn....
tools/cr/cr/actions/gn.py:58: if key in args:
On 2015/04/17 16:14:54, petrcermak wrote:
> As discussed offline, it would probably be better to do:
> 
>   if key not in args:
>     arg_lines.append(line.strip())
> 
> If the key is in args, then it will be added in the next loop on line 66.

Yep, done.

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/actions/gn....
tools/cr/cr/actions/gn.py:59: arg_lines.append('%s = %s' % (key, value))
On 2015/04/17 16:14:54, petrcermak wrote:
> s/value/args[key]/

Done.

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/base/androi...
File tools/cr/cr/base/android.py (right):

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/base/androi...
tools/cr/cr/base/android.py:42: GN_ARG_os='"android"'
On 2015/04/17 16:14:54, petrcermak wrote:
> Shouldn't it be GN_ARG_OS (capital "OS")?

Unlike in gyp, gn attributes are generally lower case (but not sure if always so
I opted to not to do any automatic case correction).

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/commands/pr...
File tools/cr/cr/commands/prepare.py (right):

https://codereview.chromium.org/1095613002/diff/20001/tools/cr/cr/commands/pr...
tools/cr/cr/commands/prepare.py:56: help='Sets the build file generator to use.
Overrides ' + cls.SELECTOR
On 2015/04/17 16:14:54, petrcermak wrote:
> nit: Should probably add a full stop at the end of the help string. Perhaps
use:
> '[...]. Overrides %s.' % cls.Selector

Done.

[petrcermak, 2015-04-17 16:53:14]

LGTM % 3 nits. Thanks :-)

https://codereview.chromium.org/1095613002/diff/40001/tools/cr/cr/actions/gn.py
File tools/cr/cr/actions/gn.py (right):

https://codereview.chromium.org/1095613002/diff/40001/tools/cr/cr/actions/gn....
tools/cr/cr/actions/gn.py:33: gn_args += ' is_debug=%s' % \
I think it would be better to use brackets rather than backslash here. I grep'd
through the cr codebase and there were no occurences of this pattern.

https://codereview.chromium.org/1095613002/diff/40001/tools/cr/cr/actions/gn....
tools/cr/cr/actions/gn.py:58: if not key in args:
nit: "not in" is more readable (I know I suggested this change, sorry)

https://codereview.chromium.org/1095613002/diff/40001/tools/cr/cr/commands/pr...
File tools/cr/cr/commands/prepare.py (right):

https://codereview.chromium.org/1095613002/diff/40001/tools/cr/cr/commands/pr...
tools/cr/cr/commands/prepare.py:56: help='Sets the build file generator to use.
' + \
Again, I think it would be better to use brackets rather than backslash here.

[Sami, 2015-04-17 16:57:17]

Thanks!

https://codereview.chromium.org/1095613002/diff/40001/tools/cr/cr/actions/gn.py
File tools/cr/cr/actions/gn.py (right):

https://codereview.chromium.org/1095613002/diff/40001/tools/cr/cr/actions/gn....
tools/cr/cr/actions/gn.py:33: gn_args += ' is_debug=%s' % \
On 2015/04/17 16:53:14, petrcermak wrote:
> I think it would be better to use brackets rather than backslash here. I
grep'd
> through the cr codebase and there were no occurences of this pattern.

Done.

https://codereview.chromium.org/1095613002/diff/40001/tools/cr/cr/actions/gn....
tools/cr/cr/actions/gn.py:58: if not key in args:
On 2015/04/17 16:53:14, petrcermak wrote:
> nit: "not in" is more readable (I know I suggested this change, sorry)

Yeah, that reads a bit better. Done.

[Sami, 2015-04-17 16:57:35]

https://codereview.chromium.org/1095613002/diff/40001/tools/cr/cr/commands/pr...
File tools/cr/cr/commands/prepare.py (right):

https://codereview.chromium.org/1095613002/diff/40001/tools/cr/cr/commands/pr...
tools/cr/cr/commands/prepare.py:56: help='Sets the build file generator to use.
' + \
On 2015/04/17 16:53:14, petrcermak wrote:
> Again, I think it would be better to use brackets rather than backslash here.

Done.

[Sami, 2015-04-17 16:57:40]

The CQ bit was checked by skyostil@chromium.org

[Sami, 2015-04-17 16:57:40]

The patchset sent to the CQ was uploaded after l-g-t-m from
petrcermak@chromium.org
Link to the patchset: https://codereview.chromium.org/1095613002/#ps60001
(title: "Nits addressed.")

[commit-bot: I haz the power, 2015-04-17 16:58:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1095613002/60001

[commit-bot: I haz the power, 2015-04-17 17:35:17]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-17 17:36:58]

Patchset 4 (id:??) landed as
https://crrev.com/1c0cfffc940e5b4dfed4a528e2f8ee3d8157bc56
Cr-Commit-Position: refs/heads/master@{#325664}