Add authorize_adb_keys step that will add host key to device.

Add authorize_adb_keys step that will add host key to device.

This will use ADB_VENDOR_KEYS to push the host specific public adb key onto all
unauthorized devices.

BUG=463496
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=294454

[navabi, 2015-03-05 05:09:04]

navabi@google.com changed reviewers:
+ friedman@chromium.org, jbudorick@chromium.org, vadimsh@chromium.org

[navabi, 2015-03-05 05:14:45]

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
File scripts/slave/android/authorize_adb_devices.py (right):

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:46: GetCmdOutput(['cat', f.name],
env)
this was for debugging. remove.

[jbudorick, 2015-03-05 05:15:02]

I can't comment on the file directly, but checking in the adb binary is scary. I
would really prefer a different approach.

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
File scripts/slave/android/authorize_adb_devices.py (right):

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:20: ADB = os.path.join(CURPATH,
os.pardir, os.pardir, 'tools', 'adb')
O_O

What's this script's intended lifetime? We've seen elsewhere (i.e., the test
runner scripts) how rapidly unmaintainable handrolled adb commands can become.
I'd really prefer to do this some other way if this is intended to live for any
length of time.

[Vadim Sh., 2015-03-05 05:20:26]

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
File scripts/slave/android/authorize_adb_devices.py (right):

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:20: ADB = os.path.join(CURPATH,
os.pardir, os.pardir, 'tools', 'adb')
Committing binary into the repository is not a good idea:
1. It will permanently make the repo fatter.
2. The binary in repo may depend on shared libraries not present elsewhere. For
example, binaries for Ubuntu Precise may not work on Trusty (because libc.so
version changed).

Is "adb" available on android buildbot slaves? I think it would be better to
hardcode "/usr/bin/adb" (or whereever it is) instead of trying to distribute it
with scripts.

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/recipe_mod...
File scripts/slave/recipe_modules/chromium_android/api.py (right):

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/recipe_mod...
scripts/slave/recipe_modules/chromium_android/api.py:299:
'authorize_adb_devices')]
authorize_adb_devices.py

[jbudorick, 2015-03-05 05:30:03]

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
File scripts/slave/android/authorize_adb_devices.py (right):

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:20: ADB = os.path.join(CURPATH,
os.pardir, os.pardir, 'tools', 'adb')
On 2015/03/05 05:20:26, Vadim Sh. wrote:
> Committing binary into the repository is not a good idea:
> 1. It will permanently make the repo fatter.
> 2. The binary in repo may depend on shared libraries not present elsewhere.
For
> example, binaries for Ubuntu Precise may not work on Trusty (because libc.so
> version changed).
> 
> Is "adb" available on android buildbot slaves? I think it would be better to
> hardcode "/usr/bin/adb" (or whereever it is) instead of trying to distribute
it
> with scripts.

adb is part of the android SDK, which gets pulled into android checkouts of
chromium. There's an adb recipe_module that uses it (though I don't have any
experience with using that recipe_module).

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:84: PushHostPubkey(device, env,
public_keys_set)
Also, does this even work? If they're unauthorized and don't have the public
keys, does adb let you push?

[navabi, 2015-03-05 05:30:20]

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
File scripts/slave/android/authorize_adb_devices.py (right):

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:20: ADB = os.path.join(CURPATH,
os.pardir, os.pardir, 'tools', 'adb')
I checked and we don't have an adb binary in /usr/bin/adb or /bin/.

There is one in build_internal/scripts/slave/android/adb, but that may not be
available because this is a public bot.

I don't know of an adb binary I can call from the /b/build/ directory. Perhaps
we can get the sys admins to roll out adb to /usr/bin on all android bots.

We intend for this script to stay around. The problem is, this script belongs in
the infra stuff (i.e. /b/build directory) instead of the chromium checkout (as
it is specifically for the bots) and the checkout is where we have all the
utilities for running adb commands.

I'm open to other alternatives, but putting this in the chromium checkout just
because that is where the adb utility libraries are seems wrong to me.

