generate_gradle.py: Allow multiple --target and add --all

generate_gradle.py: Allow multiple --target and add --all

While --all does what it says, Android Studio was not happy trying to
import the swath of generated projects. The flag will be available only
for the adventurous :).

BUG=620034
NOTRY=true

Committed: https://crrev.com/e244c1ae9cd6729b0f89f33075aac99f71a678bf
Cr-Commit-Position: refs/heads/master@{#420426}

[agrieve, 2016-09-22 01:40:32]

Description was changed from

==========
generate_gradle.py: Allow multiple --target and add --all

While --all does what it says, Android Studio was not happy trying to
import the swath of generated projects. The flag will be available for
the adventurous :).

BUG=620034
==========

to

==========
generate_gradle.py: Allow multiple --target and add --all

While --all does what it says, Android Studio was not happy trying to
import the swath of generated projects. The flag will be available for
the adventurous :).

BUG=620034
==========

[agrieve, 2016-09-22 01:40:32]

agrieve@chromium.org changed reviewers:
+ jbudorick@chromium.org

[agrieve, 2016-09-22 01:40:39]

On 2016/09/22 01:40:32, agrieve wrote:
> mailto:agrieve@chromium.org changed reviewers:
> + mailto:jbudorick@chromium.org

[jbudorick, 2016-09-22 02:34:46]

https://codereview.chromium.org/2362613002/diff/1/build/android/gradle/genera...
File build/android/gradle/generate_gradle.py (right):

https://codereview.chromium.org/2362613002/diff/1/build/android/gradle/genera...
build/android/gradle/generate_gradle.py:73: if len(args) < 10:
Why < 10? What about only logging part of cmd if it's too big?

https://codereview.chromium.org/2362613002/diff/1/build/android/gradle/genera...
build/android/gradle/generate_gradle.py:87: if ':' in ninja_target and
ninja_target.endswith('__build_config'):
Why is this checking for ':'? We'd never hit a __build_config that doesn't have
one, and it seems like it takes more time to check than it'd take for
endswith...

https://codereview.chromium.org/2362613002/diff/1/build/android/gradle/genera...
build/android/gradle/generate_gradle.py:352: _RunNinja(output_dir,
['build.ninja'])
This is ... building build.ninja? I take it that's a no-op?

If you want to run gn, why not just run gn directly?

[agrieve, 2016-09-22 15:51:25]

Addressed comments, and also updated the docs & default targets.

https://codereview.chromium.org/2362613002/diff/1/build/android/gradle/genera...
File build/android/gradle/generate_gradle.py (right):

https://codereview.chromium.org/2362613002/diff/1/build/android/gradle/genera...
build/android/gradle/generate_gradle.py:73: if len(args) < 10:
On 2016/09/22 02:34:46, jbudorick wrote:
> Why < 10? What about only logging part of cmd if it's too big?

meh, removed the check. It shows only when running with -v, and it's not *that*
bad (~250 args)

https://codereview.chromium.org/2362613002/diff/1/build/android/gradle/genera...
build/android/gradle/generate_gradle.py:87: if ':' in ninja_target and
ninja_target.endswith('__build_config'):
On 2016/09/22 02:34:46, jbudorick wrote:
> Why is this checking for ':'? We'd never hit a __build_config that doesn't
have
> one, and it seems like it takes more time to check than it'd take for
> endswith...

It's because we're querying for ninja rather than GN for the list of targets
(added a comment to the top that we do this because it's faster). GN creates
non-fully-qualified aliases for unique target names (which is why you can "ninja
chrome_apk". Ensuring there's a : in it ensures we're skipping over these
aliases.

https://codereview.chromium.org/2362613002/diff/1/build/android/gradle/genera...
build/android/gradle/generate_gradle.py:352: _RunNinja(output_dir,
['build.ninja'])
On 2016/09/22 02:34:46, jbudorick wrote:
> This is ... building build.ninja? I take it that's a no-op?
> 
> If you want to run gn, why not just run gn directly?

Added a comment - it's faster than running gn gen in the no-op case.

[agrieve, 2016-09-22 15:52:01]

Description was changed from

==========
generate_gradle.py: Allow multiple --target and add --all

While --all does what it says, Android Studio was not happy trying to
import the swath of generated projects. The flag will be available for
the adventurous :).

BUG=620034
==========

to

==========
generate_gradle.py: Allow multiple --target and add --all

While --all does what it says, Android Studio was not happy trying to
import the swath of generated projects. The flag will be available only
for the adventurous :).

BUG=620034
==========

[jbudorick, 2016-09-22 15:57:44]

lgtm w/ q

https://codereview.chromium.org/2362613002/diff/1/build/android/gradle/genera...
File build/android/gradle/generate_gradle.py (right):

