[Android] More old_interface conversions in build/android/.

[Android] More old_interface conversions in build/android/.

This also creates build/android/pylib/constants/ and moves the existing
constants.py file into it as __init__.py.

BUG=267773
Committed: https://crrev.com/cdb5bb0b94d7d2716a0c826fca576a0efdd33511
Cr-Commit-Position: refs/heads/master@{#326025}

[jbudorick, 2015-04-16 17:55:20]

jbudorick@chromium.org changed reviewers:
+ klundberg@chromium.org, perezju@chromium.org

[perezju, 2015-04-17 09:16:43]

https://codereview.chromium.org/1092703002/diff/20001/build/android/gyp/util/...
File build/android/gyp/util/build_device.py (right):

https://codereview.chromium.org/1092703002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/build_device.py:26: class BuildDevice(object):
not sure if this falls under our scope, but maybe add a TODO for completely
removing this object? Almost seems like clients should just be using DeviceUtils
directly.

https://codereview.chromium.org/1092703002/diff/20001/build/android/pylib/con...
File build/android/pylib/constants/keycode.py (right):

https://codereview.chromium.org/1092703002/diff/20001/build/android/pylib/con...
build/android/pylib/constants/keycode.py:10: BACK = 4
To better mirror the Android SDK, which uses e.g. KeyEvent.KEYCODE_MENU in Java,
we should perhaps name this file "keyevent" and keep the KEYCODE_ prefix on the
constant names.

It seems a bit redundant, but will be better if at some point we also need to
add some of the META_ or other related KeyEvent constants.

Also there are *lots* of constants, we probably don't need to have all of them,
but we might also want to start with a few more than just 4. Not sure what
should be the criteria for inclusion, though.

[jbudorick, 2015-04-17 13:32:23]

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

[jbudorick, 2015-04-17 13:32:24]

https://codereview.chromium.org/1092703002/diff/20001/build/android/gyp/util/...
File build/android/gyp/util/build_device.py (right):

https://codereview.chromium.org/1092703002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/build_device.py:26: class BuildDevice(object):
On 2015/04/17 09:16:43, perezju wrote:
> not sure if this falls under our scope, but maybe add a TODO for completely
> removing this object? Almost seems like clients should just be using
DeviceUtils
> directly.

This is a question for +cjhopman.

https://codereview.chromium.org/1092703002/diff/20001/build/android/pylib/con...
File build/android/pylib/constants/keycode.py (right):

https://codereview.chromium.org/1092703002/diff/20001/build/android/pylib/con...
build/android/pylib/constants/keycode.py:10: BACK = 4
On 2015/04/17 09:16:43, perezju wrote:
> To better mirror the Android SDK, which uses e.g. KeyEvent.KEYCODE_MENU in
Java,
> we should perhaps name this file "keyevent" and keep the KEYCODE_ prefix on
the
> constant names.

sgtm

> 
> It seems a bit redundant, but will be better if at some point we also need to
> add some of the META_ or other related KeyEvent constants.
> 
> Also there are *lots* of constants, we probably don't need to have all of
them,
> but we might also want to start with a few more than just 4. Not sure what
> should be the criteria for inclusion, though.

Currently, the criterion is "are they used?" (BACK and MENU are used
downstream.)

I'm not really keen on adding things we aren't using, and I"m not opposed to
adding things as we start using them.

[cjhopman, 2015-04-17 18:44:37]

https://codereview.chromium.org/1092703002/diff/20001/build/android/gyp/util/...
File build/android/gyp/util/build_device.py (right):

https://codereview.chromium.org/1092703002/diff/20001/build/android/gyp/util/...
build/android/gyp/util/build_device.py:26: class BuildDevice(object):
On 2015/04/17 13:32:24, jbudorick wrote:
> On 2015/04/17 09:16:43, perezju wrote:
> > not sure if this falls under our scope, but maybe add a TODO for completely
> > removing this object? Almost seems like clients should just be using
> DeviceUtils
> > directly.
> 
> This is a question for +cjhopman.

So, it's complicated.

This is used for a bunch of actions in gyp/gn. For builds to be correct, those
actions need to correctly specify all of their inputs. In the case of a python
script like this, that means that they should essentially specify all of the
python files that are imported.

For that reason, we really, really try not to use things outside of
build/android/gyp (because it's impractical to expect people to update a whole
bunch of lists of inputs in gyp when changing imports in build/android/pylib).

In this case, we know that our inputs are incorrect. A change to the bits of
device_utils that we use could definitely lead to our builds flakily
failing/succeeding. Not using device_utils directly sort of makes it easier to
restrict ourselves to more stable bits of that interface.

Now, in GN, we actually don't really have that problem. GN actions support
writing a depfile (basically it allows the action itself to specify its inputs
when it is run). All of our python actions then write the list of imported files
into their depfile.

Recently, gyp has also gotten support for depfiles on actions, but we haven't
taken the time to go back and actually use that.

Once all of our actions use depfiles (either because we don't use gyp anymore or
because the gyp actions are all updated) we can freely use whatever things we
want.

So, yes we should be using DeviceUtils directly, but... it's complicated.

[jbudorick, 2015-04-20 19:44:50]

perezju: ping

[jbudorick, 2015-04-20 19:45:37]

On 2015/04/17 18:44:37, cjhopman wrote:
>
https://codereview.chromium.org/1092703002/diff/20001/build/android/gyp/util/...
> File build/android/gyp/util/build_device.py (right):
> 
>
https://codereview.chromium.org/1092703002/diff/20001/build/android/gyp/util/...
> build/android/gyp/util/build_device.py:26: class BuildDevice(object):
> On 2015/04/17 13:32:24, jbudorick wrote:
> > On 2015/04/17 09:16:43, perezju wrote:
> > > not sure if this falls under our scope, but maybe add a TODO for
completely
> > > removing this object? Almost seems like clients should just be using
> > DeviceUtils
> > > directly.
> > 
> > This is a question for +cjhopman.
> 
> So, it's complicated.
> 
> This is used for a bunch of actions in gyp/gn. For builds to be correct, those
> actions need to correctly specify all of their inputs. In the case of a python
> script like this, that means that they should essentially specify all of the
> python files that are imported.
> 
> For that reason, we really, really try not to use things outside of
> build/android/gyp (because it's impractical to expect people to update a whole
> bunch of lists of inputs in gyp when changing imports in build/android/pylib).
> 
> In this case, we know that our inputs are incorrect. A change to the bits of
> device_utils that we use could definitely lead to our builds flakily
> failing/succeeding. Not using device_utils directly sort of makes it easier to
> restrict ourselves to more stable bits of that interface.
> 
> Now, in GN, we actually don't really have that problem. GN actions support
> writing a depfile (basically it allows the action itself to specify its inputs
> when it is run). All of our python actions then write the list of imported
files
> into their depfile.
> 
> Recently, gyp has also gotten support for depfiles on actions, but we haven't
> taken the time to go back and actually use that.
> 
> Once all of our actions use depfiles (either because we don't use gyp anymore
or
> because the gyp actions are all updated) we can freely use whatever things we
> want.
> 
> So, yes we should be using DeviceUtils directly, but... it's complicated.

Thanks for the explanation, Chris. Leaving b/a/gyp/util/build_device.py as is.

[perezju, 2015-04-21 08:58:03]

lgtm, thanks!

[jbudorick, 2015-04-21 12:57:47]

The CQ bit was checked by jbudorick@chromium.org

[commit-bot: I haz the power, 2015-04-21 12:58:11]

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

[commit-bot: I haz the power, 2015-04-21 14:03:02]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-04-21 14:04:55]

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