recipes: Add a GN flavor.

recipes: Add a GN flavor.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2189713003

Committed: https://skia.googlesource.com/skia/+/ba59595ea3b5c4a632ba5a67e47d7ac96d93ee39

[jcgregorio, 2016-07-27 17:58:51]

Description was changed from

==========
recipes: Add a GN flavor.

BUG=skia:
==========

to

==========
recipes: Add a GN flavor.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2189713003
==========

[borenet, 2016-07-27 19:51:36]

borenet@google.com changed reviewers:
+ borenet@google.com

[borenet, 2016-07-27 19:51:37]

https://codereview.chromium.org/2189713003/diff/20001/infra/bots/recipe_modul...
File infra/bots/recipe_modules/skia/gn_flavor.py (right):

https://codereview.chromium.org/2189713003/diff/20001/infra/bots/recipe_modul...
infra/bots/recipe_modules/skia/gn_flavor.py:15:
cwd=self._skia_api.m.path['checkout'])
Prefer cwd=self._skia_api.skia_dir here and below.

https://codereview.chromium.org/2189713003/diff/20001/infra/bots/recipe_modul...
infra/bots/recipe_modules/skia/gn_flavor.py:18: 'true' if self.configuration ==
'Debug' else 'false'
Does self.configuration exist?  It might need to be
self._skia_api.configuration.

https://codereview.chromium.org/2189713003/diff/20001/infra/bots/recipe_modul...
infra/bots/recipe_modules/skia/gn_flavor.py:19: )
Indent?

https://codereview.chromium.org/2189713003/diff/20001/infra/bots/recipe_modul...
infra/bots/recipe_modules/skia/gn_flavor.py:23: gn_gen = ['gn', 'gen', out_dir,
'--args=%s' % ' '.join(gn_args)]
It may not matter yet, but if it's "gn.exe" on windows this will need to pick
the right thing based on the builder platform.

[jcgregorio, 2016-07-27 20:01:01]

https://codereview.chromium.org/2189713003/diff/20001/infra/bots/recipe_modul...
File infra/bots/recipe_modules/skia/gn_flavor.py (right):

https://codereview.chromium.org/2189713003/diff/20001/infra/bots/recipe_modul...
infra/bots/recipe_modules/skia/gn_flavor.py:15:
cwd=self._skia_api.m.path['checkout'])
On 2016/07/27 at 19:51:37, borenet wrote:
> Prefer cwd=self._skia_api.skia_dir here and below.

Done.

https://codereview.chromium.org/2189713003/diff/20001/infra/bots/recipe_modul...
infra/bots/recipe_modules/skia/gn_flavor.py:18: 'true' if self.configuration ==
'Debug' else 'false'
On 2016/07/27 at 19:51:37, borenet wrote:
> Does self.configuration exist?  It might need to be
self._skia_api.configuration.

Done.

https://codereview.chromium.org/2189713003/diff/20001/infra/bots/recipe_modul...
infra/bots/recipe_modules/skia/gn_flavor.py:19: )
On 2016/07/27 at 19:51:37, borenet wrote:
> Indent?

Did in a different way.

https://codereview.chromium.org/2189713003/diff/20001/infra/bots/recipe_modul...
infra/bots/recipe_modules/skia/gn_flavor.py:23: gn_gen = ['gn', 'gen', out_dir,
'--args=%s' % ' '.join(gn_args)]
On 2016/07/27 at 19:51:37, borenet wrote:
> It may not matter yet, but if it's "gn.exe" on windows this will need to pick
the right thing based on the builder platform.

Fixed.

[mtklein, 2016-07-27 20:06:30]

mtklein@google.com changed reviewers:
+ mtklein@google.com

[mtklein, 2016-07-27 20:06:31]

https://codereview.chromium.org/2189713003/diff/140001/infra/bots/recipe_modu...
File infra/bots/recipe_modules/skia/gn_flavor.py (right):

https://codereview.chromium.org/2189713003/diff/140001/infra/bots/recipe_modu...
infra/bots/recipe_modules/skia/gn_flavor.py:32: ninja_cmd = ['ninja', '-C',
out_dir, 'skia']
Drop 'skia' so it builds everything?  (Or is that coming later?)

[jcgregorio, 2016-07-27 20:12:22]

https://codereview.chromium.org/2189713003/diff/140001/infra/bots/recipe_modu...
File infra/bots/recipe_modules/skia/gn_flavor.py (right):

https://codereview.chromium.org/2189713003/diff/140001/infra/bots/recipe_modu...
infra/bots/recipe_modules/skia/gn_flavor.py:32: ninja_cmd = ['ninja', '-C',
out_dir, 'skia']
On 2016/07/27 at 20:06:31, mtklein wrote:
> Drop 'skia' so it builds everything?  (Or is that coming later?)

Done.

[borenet, 2016-07-28 14:23:27]

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
File infra/bots/recipe_modules/skia/api.py (right):

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
infra/bots/recipe_modules/skia/api.py:70: return 'GN' in
builder_cfg.get('extra_config', '')
Not sure if we should just do a equals here.  We might accidentally apply this
flavor to Build-Some-Bot-IGNORE_STUFF or some such.

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
File infra/bots/recipe_modules/skia/gn_flavor.py (right):

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
infra/bots/recipe_modules/skia/gn_flavor.py:16: out_dir = 'out/%s' %
self._skia_api.configuration
use self.out_dir instead

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
infra/bots/recipe_modules/skia/gn_flavor.py:32: ninja_cmd = ['ninja', '-C',
out_dir]
'ninja' probably needs to be 'ninja.exe' on Windows, but maybe that can wait.

