Push resources to Android device

Push resources to Android device

The resources directory is required by some benchmarks. Push it to
device's /data/local/tmp. Add "-i /data/local/tmp/resources" to command
line when using these resources.

BUG=skia:

Committed: https://skia.googlesource.com/skia/+/b2e1fa5b76bc6d38b3cb7074470512f0004d58ce

[qiankun, 2014-06-26 07:00:34]

PTAL.

[mtklein, 2014-06-26 13:53:44]

Foresee any problems Eric?  I ran a bot and it seems happy.

[borenet, 2014-06-26 14:00:29]

So the bots already do this; they push resources to
/storage/emulated/legacy/skiabot/skia_resources in the Install step.  I guess
there's no harm in doing it twice, since they won't conflict. We'll just waste
some space on the device.

Buildbot-wise, I definitely want the pushing of resources to the device and
actually running the command to be separate steps.

[mtklein, 2014-06-26 14:02:26]

On 2014/06/26 13:53:44, mtklein wrote:
> Foresee any problems Eric?  I ran a bot and it seems happy.

Hmm.  Looks like we're pushing the resources every time we run a command
(RunTests, GenerateGMs, RenderPdfs, etc.):
http://108.170.220.21:10117/builders/Test-Android-Nexus4-Adreno320-Arm7-Relea...

Qiankun, as it is now adb_push_if_needed doesn't really work great for
directories.  Can you adapt it and its MD5-checking code to work for resources/
too?  It'd be nice if we didn't just keep pushing the same resources again and
again and again.

[mtklein, 2014-06-26 14:04:41]

On 2014/06/26 14:00:29, borenet wrote:
> So the bots already do this; they push resources to
> /storage/emulated/legacy/skiabot/skia_resources in the Install step.  I guess
> there's no harm in doing it twice, since they won't conflict. We'll just waste
> some space on the device.
> 
> Buildbot-wise, I definitely want the pushing of resources to the device and
> actually running the command to be separate steps.

Seems reasonable to me.  Once the push-caching works, we could switch the bots
over to this?  Or we could hide this resource pushing behind a flag to
android_run_skia?

[borenet, 2014-06-26 14:07:23]

On 2014/06/26 14:04:41, mtklein wrote:
> On 2014/06/26 14:00:29, borenet wrote:
> > So the bots already do this; they push resources to
> > /storage/emulated/legacy/skiabot/skia_resources in the Install step.  I
guess
> > there's no harm in doing it twice, since they won't conflict. We'll just
waste
> > some space on the device.
> > 
> > Buildbot-wise, I definitely want the pushing of resources to the device and
> > actually running the command to be separate steps.
> 
> Seems reasonable to me.  Once the push-caching works, we could switch the bots
> over to this?  Or we could hide this resource pushing behind a flag to
> android_run_skia?

If possible I'd prefer no pushing outside of the Install and Pre[Render/Bench]
steps. So my preference would be to hide behind a flag.

[mtklein, 2014-06-26 14:13:07]

On 2014/06/26 14:07:23, borenet wrote:
> On 2014/06/26 14:04:41, mtklein wrote:
> > On 2014/06/26 14:00:29, borenet wrote:
> > > So the bots already do this; they push resources to
> > > /storage/emulated/legacy/skiabot/skia_resources in the Install step.  I
> guess
> > > there's no harm in doing it twice, since they won't conflict. We'll just
> waste
> > > some space on the device.
> > > 
> > > Buildbot-wise, I definitely want the pushing of resources to the device
and
> > > actually running the command to be separate steps.
> > 
> > Seems reasonable to me.  Once the push-caching works, we could switch the
bots
> > over to this?  Or we could hide this resource pushing behind a flag to
> > android_run_skia?
> 
> If possible I'd prefer no pushing outside of the Install and Pre[Render/Bench]
> steps. So my preference would be to hide behind a flag.

SGTM.  Just, FYI, the run steps currently push their own binaries as needed:

// From RunTests
[06:28:21.204985] /data/local/tmp/skia_launcher 289 KB/s (12932 bytes in 0.043s)
[06:28:32.216941] /data/local/tmp/libskia_android.so 3700 KB/s (39140212 bytes
in 10.328s)
[06:28:37.222556] /data/local/tmp/libtests.so 3720 KB/s (16164764 bytes in
4.242s)