[navabi, 2015-03-05 05:35:34]

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
File scripts/slave/android/authorize_adb_devices.py (right):

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:20: ADB = os.path.join(CURPATH,
os.pardir, os.pardir, 'tools', 'adb')
> adb is part of the android SDK, which gets pulled into android checkouts of
> chromium. There's an adb recipe_module that uses it (though I don't have any
> experience with using that recipe_module).

Yes, we can point to the ADB in the chromium checkout. I can make that change.

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:84: PushHostPubkey(device, env,
public_keys_set)
On 2015/03/05 05:30:03, jbudorick wrote:
> Also, does this even work? If they're unauthorized and don't have the public
> keys, does adb let you push?

The idea is that all the devices will have the chromium-infra key on them. The
sys admins make sure of that after they flash.

Then we can communicate with the devices with ADB_VENDOR_KEYS=<path to
chromium-infra private key on host>.

Instead of requiring we do everything with ADB_VENDOR_KEYS in the environment,
this single step uses ADB_VENDOR_KEYS just to push the host-specific public key
onto the device, so from there on out, the device is authorized for the host,
just like we currently do manually.

[jbudorick, 2015-03-05 05:37:37]

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
File scripts/slave/android/authorize_adb_devices.py (right):

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:20: ADB = os.path.join(CURPATH,
os.pardir, os.pardir, 'tools', 'adb')
On 2015/03/05 05:30:20, navabi wrote:
> I checked and we don't have an adb binary in /usr/bin/adb or /bin/.
> 
> There is one in build_internal/scripts/slave/android/adb, but that may not be
> available because this is a public bot.
> 
> I don't know of an adb binary I can call from the /b/build/ directory. Perhaps
> we can get the sys admins to roll out adb to /usr/bin on all android bots.

Can we not just use the one from the checkout?

> 
> We intend for this script to stay around. The problem is, this script belongs
in
> the infra stuff (i.e. /b/build directory) instead of the chromium checkout (as
> it is specifically for the bots) and the checkout is where we have all the
> utilities for running adb commands.
> 
> I'm open to other alternatives, but putting this in the chromium checkout just
> because that is where the adb utility libraries are seems wrong to me.

Yeah, after reading this, I agree that it doesn't belong in the chromium
checkout. We're looking at extracting the device-handling python stuff from
build/android/ into its own library s.t. telemetry can use it when it gets
pulled out of chromium; perhaps this could use that at that point, too.

[jbudorick, 2015-03-05 05:38:53]

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
File scripts/slave/android/authorize_adb_devices.py (right):

https://codereview.chromium.org/979013002/diff/20001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:20: ADB = os.path.join(CURPATH,
os.pardir, os.pardir, 'tools', 'adb')
On 2015/03/05 05:35:34, navabi wrote:
> > adb is part of the android SDK, which gets pulled into android checkouts of
> > chromium. There's an adb recipe_module that uses it (though I don't have any
> > experience with using that recipe_module).
> 
> Yes, we can point to the ADB in the chromium checkout. I can make that change.
> 

didn't see this before my last response, oops.

[navabi, 2015-03-05 20:07:58]

PTAL.

[jbudorick, 2015-03-05 20:21:31]

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
File scripts/slave/android/authorize_adb_devices.py (right):

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:13: import optparse
I know we use this in several places, but it is deprecated in python 2.7:
https://docs.python.org/2/library/optparse.html

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:13: import optparse
I know we use this in a variety of places, but this module is deprecated in
python 2.7: https://docs.python.org/2/library/optparse.html

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:42: f =
tempfile.NamedTemporaryFile(delete=False)
why delete = false? can't this section just be

with tempfile.NamedTemporaryFile() as f:
  f.write('\n'.join(public_keys_set))
  f.flush()
  GetCmdOutput([adb, '-s', device, 'push', f.name, ADB_KEYS_PATH], env)

