[Devil] Replace generated Devil config with jinja template.

[Devil] Replace generated Devil config with jinja template.

This change will hopefully allow us to configure devil more based
on the build config. For example, it will let us use the same
android_sdk_tools to run tests that we use to build with.

BUG=

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

[mikecase (-- gone --), 2016-03-18 23:42:36]

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

[mikecase (-- gone --), 2016-03-18 23:42:37]

Wanted your opinion on this change before I finish it. This CL is not finished
so don't bother looking too closely. But what do you think about using a jinja
template to generate the devil_chromium.json file? This will let us plug in the
android_sdk_path we use to build into the
config. It can also plug in the output_dir so we can remove some of the
generated devil configs from devil_chromium.py

[jbudorick, 2016-03-18 23:50:33]

On 2016/03/18 23:42:37, mikecase wrote:
> Wanted your opinion on this change before I finish it. This CL is not finished
> so don't bother looking too closely. But what do you think about using a jinja
> template to generate the devil_chromium.json file? This will let us plug in
the
> android_sdk_path we use to build into the
> config. It can also plug in the output_dir so we can remove some of the
> generated devil configs from devil_chromium.py

👍

[jbudorick, 2016-03-18 23:55:43]

On 2016/03/18 23:50:33, jbudorick wrote:
> On 2016/03/18 23:42:37, mikecase wrote:
> > Wanted your opinion on this change before I finish it. This CL is not
finished
> > so don't bother looking too closely. But what do you think about using a
jinja
> > template to generate the devil_chromium.json file? This will let us plug in
> the
> > android_sdk_path we use to build into the
> > config. It can also plug in the output_dir so we can remove some of the
> > generated devil configs from devil_chromium.py
> 
> 👍

and by that, I mean that this is much better than the existing implementation,
please continue.

[mikecase (-- gone --), 2016-03-21 18:45:49]

Updated CL. Tested running tests in both gyp and gn and everything seems to work
as expected.

The only ugly thing about this CL imo is that I don't see a way to not leave
devil_chromium.json. So I have devil_chromium.jinja which is used if you build
and a 
default config devil_chromium.json is you haven't built.

[jbudorick, 2016-03-21 18:52:35]

On 2016/03/21 18:45:49, mikecase wrote:
> Updated CL. Tested running tests in both gyp and gn and everything seems to
work
> as expected.
> 
> The only ugly thing about this CL imo is that I don't see a way to not leave
> devil_chromium.json. So I have devil_chromium.jinja which is used if you build
> and a 
> default config devil_chromium.json is you haven't built.

Can you expand on this? Why can't we remove it and just use the things in the
default devil config if you haven't built?

[mikecase (-- gone --), 2016-03-21 21:26:11]

On 2016/03/21 at 18:52:35, jbudorick wrote:
> On 2016/03/21 18:45:49, mikecase wrote:
> > Updated CL. Tested running tests in both gyp and gn and everything seems to
work
> > as expected.
> > 
> > The only ugly thing about this CL imo is that I don't see a way to not leave
> > devil_chromium.json. So I have devil_chromium.jinja which is used if you
build
> > and a 
> > default config devil_chromium.json is you haven't built.
> 
> Can you expand on this? Why can't we remove it and just use the things in the
default devil config if you haven't built?

Updated a new patch where I deleted devil_chromium.json. This will result in a
slight behavior change in that if you haven't
built anything you will now use the dependencies downloaded from Devil instead
of those listed in devil_chromium.json.

If you have built, you will use the dependencies generated from the jinja file.

[jbudorick, 2016-03-24 18:26:26]

lgtm w/ logging nit

This is _much_ better than my previous implementation.

https://codereview.chromium.org/1812383003/diff/40001/build/android/devil_chr...
File build/android/devil_chromium.py (right):

https://codereview.chromium.org/1812383003/diff/40001/build/android/devil_chr...
build/android/devil_chromium.py:49: config_files = [generated_config_file]
This should log a warning if generated_config_file doesn't exist.

https://codereview.chromium.org/1812383003/diff/40001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1812383003/diff/40001/build/common.gypi#newco...
build/common.gypi:1833: 'android_sdk_build_tools_version%':
'<(android_sdk_build_tools_version)',
rebase, this exists now

[jbudorick, 2016-03-24 18:27:39]

https://codereview.chromium.org/1812383003/diff/40001/build/android/devil_chr...
File build/android/devil_chromium.jinja (right):