[mtklein, 2016-07-28 14:25:10]

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
File infra/bots/recipe_modules/skia/gn_flavor.py (right):

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
infra/bots/recipe_modules/skia/gn_flavor.py:27: gn_gen = [gn_exe, 'gen',
out_dir, '--args=%s' % ' '.join(gn_args)]
I suspect when we have more than one, we'll want

 --args="%s"

but we'll find out for sure next time we try to set an arg.

[jcgregorio, 2016-07-28 15:26:26]

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
File infra/bots/recipe_modules/skia/api.py (right):

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
infra/bots/recipe_modules/skia/api.py:70: return 'GN' in
builder_cfg.get('extra_config', '')
On 2016/07/28 at 14:23:27, borenet wrote:
> Not sure if we should just do a equals here.  We might accidentally apply this
flavor to Build-Some-Bot-IGNORE_STUFF or some such.

Done.

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
File infra/bots/recipe_modules/skia/gn_flavor.py (right):

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
infra/bots/recipe_modules/skia/gn_flavor.py:16: out_dir = 'out/%s' %
self._skia_api.configuration
On 2016/07/28 at 14:23:27, borenet wrote:
> use self.out_dir instead

Done.

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
infra/bots/recipe_modules/skia/gn_flavor.py:27: gn_gen = [gn_exe, 'gen',
out_dir, '--args=%s' % ' '.join(gn_args)]
On 2016/07/28 at 14:25:10, mtklein wrote:
> I suspect when we have more than one, we'll want
> 
>  --args="%s"
> 
> but we'll find out for sure next time we try to set an arg.

I was thinking args would just get added to gn_args, but we'll see how that
works as we add more args.

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
infra/bots/recipe_modules/skia/gn_flavor.py:32: ninja_cmd = ['ninja', '-C',
out_dir]
On 2016/07/28 at 14:23:27, borenet wrote:
> 'ninja' probably needs to be 'ninja.exe' on Windows, but maybe that can wait.

Windows can wait.

[mtklein, 2016-07-28 15:30:11]

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
File infra/bots/recipe_modules/skia/gn_flavor.py (right):

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
infra/bots/recipe_modules/skia/gn_flavor.py:27: gn_gen = [gn_exe, 'gen',
out_dir, '--args=%s' % ' '.join(gn_args)]
On 2016/07/28 15:26:25, jcgregorio wrote:
> On 2016/07/28 at 14:25:10, mtklein wrote:
> > I suspect when we have more than one, we'll want
> > 
> >  --args="%s"
> > 
> > but we'll find out for sure next time we try to set an arg.
> 
> I was thinking args would just get added to gn_args, but we'll see how that
> works as we add more args.

Yeah, I'm on board there.  I was worried about the ' '.join() part.

If we had gn_args = [ 'is_debug=true', 'cc=clang', 'cxx=clang++' ], we'd get 
'--args=is_debug=true cc=clang cxx=clang++'.  Is GN cool with that?

[mtklein, 2016-07-28 15:31:50]

Man it's cool that we can use the trybots for this.

[jcgregorio, 2016-07-28 15:33:17]

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
File infra/bots/recipe_modules/skia/gn_flavor.py (right):

https://codereview.chromium.org/2189713003/diff/160001/infra/bots/recipe_modu...
infra/bots/recipe_modules/skia/gn_flavor.py:27: gn_gen = [gn_exe, 'gen',
out_dir, '--args=%s' % ' '.join(gn_args)]
On 2016/07/28 at 15:30:11, mtklein wrote:
> On 2016/07/28 15:26:25, jcgregorio wrote:
> > On 2016/07/28 at 14:25:10, mtklein wrote:
> > > I suspect when we have more than one, we'll want
> > > 
> > >  --args="%s"
> > > 
> > > but we'll find out for sure next time we try to set an arg.
> > 
> > I was thinking args would just get added to gn_args, but we'll see how that
> > works as we add more args.
> 
> Yeah, I'm on board there.  I was worried about the ' '.join() part.
> 
> If we had gn_args = [ 'is_debug=true', 'cc=clang', 'cxx=clang++' ], we'd get 
> '--args=is_debug=true cc=clang cxx=clang++'.  Is GN cool with that?

Yeah, it is, I copied this from the pdfium flavor which does the same thing,
which I also initially found surprising.

[mtklein, 2016-07-28 15:34:18]

> Yeah, it is, I copied this from the pdfium flavor which does the same thing,
> which I also initially found surprising.

Neat.  I'm so used to broken tools that I'm freaked out by humane ones...

[jcgregorio, 2016-07-28 15:40:17]

On 2016/07/28 at 15:34:18, mtklein wrote:
> > Yeah, it is, I copied this from the pdfium flavor which does the same thing,
> > which I also initially found surprising.
> 
> Neat.  I'm so used to broken tools that I'm freaked out by humane ones...

The recipe works, the failures in the trybots are issues in the BUILD.gn itself.

[mtklein, 2016-07-28 15:48:47]

lgtm

[jcgregorio, 2016-07-28 15:49:29]

The CQ bit was checked by jcgregorio@google.com

[commit-bot: I haz the power, 2016-07-28 15:49:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-28 16:39:02]

Description was changed from

==========
recipes: Add a GN flavor.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2189713003
==========

to

==========
recipes: Add a GN flavor.

BUG=skia:
GOLD_TRYBOT_URL= https://gold.skia.org/search?issue=2189713003

Committed:
https://skia.googlesource.com/skia/+/ba59595ea3b5c4a632ba5a67e47d7ac96d93ee39
==========

[commit-bot: I haz the power, 2016-07-28 16:39:03]

Committed patchset #11 (id:200001) as
https://skia.googlesource.com/skia/+/ba59595ea3b5c4a632ba5a67e47d7ac96d93ee39