?

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:46: os.unlink(f.name)
What's up with these two lines?

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:52: unauthorized_devices =
[line.split('\t')[0] for line in output
nit: could just return the list instead of assigning it to unauthorized_devices

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:59: parser.add_option('--adb',
help='Path to adb binary.', default='adb')
nit: I think something like '--adb-path' would be a bit more clear.

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:81: env = {'ADB_VENDOR_KEYS':
':'.join(private_key_files)} \
I'll take it on faith that this works, then; I have no experience with
ADB_VENDOR_KEYS.

[jbudorick, 2015-03-05 20:22:41]

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
File scripts/slave/android/authorize_adb_devices.py (right):

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:13: import optparse
On 2015/03/05 20:21:30, jbudorick wrote:
> I know we use this in a variety of places, but this module is deprecated in
> python 2.7: https://docs.python.org/2/library/optparse.html

I swear this only showed up once while I was writing comments :/

[navabi, 2015-03-05 23:05:59]

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
File scripts/slave/android/authorize_adb_devices.py (right):

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:13: import optparse
On 2015/03/05 20:21:30, jbudorick wrote:
> I know we use this in a variety of places, but this module is deprecated in
> python 2.7: https://docs.python.org/2/library/optparse.html

Done and done :).

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:42: f =
tempfile.NamedTemporaryFile(delete=False)
On 2015/03/05 20:21:30, jbudorick wrote:
> why delete = false? can't this section just be
> 
> with tempfile.NamedTemporaryFile() as f:
>   f.write('\n'.join(public_keys_set))
>   f.flush()
>   GetCmdOutput([adb, '-s', device, 'push', f.name, ADB_KEYS_PATH], env)
> 
> ?

Done.

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:46: os.unlink(f.name)
On 2015/03/05 20:21:30, jbudorick wrote:
> What's up with these two lines?

Not needed if we use with. Changed to use with.

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:52: unauthorized_devices =
[line.split('\t')[0] for line in output
On 2015/03/05 20:21:30, jbudorick wrote:
> nit: could just return the list instead of assigning it to
unauthorized_devices

Done.

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:59: parser.add_option('--adb',
help='Path to adb binary.', default='adb')
On 2015/03/05 20:21:30, jbudorick wrote:
> nit: I think something like '--adb-path' would be a bit more clear.

Done.

https://codereview.chromium.org/979013002/diff/80001/scripts/slave/android/au...
scripts/slave/android/authorize_adb_devices.py:81: env = {'ADB_VENDOR_KEYS':
':'.join(private_key_files)} \
On 2015/03/05 20:21:30, jbudorick wrote:
> I'll take it on faith that this works, then; I have no experience with
> ADB_VENDOR_KEYS.

Acknowledged.

[navabi, 2015-03-06 01:09:22]

PTAL

[Vadim Sh., 2015-03-06 01:14:33]

https://codereview.chromium.org/979013002/diff/100001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_android/api.py (right):

https://codereview.chromium.org/979013002/diff/100001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_android/api.py:299:
'authorize_adb_devices'),
authorize_adb_devices.py (with '.py'), unless I do not understand something

https://codereview.chromium.org/979013002/diff/100001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_android/api.py:300: '--adb',
self.m.path['checkout'].join('third_party', 'android_tools',
--adb-path

[navabi, 2015-03-06 01:45:14]

https://codereview.chromium.org/979013002/diff/100001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_android/api.py (right):

https://codereview.chromium.org/979013002/diff/100001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_android/api.py:299:
'authorize_adb_devices'),
On 2015/03/06 01:14:32, Vadim Sh. wrote:
> authorize_adb_devices.py (with '.py'), unless I do not understand something

Done.

