Add script to validate installed Android SDK packages

Add script to validate installed Android SDK packages

This script will allow to verify at build time that the local
setup properly contains Google Play services at a given version.

BUG=490091
Committed: https://crrev.com/e17297a2f21f653f3e79544fad7ef7bc83b4f8ba
Cr-Commit-Position: refs/heads/master@{#336350}

[dgn, 2015-05-22 13:26:59]

dgn@chromium.org changed reviewers:
+ aberent@chromium.org, andrewhayden@chromium.org, cjhopman@chromium.org

[dgn, 2015-05-22 13:27:01]

cjhopman@: PTAL
aberent@, andrewhayden@: FYI

https://codereview.chromium.org/1152203003/diff/1/build/android_sdk_extras.json
File build/android_sdk_extras.json (right):

https://codereview.chromium.org/1152203003/diff/1/build/android_sdk_extras.js...
build/android_sdk_extras.json:7: "package_id":
"extra-google-google_play_services"
Added package_id to be able to directly provide packages to download without
having to parse the output of the sdkmanager's interactive mode

https://codereview.chromium.org/1152203003/diff/1/build/android_sdk_extras.js...
build/android_sdk_extras.json:14: "package_id": "extra-google-m2repository"
This is to prepare for using split clients for the Google Play services library

[dgn, 2015-05-22 13:44:02]

PTAL

How can I check that the zip for the desired packages and versions is available
for the bots? I don't seem to have access to that bucket with gsutil

[cjhopman, 2015-05-22 20:25:22]

https://codereview.chromium.org/1152203003/diff/20001/build/android_sdk_extra...
File build/android_sdk_extras.json (right):

https://codereview.chromium.org/1152203003/diff/20001/build/android_sdk_extra...
build/android_sdk_extras.json:3: "dir_name": "google",
Are the changes to the packages required in this change? Can we do that
separately?

https://codereview.chromium.org/1152203003/diff/20001/build/check_play_servic...
File build/check_play_services_version.py (right):

https://codereview.chromium.org/1152203003/diff/20001/build/check_play_servic...
build/check_play_services_version.py:25: UPDATE_NEEDED_MSG = ('Google Play
services package version {required_version} '
Places in this file explicitly refer to google play services as the package and
other places refer to m2repo. These references should be made generic if we want
this to be used for different packages.

https://codereview.chromium.org/1152203003/diff/20001/build/check_play_servic...
build/check_play_services_version.py:31: BOLD_STYLE = '\033[1m'
I don't think I'd hardcode in these escape codes. You could use
third_party/colorama if you really want to style the output.

https://codereview.chromium.org/1152203003/diff/20001/build/check_play_servic...
build/check_play_services_version.py:34: def print_and_exit(msg):
python functions use CapWords() style

[Andrew Hayden (chromium.org), 2015-05-28 12:55:01]

https://codereview.chromium.org/1152203003/diff/1/build/check_play_services_v...
File build/check_play_services_version.py (right):

https://codereview.chromium.org/1152203003/diff/1/build/check_play_services_v...
build/check_play_services_version.py:2: # Copyright 2013 The Chromium Authors.
All rights reserved.
It's totally 2015, man.

[dgn, 2015-06-08 14:34:10]

Thanks for your comments. PTAL at the updated patch.

https://codereview.chromium.org/1152203003/diff/1/build/check_play_services_v...
File build/check_play_services_version.py (right):

https://codereview.chromium.org/1152203003/diff/1/build/check_play_services_v...
build/check_play_services_version.py:2: # Copyright 2013 The Chromium Authors.
All rights reserved.
On 2015/05/28 at 12:55:00, Andrew Hayden wrote:
> It's totally 2015, man.

Done.

https://codereview.chromium.org/1152203003/diff/20001/build/android_sdk_extra...
File build/android_sdk_extras.json (right):

https://codereview.chromium.org/1152203003/diff/20001/build/android_sdk_extra...
build/android_sdk_extras.json:3: "dir_name": "google",
On 2015/05/22 at 20:25:22, cjhopman wrote:
> Are the changes to the packages required in this change? Can we do that
separately?

adding package_id is the only change that would be required. I removed the rest.

https://codereview.chromium.org/1152203003/diff/20001/build/check_play_servic...
File build/check_play_services_version.py (right):

https://codereview.chromium.org/1152203003/diff/20001/build/check_play_servic...
build/check_play_services_version.py:25: UPDATE_NEEDED_MSG = ('Google Play
services package version {required_version} '
On 2015/05/22 at 20:25:22, cjhopman wrote:
> Places in this file explicitly refer to google play services as the package
and other places refer to m2repo. These references should be made generic if we
want this to be used for different packages.

Done.

https://codereview.chromium.org/1152203003/diff/20001/build/check_play_servic...
build/check_play_services_version.py:31: BOLD_STYLE = '\033[1m'
On 2015/05/22 at 20:25:22, cjhopman wrote:
> I don't think I'd hardcode in these escape codes. You could use
third_party/colorama if you really want to style the output.

Done.

https://codereview.chromium.org/1152203003/diff/20001/build/check_play_servic...
build/check_play_services_version.py:34: def print_and_exit(msg):
On 2015/05/22 at 20:25:22, cjhopman wrote:
> python functions use CapWords() style

Done.

[dgn, 2015-06-11 16:23:30]

Hello Chris, could you PTAL?

Since the bots download the v21 package and the version has not changed, the
check I added to the gn should not do anything. I'll add the check to the gyp
files in another CL, since it's a different repo.

https://codereview.chromium.org/1152203003/diff/60001/build/secondary/third_p...
File build/secondary/third_party/android_tools/BUILD.gn (right):

https://codereview.chromium.org/1152203003/diff/60001/build/secondary/third_p...
build/secondary/third_party/android_tools/BUILD.gn:89: deps = [
gyp check: https://codereview.appspot.com/243130043

[cjhopman, 2015-06-17 21:14:32]

On 2015/06/11 16:23:30, dgn wrote:
> Hello Chris, could you PTAL?
> 
> Since the bots download the v21 package and the version has not changed, the
> check I added to the gn should not do anything. I'll add the check to the gyp
> files in another CL, since it's a different repo.
> 
>
https://codereview.chromium.org/1152203003/diff/60001/build/secondary/third_p...
> File build/secondary/third_party/android_tools/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1152203003/diff/60001/build/secondary/third_p...
> build/secondary/third_party/android_tools/BUILD.gn:89: deps = [
> gyp check: https://codereview.appspot.com/243130043

Does Android allow you to get old versions of things like google play services?
If we say that you need v21, and the sdk manager only provides 25, how is a user
supposed to handle that case?

[cjhopman, 2015-06-17 21:15:16]

On 2015/06/17 21:14:32, cjhopman wrote:
> On 2015/06/11 16:23:30, dgn wrote:
> > Hello Chris, could you PTAL?
> > 
> > Since the bots download the v21 package and the version has not changed, the
> > check I added to the gn should not do anything. I'll add the check to the
gyp
> > files in another CL, since it's a different repo.
> > 
> >
>
https://codereview.chromium.org/1152203003/diff/60001/build/secondary/third_p...
> > File build/secondary/third_party/android_tools/BUILD.gn (right):
> > 
> >
>
https://codereview.chromium.org/1152203003/diff/60001/build/secondary/third_p...
> > build/secondary/third_party/android_tools/BUILD.gn:89: deps = [
> > gyp check: https://codereview.appspot.com/243130043
> 
> Does Android allow you to get old versions of things like google play
services?
> If we say that you need v21, and the sdk manager only provides 25, how is a
user
> supposed to handle that case?

Nevermind, we use it as just a minimum version I see.

[cjhopman, 2015-06-17 21:18:45]

https://codereview.chromium.org/1152203003/diff/100001/build/check_sdk_extras...
File build/check_sdk_extras_version.py (right):

https://codereview.chromium.org/1152203003/diff/100001/build/check_sdk_extras...
build/check_sdk_extras_version.py:9: that it is at an appropriate version.
Clarify that we just check a minimum version.

https://codereview.chromium.org/1152203003/diff/100001/build/check_sdk_extras...
build/check_sdk_extras_version.py:37: 'Version {required_version} is required
for the package '
Maybe clarify that this is just a minimum version.

https://codereview.chromium.org/1152203003/diff/100001/build/check_sdk_extras...
build/check_sdk_extras_version.py:61: required_version =
GetRequiredVersion(args.package_id)
I'd call this minimum_version to make it clear that it doesn't have to match
exactly.

[dgn, 2015-06-18 09:59:05]

PTAL

https://codereview.chromium.org/1152203003/diff/100001/build/check_sdk_extras...
File build/check_sdk_extras_version.py (right):

https://codereview.chromium.org/1152203003/diff/100001/build/check_sdk_extras...
build/check_sdk_extras_version.py:9: that it is at an appropriate version.
On 2015/06/17 at 21:18:45, cjhopman wrote:
> Clarify that we just check a minimum version.

Done.

https://codereview.chromium.org/1152203003/diff/100001/build/check_sdk_extras...
build/check_sdk_extras_version.py:37: 'Version {required_version} is required
for the package '
On 2015/06/17 at 21:18:45, cjhopman wrote:
> Maybe clarify that this is just a minimum version.

Done.

https://codereview.chromium.org/1152203003/diff/100001/build/check_sdk_extras...
build/check_sdk_extras_version.py:61: required_version =
GetRequiredVersion(args.package_id)
On 2015/06/17 at 21:18:45, cjhopman wrote:
> I'd call this minimum_version to make it clear that it doesn't have to match
exactly.

Done.

[aberent, 2015-06-18 15:36:04]

LGTM with one nit.

https://codereview.chromium.org/1152203003/diff/120001/build/secondary/third_...
File build/secondary/third_party/android_tools/BUILD.gn (right):

https://codereview.chromium.org/1152203003/diff/120001/build/secondary/third_...
build/secondary/third_party/android_tools/BUILD.gn:108:
"$android_sdk_root/extras/google/google_play_services/libproject/google-play-services_lib/libs/google-play-services.jar",
Strictly, I think, this should depend upon the whole of
"$android_sdk_root/extras/google/google_play_services", if GYP allows directory
dependencies, or on 
"$android_sdk_root/extras/google/google_play_services/source.properties". As it
is at the moment it builds in the current structure of google play services.

[dgn, 2015-06-18 17:20:17]

https://codereview.chromium.org/1152203003/diff/120001/build/secondary/third_...
File build/secondary/third_party/android_tools/BUILD.gn (right):

https://codereview.chromium.org/1152203003/diff/120001/build/secondary/third_...
build/secondary/third_party/android_tools/BUILD.gn:108:
"$android_sdk_root/extras/google/google_play_services/libproject/google-play-services_lib/libs/google-play-services.jar",
On 2015/06/18 at 15:36:03, aberent wrote:
> Strictly, I think, this should depend upon the whole of
"$android_sdk_root/extras/google/google_play_services", if GYP allows directory
dependencies, or on 
> "$android_sdk_root/extras/google/google_play_services/source.properties". As
it is at the moment it builds in the current structure of google play services.

Good point, thanks.

That being said, I'm having trouble making it build. I run into this error:

Unknown deps:
['gen/third_party/android_tools/check_sdk_extras_version.build_config']

@cjhopman: You added the build_configs related stuff, right? Do you know what
I'm doing wrong here?

[cjhopman, 2015-06-19 22:58:03]

On 2015/06/18 17:20:17, dgn wrote:
>
https://codereview.chromium.org/1152203003/diff/120001/build/secondary/third_...
> File build/secondary/third_party/android_tools/BUILD.gn (right):
> 
>
https://codereview.chromium.org/1152203003/diff/120001/build/secondary/third_...
> build/secondary/third_party/android_tools/BUILD.gn:108:
>
"$android_sdk_root/extras/google/google_play_services/libproject/google-play-services_lib/libs/google-play-services.jar",
> On 2015/06/18 at 15:36:03, aberent wrote:
> > Strictly, I think, this should depend upon the whole of
> "$android_sdk_root/extras/google/google_play_services", if GYP allows
directory
> dependencies, or on 
> > "$android_sdk_root/extras/google/google_play_services/source.properties". As
> it is at the moment it builds in the current structure of google play
services.
> 
> Good point, thanks.
> 
> That being said, I'm having trouble making it build. I run into this error:
> 
> Unknown deps:
> ['gen/third_party/android_tools/check_sdk_extras_version.build_config']
> 
> @cjhopman: You added the build_configs related stuff, right? Do you know what
> I'm doing wrong here?

That error is because you've added a dep that doesn't actually have any effect
on how the java target is built. I don't know if/how we should allow you to do
what you want to do here (I've been considering just removing the unknown deps
thing). For now, I'd add the dep to data_deps instead of deps with a TODO about
changing it (this won't actually ensure that the check is done before compiling,
but in practice it'll mostly work).

[dgn, 2015-06-23 10:58:09]

Thanks, as far as I could see, this worked perfectly. I had to add data_deps
support to the whole chain of targets though. Was that the expected thing to do?
I add it in this CL: https://codereview.chromium.org/1196173004

[cjhopman, 2015-06-23 20:46:06]

lgtm

[dgn, 2015-06-26 09:16:23]

The CQ bit was checked by dgn@chromium.org

[dgn, 2015-06-26 09:16:24]

The patchset sent to the CQ was uploaded after l-g-t-m from aberent@chromium.org
Link to the patchset: https://codereview.chromium.org/1152203003/#ps180001
(title: "Split the data_deps changes to another CL")

[commit-bot: I haz the power, 2015-06-26 09:16:39]

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

[commit-bot: I haz the power, 2015-06-26 09:18:46]

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

[commit-bot: I haz the power, 2015-06-26 09:18:47]

Exceeded global retry quota

[dgn, 2015-06-26 09:58:15]

The CQ bit was checked by dgn@chromium.org

[dgn, 2015-06-26 09:58:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
aberent@chromium.org, cjhopman@chromium.org
Link to the patchset: https://codereview.chromium.org/1152203003/#ps200001
(title: "Rebase")

[commit-bot: I haz the power, 2015-06-26 09:58:35]

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

[commit-bot: I haz the power, 2015-06-26 10:30:15]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2015-06-26 10:31:08]

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