// From GenerateGMs
[06:29:09.000673] /data/local/tmp/libgm.so 3772 KB/s (15812228 bytes in 4.093s)

[borenet, 2014-06-26 14:14:46]

On 2014/06/26 14:13:07, mtklein wrote:
> On 2014/06/26 14:07:23, borenet wrote:
> > On 2014/06/26 14:04:41, mtklein wrote:
> > > On 2014/06/26 14:00:29, borenet wrote:
> > > > So the bots already do this; they push resources to
> > > > /storage/emulated/legacy/skiabot/skia_resources in the Install step.  I
> > guess
> > > > there's no harm in doing it twice, since they won't conflict. We'll just
> > waste
> > > > some space on the device.
> > > > 
> > > > Buildbot-wise, I definitely want the pushing of resources to the device
> and
> > > > actually running the command to be separate steps.
> > > 
> > > Seems reasonable to me.  Once the push-caching works, we could switch the
> bots
> > > over to this?  Or we could hide this resource pushing behind a flag to
> > > android_run_skia?
> > 
> > If possible I'd prefer no pushing outside of the Install and
Pre[Render/Bench]
> > steps. So my preference would be to hide behind a flag.
> 
> SGTM.  Just, FYI, the run steps currently push their own binaries as needed:
> 
> // From RunTests
> [06:28:21.204985] /data/local/tmp/skia_launcher 289 KB/s (12932 bytes in
0.043s)
> [06:28:32.216941] /data/local/tmp/libskia_android.so 3700 KB/s (39140212 bytes
> in 10.328s)
> [06:28:37.222556] /data/local/tmp/libtests.so 3720 KB/s (16164764 bytes in
> 4.242s)
> 
> // From GenerateGMs
> [06:29:09.000673] /data/local/tmp/libgm.so 3772 KB/s (15812228 bytes in
4.093s)

Yeah.  I'm still not really comfortable with that, but the bots should do
roughly what a human does, and pushing the binaries themselves is the logical
behavior when run by a human.

[mtklein, 2014-06-26 14:16:06]

> Yeah.  I'm still not really comfortable with that, but the bots should do
> roughly what a human does, and pushing the binaries themselves is the logical
> behavior when run by a human.

How is resources/ different?

[borenet, 2014-06-26 14:38:13]

On 2014/06/26 14:16:06, mtklein wrote:
> > Yeah.  I'm still not really comfortable with that, but the bots should do
> > roughly what a human does, and pushing the binaries themselves is the
logical
> > behavior when run by a human.
> 
> How is resources/ different?

It's not strictly required for running the app.  Organizationally, it makes
things significantly less headachy for me when all of the bots do things the
same way.  On ChromeOS, for example, we scp the executables and the resources to
the device in the Install step.  I like to have things which fail for different
reasons (eg. failure to push to the device vs the tests themselves failing) in
different steps.  Plus, the more magic we put within the individual steps, the
less maintainable the system gets as a whole.

[mtklein, 2014-06-26 15:09:51]

> It's not strictly required for running the app.  Organizationally, it makes
> things significantly less headachy for me when all of the bots do things the
> same way.  On ChromeOS, for example, we scp the executables and the resources
to
> the device in the Install step.  I like to have things which fail for
different
> reasons (eg. failure to push to the device vs the tests themselves failing) in
> different steps.  Plus, the more magic we put within the individual steps, the
> less maintainable the system gets as a whole.