https://codereview.chromium.org/979013002/diff/100001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_android/api.py:300: '--adb',
self.m.path['checkout'].join('third_party', 'android_tools',
On 2015/03/06 01:14:32, Vadim Sh. wrote:
> --adb-path

Done.

[navabi, 2015-03-06 01:50:21]

PTAL

[Vadim Sh., 2015-03-06 01:54:42]

Recipe part lgtm.

I don't really understand what's going on in authorize_adb_devices.py, so I
prefer John to finish the review of this file.

[navabi, 2015-03-06 02:05:29]

On 2015/03/06 01:54:42, Vadim Sh. wrote:
> Recipe part lgtm.
> 
> I don't really understand what's going on in authorize_adb_devices.py, so I
> prefer John to finish the review of this file.

I'll wait for John. John, I know you had questions about the authorize script.
The script is stand alone and easily tested. If you want to convince yourself,
here is how I tested:

(start with an authorized device attached locally)

1) cp ~/.android/chrome_infrastructure_adbkey.pub and
~/.android/chrome_infrastructure_adbkey from one of the Android bots into your
~/.android dir.
2) Device has your host specific adb key in /data/misc/adb/adb_keys. Check by
cat'ing that file. Then let's get rid of it:
<path-to-adb>/adb push ~/.android/chrome_infrastructure_adbkey.pub
/data/misc/adb/adb_keys (you can get the chromium_infrastructure_adbkey from the

3) You just got rid of the adb key for your host. You can cat the adb_keys file
to make sure. Then you can really make sure:
<path-to-adb>/adb kill-server
<path-to-adb>/adb devices # This will fail because you don't have the host on
there.

NOTE: This is the state all new devices put on the hosts will be on (i.e. they
will have the chrome_infrastructure_adbkey.pub as /data/misc/adb/adb_keys)

4) Now convince yourself if you have ADB_VENDOR_KEYS it works:
<path-to-adb>/adb kill-server # you have the kill the old adb server
ADB_VENDOR_KEYS=~/.android/chrome_infrastructure_adbkey <path-to-adb>/adb
devices # This will show you devices authorized

Now run the script:
python authorize_adb_devices.py --adb-path
/usr/local/google/code/clankium-master/src/third_party/android_tools/sdk/platform-tools/adb

What just happened was that the script used ADB_VENDOR_KEYS to put your host
specific adb key on the device. So now you can do this and the devices should be
authorized:
<path-to-adb>/adb kill-server
<path-to-adb>/adb devices

[jbudorick, 2015-03-06 02:13:02]

authorize_adb_devices lgtm w/ nits

Thanks for the explanation!

https://codereview.chromium.org/979013002/diff/120001/scripts/slave/android/a...
File scripts/slave/android/authorize_adb_devices.py (right):

https://codereview.chromium.org/979013002/diff/120001/scripts/slave/android/a...
scripts/slave/android/authorize_adb_devices.py:62: adb_path = options.adb_path
if options.adb_path else 'adb'
nit: with the default you've set in the add_argument call above, this can just
be

  adb_path = options.adb_path

https://codereview.chromium.org/979013002/diff/120001/scripts/slave/android/a...
scripts/slave/android/authorize_adb_devices.py:78: env = {'ADB_VENDOR_KEYS':
':'.join(private_key_files)} \
nit: use parentheses instead of a \ continuation

[navabi, 2015-03-06 02:56:38]

https://codereview.chromium.org/979013002/diff/120001/scripts/slave/android/a...
File scripts/slave/android/authorize_adb_devices.py (right):

https://codereview.chromium.org/979013002/diff/120001/scripts/slave/android/a...
scripts/slave/android/authorize_adb_devices.py:62: adb_path = options.adb_path
if options.adb_path else 'adb'
On 2015/03/06 02:13:02, jbudorick wrote:
> nit: with the default you've set in the add_argument call above, this can just
> be
> 
>   adb_path = options.adb_path

Done.

[sullivan, 2015-03-06 21:50:58]

The CQ bit was checked by sullivan@chromium.org

[sullivan, 2015-03-06 21:50:59]

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

[commit-bot: I haz the power, 2015-03-06 21:51:49]

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

[commit-bot: I haz the power, 2015-03-06 21:54:09]

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

[commit-bot: I haz the power, 2015-03-06 21:54:11]

Presubmit check for 979013002-140001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Pylint (575 files) (0.03s) failed
/b/infra_internal/commit_queue/../ENV/bin/python: can't open file
'/b/infra_internal/commit_queue/utils/third_party/pylint.py': [Errno 2] No such
file or directory


scripts/slave/unittests/recipe_simulation_test.py (88.55s) failed
....................................................................................................................................................................................................................................................................................................F.........................FF.....................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................FFF.................................................................................................................................................................................................................