https://codereview.chromium.org/1812383003/diff/40001/build/android/devil_chr...
build/android/devil_chromium.jinja:74: "forwarder_device": {
wait, this is wrong. should be forwarder_host

[jbudorick, 2016-03-24 18:28:43]

https://codereview.chromium.org/1812383003/diff/40001/build/android/devil_chr...
File build/android/devil_chromium.jinja (right):

https://codereview.chromium.org/1812383003/diff/40001/build/android/devil_chr...
build/android/devil_chromium.jinja:47: "android_arm64-v8a": {
and actually, we know the device architecture at build time. you should be able
to only generate one of these.

https://codereview.chromium.org/1812383003/diff/40001/build/android/devil_chr...
build/android/devil_chromium.jinja:85: "android_armeabi-v7a": {
(same here)

[mikecase (-- gone --), 2016-03-28 20:47:01]

Removed the "../../.."'s that I had in my jinja template.

I also realized I had some mistakes in the paths that I was passing to the jinja
template processor. I wasn't basically need to just pass all the paths relative
to <output_dir>/gen which I wasn't going.

This is super easy to do in GN. (just added some calls to rebase_path(<path>,
<output_dir>/gen)).

This is basically impossible to do in GYP because the <output_dir> is not known
at GYP time. This only way I think this is possible is to create a wrapper
around the jinja_template.py that processes the <output_dir>. This would be
processed when you run ninja. Unfortunately, this is pretty ugly since I can't
reuse the jinja_template.gypi file.

https://codereview.chromium.org/1812383003/diff/40001/build/android/devil_chr...
File build/android/devil_chromium.jinja (right):

https://codereview.chromium.org/1812383003/diff/40001/build/android/devil_chr...
build/android/devil_chromium.jinja:74: "forwarder_device": {
On 2016/03/24 at 18:27:38, jbudorick wrote:
> wait, this is wrong. should be forwarder_host

Done

https://codereview.chromium.org/1812383003/diff/40001/build/android/devil_chr...
File build/android/devil_chromium.py (right):

https://codereview.chromium.org/1812383003/diff/40001/build/android/devil_chr...
build/android/devil_chromium.py:49: config_files = [generated_config_file]
On 2016/03/24 at 18:26:26, jbudorick wrote:
> This should log a warning if generated_config_file doesn't exist.

Added logging

https://codereview.chromium.org/1812383003/diff/40001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1812383003/diff/40001/build/common.gypi#newco...
build/common.gypi:1833: 'android_sdk_build_tools_version%':
'<(android_sdk_build_tools_version)',
On 2016/03/24 at 18:26:26, jbudorick wrote:
> rebase, this exists now

Woohoo! Rebased.

[mikecase (-- gone --), 2016-03-28 20:47:35]

https://codereview.chromium.org/1812383003/diff/60001/build/android/gyp_devil...
File build/android/gyp_devil_jinja_processor.py (right):

https://codereview.chromium.org/1812383003/diff/60001/build/android/gyp_devil...
build/android/gyp_devil_jinja_processor.py:29: rebased_product_dir =
os.path.relpath(args.product_dir, gen_dir)
Basically, these two calls is all I need this entire script for  :/

[mikecase (-- gone --), 2016-03-31 18:53:13]

friendly ping. 

Does my work around with the gyp_devil_jinja_processor script look reasonably to
you. It would
be deleted as soon as gyp goes away.

[jbudorick, 2016-04-01 04:17:01]

On 2016/03/31 18:53:13, mikecase wrote:
> friendly ping. 

lgtm

> 
> Does my work around with the gyp_devil_jinja_processor script look reasonably
to
> you. It would
> be deleted as soon as gyp goes away.

Yeah, I suppose so.

[mikecase (-- gone --), 2016-04-05 17:43:54]

The CQ bit was checked by mikecase@chromium.org

[commit-bot: I haz the power, 2016-04-05 17:44:19]

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

[commit-bot: I haz the power, 2016-04-05 18:16:55]

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

[commit-bot: I haz the power, 2016-04-05 18:16:56]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[mikecase (-- gone --), 2016-04-05 21:05:52]

mikecase@chromium.org changed reviewers:
+ droger@chromium.org

[mikecase (-- gone --), 2016-04-05 21:05:53]

+droger for review of tools/android/loading/gce/deploy.sh

Im removing build/android/devil_chromium.json (and replacing it with a generated
out_dir/gen/devil_chromium.json). Not really sure how the deploy.sh script
works.
Is Chromium built when the deploy script runs?

Can I replace...

build/android/devil_chromium.json with $build_dir/gen/devil_chromium.json

...in the script? For now I just removed the reference to devil_chromium.json
from
the file.


To jbudorick

Also updated build/android/android.isolate with the new path to
devil_chromium.json

[jbudorick, 2016-04-05 21:10:49]

https://codereview.chromium.org/1812383003/diff/80001/build/android/BUILD.gn
File build/android/BUILD.gn (right):

https://codereview.chromium.org/1812383003/diff/80001/build/android/BUILD.gn#...
build/android/BUILD.gn:89: output = "$root_gen_dir/devil_chromium.json"
Does this output file wind up in the data for test_runner_py? It should.

https://codereview.chromium.org/1812383003/diff/80001/build/android/android.i...
File build/android/android.isolate (right):

https://codereview.chromium.org/1812383003/diff/80001/build/android/android.i...
build/android/android.isolate:20: '<(PRODUCT_DIR)/gen/devil_chromium.py',
nit: alphabetize

[droger, 2016-04-06 09:35:41]

On 2016/04/05 21:05:53, mikecase wrote:
> +droger for review of tools/android/loading/gce/deploy.sh
> 
> Im removing build/android/devil_chromium.json (and replacing it with a
generated
> out_dir/gen/devil_chromium.json). Not really sure how the deploy.sh script
> works.
> Is Chromium built when the deploy script runs?
> 
> Can I replace...
> 
> build/android/devil_chromium.json with $build_dir/gen/devil_chromium.json
> 
> ...in the script? For now I just removed the reference to devil_chromium.json
> from
> the file.

Thanks for the heads up.
You can probably replace it (be careful it's $builddir, not $build_dir).

However, I don't know where it should be copied.
I need this because devil_chromium.py needs it, but I am not familiar with this
code at all.
I guess it has to be copied somewhere where the python script will find it.

[droger, 2016-04-06 09:37:48]

Also: lgtm. I don't want to block you on this.

[mikecase (-- gone --), 2016-04-08 17:20:05]

Added line to copy the generated devil config to $tmp_src_dir/$builddir/gen/ in
deploy.sh.

As long as the output_directoriy varible is set to $tmp_src_dir/$builddir/ when
running
devil_chromium.Initialize(output_directory=), then it should just find this
config.

https://codereview.chromium.org/1812383003/diff/80001/build/android/BUILD.gn
File build/android/BUILD.gn (right):

https://codereview.chromium.org/1812383003/diff/80001/build/android/BUILD.gn#...
build/android/BUILD.gn:89: output = "$root_gen_dir/devil_chromium.json"
On 2016/04/05 at 21:10:49, jbudorick (ooo until apr 18) wrote:
> Does this output file wind up in the data for test_runner_py? It should.

Added this file to the data in test_runner_py.

https://codereview.chromium.org/1812383003/diff/80001/build/android/android.i...
File build/android/android.isolate (right):

https://codereview.chromium.org/1812383003/diff/80001/build/android/android.i...
build/android/android.isolate:20: '<(PRODUCT_DIR)/gen/devil_chromium.py',
On 2016/04/05 at 21:10:49, jbudorick (ooo until apr 18) wrote:
> nit: alphabetize

Done

[mikecase (-- gone --), 2016-04-08 17:20:13]

The CQ bit was checked by mikecase@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-08 17:20:35]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1812383003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1812383003/100001

[commit-bot: I haz the power, 2016-04-08 18:04:23]

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

[commit-bot: I haz the power, 2016-04-08 18:04:24]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[mikecase (-- gone --), 2016-04-08 18:36:35]

The CQ bit was checked by mikecase@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-08 18:37:35]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1812383003/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1812383003/120001

[commit-bot: I haz the power, 2016-04-08 20:58:35]

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

[commit-bot: I haz the power, 2016-04-08 20:58:37]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[mikecase (-- gone --), 2016-04-11 18:40:49]

Fixed android platform names being...
{{android_app_abi}}
...instead of ....
android_{{android_app_abi}}

Going to try committing this agian.

[mikecase (-- gone --), 2016-04-11 18:40:51]

The CQ bit was checked by mikecase@chromium.org

[mikecase (-- gone --), 2016-04-11 18:40:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
jbudorick@chromium.org, droger@chromium.org
Link to the patchset: https://codereview.chromium.org/1812383003/#ps140001
(title: "Fixed android platform name.")

[commit-bot: I haz the power, 2016-04-11 18:41:57]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1812383003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1812383003/140001

[commit-bot: I haz the power, 2016-04-11 20:26:50]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-04-11 20:28:30]

Description was changed from

==========
[Devil] Replace generated Devil config with jinja template.

This change will hopefully allow us to configure devil more based
on the build config. For example, it will let us use the same
android_sdk_tools to run tests that we use to build with.

BUG=
==========

to

==========
[Devil] Replace generated Devil config with jinja template.

This change will hopefully allow us to configure devil more based
on the build config. For example, it will let us use the same
android_sdk_tools to run tests that we use to build with.

BUG=

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

[commit-bot: I haz the power, 2016-04-11 20:28:31]

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

[kjellander_chromium, 2016-04-12 10:57:47]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/1885503002/ by kjellander@chromium.org.

The reason for reverting is: I believe this change breaks content_browsertests
on every Android tester in chromium.android. 

Examples:

https://build.chromium.org/p/chromium.android/builders/Lollipop%20Phone%20Tes...
https://build.chromium.org/p/chromium.android/builders/Marshmallow%2064%20bit...
https://build.chromium.org/p/chromium.android/builders/KitKat%20Tablet%20Test....