https://codereview.chromium.org/2362613002/diff/1/build/android/gradle/genera...
build/android/gradle/generate_gradle.py:87: if ':' in ninja_target and
ninja_target.endswith('__build_config'):
On 2016/09/22 15:51:25, agrieve wrote:
> On 2016/09/22 02:34:46, jbudorick wrote:
> > Why is this checking for ':'? We'd never hit a __build_config that doesn't
> have
> > one, and it seems like it takes more time to check than it'd take for
> > endswith...
> 
> It's because we're querying for ninja rather than GN for the list of targets
> (added a comment to the top that we do this because it's faster). GN creates
> non-fully-qualified aliases for unique target names (which is why you can
"ninja
> chrome_apk". Ensuring there's a : in it ensures we're skipping over these
> aliases.

Ah, yeah, I was thinking about the GN targets.

https://codereview.chromium.org/2362613002/diff/1/build/android/gradle/genera...
build/android/gradle/generate_gradle.py:352: _RunNinja(output_dir,
['build.ninja'])
On 2016/09/22 15:51:25, agrieve wrote:
> On 2016/09/22 02:34:46, jbudorick wrote:
> > This is ... building build.ninja? I take it that's a no-op?
> > 
> > If you want to run gn, why not just run gn directly?
> 
> Added a comment - it's faster than running gn gen in the no-op case. 

ah, sgtm

https://codereview.chromium.org/2362613002/diff/20001/build/android/gradle/ge...
File build/android/gradle/generate_gradle.py (right):

https://codereview.chromium.org/2362613002/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:368: targets = [re.sub(r'_test_apk$',
'_test_apk__apk', t) for t in targets]
just curious -- why the __apk target?

[agrieve, 2016-09-22 16:58:16]

https://codereview.chromium.org/2362613002/diff/20001/build/android/gradle/ge...
File build/android/gradle/generate_gradle.py (right):

https://codereview.chromium.org/2362613002/diff/20001/build/android/gradle/ge...
build/android/gradle/generate_gradle.py:368: targets = [re.sub(r'_test_apk$',
'_test_apk__apk', t) for t in targets]
On 2016/09/22 15:57:44, jbudorick wrote:
> just curious -- why the __apk target?

That's the name of the android_apk() target forinstrumentation_test_apk()s:
https://cs.chromium.org/chromium/src/build/config/android/rules.gni?q=file:ru...

It'd actually probably make more sense to just change the template so that the
android_apk() wasn't a sub-target. Maybe in a follow-up :P

[agrieve, 2016-09-22 16:58:20]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2016-09-22 16:58:46]

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-09-22 18:11:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-22 18:11:33]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[agrieve, 2016-09-22 19:28:44]

Description was changed from

==========
generate_gradle.py: Allow multiple --target and add --all

While --all does what it says, Android Studio was not happy trying to
import the swath of generated projects. The flag will be available only
for the adventurous :).

BUG=620034
==========

to

==========
generate_gradle.py: Allow multiple --target and add --all

While --all does what it says, Android Studio was not happy trying to
import the swath of generated projects. The flag will be available only
for the adventurous :).

BUG=620034
NOTRY=true
==========

[agrieve, 2016-09-22 19:29:09]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2016-09-22 19:29:32]

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-09-22 19:35:39]

Description was changed from

==========
generate_gradle.py: Allow multiple --target and add --all

While --all does what it says, Android Studio was not happy trying to
import the swath of generated projects. The flag will be available only
for the adventurous :).

BUG=620034
NOTRY=true
==========

to

==========
generate_gradle.py: Allow multiple --target and add --all

While --all does what it says, Android Studio was not happy trying to
import the swath of generated projects. The flag will be available only
for the adventurous :).

BUG=620034
NOTRY=true
==========

[commit-bot: I haz the power, 2016-09-22 19:35:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-22 19:40:55]

Description was changed from

==========
generate_gradle.py: Allow multiple --target and add --all

While --all does what it says, Android Studio was not happy trying to
import the swath of generated projects. The flag will be available only
for the adventurous :).

BUG=620034
NOTRY=true
==========

to

==========
generate_gradle.py: Allow multiple --target and add --all

While --all does what it says, Android Studio was not happy trying to
import the swath of generated projects. The flag will be available only
for the adventurous :).

BUG=620034
NOTRY=true

Committed: https://crrev.com/e244c1ae9cd6729b0f89f33075aac99f71a678bf
Cr-Commit-Position: refs/heads/master@{#420426}
==========

[commit-bot: I haz the power, 2016-09-22 19:40:56]

Patchset 2 (id:??) landed as
https://crrev.com/e244c1ae9cd6729b0f89f33075aac99f71a678bf
Cr-Commit-Position: refs/heads/master@{#420426}