======================================================================
FAIL: chromium.full_chromium_perf_Android_Nexus6_Perf
(/b/infra_internal/commit_queue/workdir/tools/build/scripts/slave/recipes/chromium.expected/full_chromium_perf_Android_Nexus6_Perf.json)
----------------------------------------------------------------------
*** expected
--- current
***************
*** 115,122 ****
--- 115,129 ----
            '[SLAVE_BUILD]/src/build/android/adb_logcat_monitor.py',
            '[SLAVE_BUILD]/src/out/logcat'],
    'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},
    'name': 'spawn_logcat_monitor'},
+  {'cmd': ['python',
+           '-u',
+           '[BUILD]/scripts/slave/android/authorize_adb_devices.py',
+           '--adb-path',
+          
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools/adb'],
+   'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},
+   'name': 'authorize_adb_devices'},
   {'cmd':
['[SLAVE_BUILD]/src/build/android/buildbot/bb_device_status_check.py',
            '--json-output',
            '/path/to/tmp/json'],
    'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},

======================================================================
FAIL: chromium.full_chromium_perf_Android_Nexus9_Perf
(/b/infra_internal/commit_queue/workdir/tools/build/scripts/slave/recipes/chromium.expected/full_chromium_perf_Android_Nexus9_Perf.json)
----------------------------------------------------------------------
*** expected
--- current
***************
*** 115,122 ****
--- 115,129 ----
            '[SLAVE_BUILD]/src/build/android/adb_logcat_monitor.py',
            '[SLAVE_BUILD]/src/out/logcat'],
    'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},
    'name': 'spawn_logcat_monitor'},
+  {'cmd': ['python',
+           '-u',
+           '[BUILD]/scripts/slave/android/authorize_adb_devices.py',
+           '--adb-path',
+          
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools/adb'],
+   'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},
+   'name': 'authorize_adb_devices'},
   {'cmd':
['[SLAVE_BUILD]/src/build/android/buildbot/bb_device_status_check.py',
            '--json-output',
            '/path/to/tmp/json'],
    'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},

======================================================================
FAIL: chromium.full_chromium_perf_Android_One_Perf
(/b/infra_internal/commit_queue/workdir/tools/build/scripts/slave/recipes/chromium.expected/full_chromium_perf_Android_One_Perf.json)
----------------------------------------------------------------------
*** expected
--- current
***************
*** 115,122 ****
--- 115,129 ----
            '[SLAVE_BUILD]/src/build/android/adb_logcat_monitor.py',
            '[SLAVE_BUILD]/src/out/logcat'],
    'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},
    'name': 'spawn_logcat_monitor'},
+  {'cmd': ['python',
+           '-u',
+           '[BUILD]/scripts/slave/android/authorize_adb_devices.py',
+           '--adb-path',
+          
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools/adb'],
+   'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},
+   'name': 'authorize_adb_devices'},
   {'cmd':
['[SLAVE_BUILD]/src/build/android/buildbot/bb_device_status_check.py',
            '--json-output',
            '/path/to/tmp/json'],
    'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},

======================================================================
FAIL: android/perf.full_chromium_perf_Android_Nexus9_Perf
(/b/infra_internal/commit_queue/workdir/tools/build/scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_Nexus9_Perf.json)
----------------------------------------------------------------------
*** expected
--- current
***************
*** 118,125 ****
--- 118,132 ----
            '[SLAVE_BUILD]/src/build/android/adb_logcat_monitor.py',
            '[SLAVE_BUILD]/src/out/logcat'],
    'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},
    'name': 'spawn_logcat_monitor'},
+  {'cmd': ['python',
+           '-u',
+           '[BUILD]/scripts/slave/android/authorize_adb_devices.py',
+           '--adb-path',
+          
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools/adb'],
+   'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},
+   'name': 'authorize_adb_devices'},
   {'cmd':
['[SLAVE_BUILD]/src/build/android/buildbot/bb_device_status_check.py',
            '--json-output',
            '/path/to/tmp/json'],
    'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},

======================================================================
FAIL: android/perf.full_chromium_perf_Android_Nexus6_Perf
(/b/infra_internal/commit_queue/workdir/tools/build/scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_Nexus6_Perf.json)
----------------------------------------------------------------------
*** expected
--- current
***************
*** 118,125 ****
--- 118,132 ----
            '[SLAVE_BUILD]/src/build/android/adb_logcat_monitor.py',
            '[SLAVE_BUILD]/src/out/logcat'],
    'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},
    'name': 'spawn_logcat_monitor'},