Yeah, we should do everything (resources/, skia_launcher, all main .so's) in
Install then.  resources/ is becoming strictly required for running some apps.

[qiankun, 2014-06-27 10:13:50]

On 2014/06/26 15:09:51, mtklein wrote:
> > It's not strictly required for running the app.  Organizationally, it makes
> > things significantly less headachy for me when all of the bots do things the
> > same way.  On ChromeOS, for example, we scp the executables and the
resources
> to
> > the device in the Install step.  I like to have things which fail for
> different
> > reasons (eg. failure to push to the device vs the tests themselves failing)
in
> > different steps.  Plus, the more magic we put within the individual steps,
the
> > less maintainable the system gets as a whole.
> 
> Yeah, we should do everything (resources/, skia_launcher, all main .so's) in
> Install then.  resources/ is becoming strictly required for running some apps.

So, do we accept the method hiding resources pushing behind a flag? I added md5
checking for directory. It changes the behavior a little. Before,
adb_push_if_needed /foo1/bar1 /foo2/bar2/ will generate /foo2/bar2/bar1/ on the
device. With this patch, all files in /foo1/bar1/ will be directly pushed into
/foo2/bar2/, directory bar1 doesn't exist in /foo2/bar2/. The same functionality
can be implemented by adb_push_if_needed /foo1/bar1/ /foo2/bar2/bar1/.

[borenet, 2014-06-27 12:36:27]

On 2014/06/27 10:13:50, qiankun wrote:
> On 2014/06/26 15:09:51, mtklein wrote:
> > > It's not strictly required for running the app.  Organizationally, it
makes
> > > things significantly less headachy for me when all of the bots do things
the
> > > same way.  On ChromeOS, for example, we scp the executables and the
> resources
> > to
> > > the device in the Install step.  I like to have things which fail for
> > different
> > > reasons (eg. failure to push to the device vs the tests themselves
failing)
> in
> > > different steps.  Plus, the more magic we put within the individual steps,
> the
> > > less maintainable the system gets as a whole.
> > 
> > Yeah, we should do everything (resources/, skia_launcher, all main .so's) in
> > Install then.  resources/ is becoming strictly required for running some
apps.
> 
> So, do we accept the method hiding resources pushing behind a flag? I added
md5
> checking for directory. It changes the behavior a little. Before,
> adb_push_if_needed /foo1/bar1 /foo2/bar2/ will generate /foo2/bar2/bar1/ on
the
> device. With this patch, all files in /foo1/bar1/ will be directly pushed into
> /foo2/bar2/, directory bar1 doesn't exist in /foo2/bar2/. The same
functionality
> can be implemented by adb_push_if_needed /foo1/bar1/ /foo2/bar2/bar1/.

Since I know my use case is not what most humans want, I'd be fine with adding
an opt-out rather than opt-in, so the bots can specify --no_push_resources or
something, but a human can get the desired behavior by default.

[mtklein, 2014-06-27 13:49:38]

Thanks, this LGTM as-is.  I think we humans can deal with opt-in.  The
resources/ directory is important, but doesn't change that often.

[qiankun, 2014-06-30 05:10:59]

The CQ bit was checked by qiankun.miao@intel.com

[commit-bot: I haz the power, 2014-06-30 05:11:18]

CQ is trying da patch. Follow status at
 https://skia-tree-status.appspot.com/cq/qiankun.miao@intel.com/352303003/40001

[commit-bot: I haz the power, 2014-06-30 05:38:59]

Change committed as b2e1fa5b76bc6d38b3cb7074470512f0004d58ce

[tomhudson, 2014-07-08 18:46:10]

This change broke
https://sites.google.com/site/skiadocs/user-documentation/quick-start-guides/....

"./platform_tools/android/bin/android_gdb_exe tests" no longer works because
adb_push_if_needed is failing to push gdbserver correctly.

[tomhudson, 2014-07-08 19:02:05]

On 2014/07/08 18:46:10, tomhudson wrote:
> This change broke
>
https://sites.google.com/site/skiadocs/user-documentation/quick-start-guides/....
> 
> "./platform_tools/android/bin/android_gdb_exe tests" no longer works because
> adb_push_if_needed is failing to push gdbserver correctly.

There was a latent bug in android_gdb_exe, but the failure exposes a flaw in
android_gdb_tests:

adb_push_if_needed $ANDROID_TOOLCHAIN/../gdbserver data/local/tmp

This command returned approximately "data/local/tmp: no such directory"; it can
be fixed (?) by adding a leading /.
The return value with a leading 'd' was being interpreted by
adb_push_if_needed() as if it were a successful listing of a directory.

Filed http://skbug.com/2729.