+  {'cmd': ['python',
+           '-u',
+           '[BUILD]/scripts/slave/android/authorize_adb_devices.py',
+           '--adb-path',
+          
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools/adb'],
+   'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},
+   'name': 'authorize_adb_devices'},
   {'cmd':
['[SLAVE_BUILD]/src/build/android/buildbot/bb_device_status_check.py',
            '--json-output',
            '/path/to/tmp/json'],
    'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},

======================================================================
FAIL: android/perf.full_chromium_perf_Android_One_Perf
(/b/infra_internal/commit_queue/workdir/tools/build/scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_One_Perf.json)
----------------------------------------------------------------------
*** expected
--- current
***************
*** 118,125 ****
--- 118,132 ----
            '[SLAVE_BUILD]/src/build/android/adb_logcat_monitor.py',
            '[SLAVE_BUILD]/src/out/logcat'],
    'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},
    'name': 'spawn_logcat_monitor'},
+  {'cmd': ['python',
+           '-u',
+           '[BUILD]/scripts/slave/android/authorize_adb_devices.py',
+           '--adb-path',
+          
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools/adb'],
+   'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},
+   'name': 'authorize_adb_devices'},
   {'cmd':
['[SLAVE_BUILD]/src/build/android/buildbot/bb_device_status_check.py',
            '--json-output',
            '/path/to/tmp/json'],
    'env': {'PATH':
'[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s'},


----------------------------------------------------------------------
Ran 1225 tests in 82.812s

FAILED (failures=6)


Presubmit checks took 124.8s to calculate.

[navabi, 2015-03-06 22:28:55]

The CQ bit was checked by navabi@google.com

[navabi, 2015-03-06 22:28:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
vadimsh@chromium.org, jbudorick@chromium.org
Link to the patchset: https://codereview.chromium.org/979013002/revert#ps160001
(title: "Add expectations for new bots.")

[commit-bot: I haz the power, 2015-03-06 22:29:29]

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

[commit-bot: I haz the power, 2015-03-06 22:32:00]

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

[commit-bot: I haz the power, 2015-03-06 22:32:01]

Presubmit check for 979013002-160001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Pylint (575 files) (0.04s) failed
/b/infra_internal/commit_queue/../ENV/bin/python: can't open file
'/b/infra_internal/commit_queue/utils/third_party/pylint.py': [Errno 2] No such
file or directory


Presubmit checks took 130.4s to calculate.

[luqui, 2015-03-06 22:34:35]

luqui@chromium.org changed reviewers:
+ luqui@chromium.org

[luqui, 2015-03-06 22:34:37]

https://codereview.chromium.org/979013002/diff/160001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_android/api.py (right):

https://codereview.chromium.org/979013002/diff/160001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_android/api.py:300: args = ['--adb-path',
self.m.path['checkout'].join('third_party',
You should use api.adb.adb_path() here.

[luqui, 2015-03-06 22:36:42]

https://codereview.chromium.org/979013002/diff/160001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_android/api.py (right):

https://codereview.chromium.org/979013002/diff/160001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_android/api.py:310:
self.authorize_adb_devices()
FYI I think there are still some recipes -- maybe in internal -- that don't use
this or common_tests_setup_steps. 

We should add an assert that makes sure they do before they try to mess with
devices, so we can be sure we set up correctly.

[navabi, 2015-03-06 22:45:09]

https://codereview.chromium.org/979013002/diff/160001/scripts/slave/recipe_mo...
File scripts/slave/recipe_modules/chromium_android/api.py (right):

https://codereview.chromium.org/979013002/diff/160001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_android/api.py:300: args = ['--adb-path',
self.m.path['checkout'].join('third_party',
On 2015/03/06 22:34:37, luqui wrote:
> You should use api.adb.adb_path() here.

Acknowledged. Will fix later.

https://codereview.chromium.org/979013002/diff/160001/scripts/slave/recipe_mo...
scripts/slave/recipe_modules/chromium_android/api.py:310:
self.authorize_adb_devices()
On 2015/03/06 22:36:42, luqui wrote:
> FYI I think there are still some recipes -- maybe in internal -- that don't
use
> this or common_tests_setup_steps. 
> 
> We should add an assert that makes sure they do before they try to mess with
> devices, so we can be sure we set up correctly.

Acknowledged. Let's get this landed for the public bots for now. There is a P0
and new bots are coming up unauthorized for that.

[navabi, 2015-03-06 22:49:57]

The CQ bit was checked by navabi@google.com

[navabi, 2015-03-06 22:49:57]

The patchset sent to the CQ was uploaded after l-g-t-m from
vadimsh@chromium.org, jbudorick@chromium.org
Link to the patchset: https://codereview.chromium.org/979013002/revert#ps180001
(title: "sync'd to fix the pylint cq thing.")

[commit-bot: I haz the power, 2015-03-06 22:50:30]

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

[commit-bot: I haz the power, 2015-03-06 22:58:19]

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

[navabi, 2015-03-16 15:49:36]

The CQ bit was checked by navabi@google.com

[commit-bot: I haz the power, 2015-03-16 15:49:55]

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

[commit-bot: I haz the power, 2015-03-16 15:50:13]

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

[commit-bot: I haz the power, 2015-03-16 15:50:14]

Failed to apply patch for
build/scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_Nexus9_Perf.json:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  can't find file to patch at input line 6
  Perhaps you used the wrong -p or --strip option?
  The text leading up to this was:
  --------------------------
  |Index:
scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_Nexus9_Perf.json
  |diff --git
build/scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_Nexus9_Perf.json
build/scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_Nexus9_Perf.json
  |index
5941da3fde69c509cad7203b9f36f2910a39d897..872f27da14ade9b5ed877a9dbbb90e1c98fd6894
100644
  |---
a/build/scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_Nexus9_Perf.json
  |+++
b/build/scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_Nexus9_Perf.json
  --------------------------
  No file to patch.  Skipping patch.
  1 out of 1 hunk ignored

Patch:      
build/scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_Nexus9_Perf.json
Index:
scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_Nexus9_Perf.json
diff --git
build/scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_Nexus9_Perf.json
build/scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_Nexus9_Perf.json
index
5941da3fde69c509cad7203b9f36f2910a39d897..872f27da14ade9b5ed877a9dbbb90e1c98fd6894
100644
---
a/build/scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_Nexus9_Perf.json
+++
b/build/scripts/slave/recipes/android/perf.expected/full_chromium_perf_Android_Nexus9_Perf.json
@@ -162,6 +162,19 @@
   },
   {
     "cmd": [
+      "python",
+      "-u",
+      "[BUILD]/scripts/slave/android/authorize_adb_devices.py",
+      "--adb-path",
+      "[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools/adb"
+    ],
+    "env": {
+      "PATH":
"[SLAVE_BUILD]/src/third_party/android_tools/sdk/platform-tools:[SLAVE_BUILD]/src/build/android:%(PATH)s"
+    },
+    "name": "authorize_adb_devices"
+  },
+  {
+    "cmd": [
       "[SLAVE_BUILD]/src/build/android/buildbot/bb_device_status_check.py",
       "--json-output",
       "/path/to/tmp/json"

[navabi, 2015-03-16 16:26:11]

The CQ bit was checked by navabi@google.com

[navabi, 2015-03-16 16:26:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
vadimsh@chromium.org, jbudorick@chromium.org
Link to the patchset: https://codereview.chromium.org/979013002/#ps200001
(title: "Rebased and added expectations.")

[commit-bot: I haz the power, 2015-03-16 16:26:27]

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

[commit-bot: I haz the power, 2015-03-16 16:31:14]

Committed patchset #11 (id:200001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=294454