Add support for building Go for Android.

Add support for building Go for Android.

With GOOS=android and GOARCH=arm set, this will let you build
binaries that run on devices.

BUG=632895
Committed: https://chromium.googlesource.com/infra/infra/+/8efe78cbfaa8f69bd0518dd6dd2048379c75bf8e

[bpastene, 2016-08-08 20:45:16]

bpastene@chromium.org changed reviewers:
+ iannucci@chromium.org, stip@chromium.org, vadim@chromium.org

[bpastene, 2016-08-08 20:45:16]

ptal!

https://www.youtube.com/watch?v=ZSCSB2bzrU8

[iannucci, 2016-08-09 00:53:40]

On 2016/08/08 at 20:45:16, bpastene wrote:
> ptal!
> 
> https://www.youtube.com/watch?v=ZSCSB2bzrU8

Talked offline and we're going to do the following instead:
  * ping `golang.org/x/mobile` in infra.git as a regular go dependency
  * add `mobile_env.py` in the go folder which:
    * runs env.py
    * runs `gomobile init` to get linker+friends (may require some logic to
cache the result of this) 
    * sets up mobile-specific PATH/envvars

This means that:
  * infra.git remains non-modal (no 'android' gclient variable needed)
  * infra.git's DEPS'd dependencies remain small (doesn't add a 3GB ndk to DEPS)
  * separate env file keeps things explicitly clear, and doesn't add mobile deps
to non-mobile work.
  * go mobile apparently supports iOS too, which would be neat if we actually
want to contemplate some mobile infra apps :).

[Vadim Sh., 2016-08-09 01:04:24]

Description was changed from

==========
Add support for building Go for Android.

With GOOS=android and GOARCH=arm set, this will let you build
binaries that run on devices.

BUG=632895
==========

to

==========
Add support for building Go for Android.

With GOOS=android and GOARCH=arm set, this will let you build
binaries that run on devices.

BUG=632895
==========

[Vadim Sh., 2016-08-09 01:04:24]

vadimsh@chromium.org changed reviewers:
+ vadimsh@chromium.org
- vadim@chromium.org

[Vadim Sh., 2016-08-09 01:06:22]

On 2016/08/09 00:53:40, iannucci wrote:
> On 2016/08/08 at 20:45:16, bpastene wrote:
> > ptal!
> > 
> > https://www.youtube.com/watch?v=ZSCSB2bzrU8
> 
> Talked offline and we're going to do the following instead:
>   * ping `golang.org/x/mobile` in infra.git as a regular go dependency
>   * add `mobile_env.py` in the go folder which:
>     * runs env.py
>     * runs `gomobile init` to get linker+friends (may require some logic to
> cache the result of this) 
>     * sets up mobile-specific PATH/envvars
> 
> This means that:
>   * infra.git remains non-modal (no 'android' gclient variable needed)
>   * infra.git's DEPS'd dependencies remain small (doesn't add a 3GB ndk to
DEPS)
>   * separate env file keeps things explicitly clear, and doesn't add mobile
deps
> to non-mobile work.
>   * go mobile apparently supports iOS too, which would be neat if we actually
> want to contemplate some mobile infra apps :).

3GB NDK is going to make our CI sloooooow in any case, because currently
remote_run nukes everything before each cycle (I don't know why we do this to
ourselves).

[iannucci, 2016-08-09 01:18:03]

On 2016/08/09 at 01:06:22, vadimsh wrote:
> On 2016/08/09 00:53:40, iannucci wrote:
> > On 2016/08/08 at 20:45:16, bpastene wrote:
> > > ptal!
> > > 
> > > https://www.youtube.com/watch?v=ZSCSB2bzrU8
> > 
> > Talked offline and we're going to do the following instead:
> >   * ping `golang.org/x/mobile` in infra.git as a regular go dependency
> >   * add `mobile_env.py` in the go folder which:
> >     * runs env.py
> >     * runs `gomobile init` to get linker+friends (may require some logic to
> > cache the result of this) 
> >     * sets up mobile-specific PATH/envvars
> > 
> > This means that:
> >   * infra.git remains non-modal (no 'android' gclient variable needed)
> >   * infra.git's DEPS'd dependencies remain small (doesn't add a 3GB ndk to
DEPS)
> >   * separate env file keeps things explicitly clear, and doesn't add mobile
deps
> > to non-mobile work.
> >   * go mobile apparently supports iOS too, which would be neat if we
actually
> > want to contemplate some mobile infra apps :).
> 
> 3GB NDK is going to make our CI sloooooow in any case, because currently
remote_run nukes everything before each cycle (I don't know why we do this to
ourselves).

(we do it because "production ready" is not a prerequisite for launching things
into production, apparently)

[iannucci, 2016-08-09 01:18:31]

On 2016/08/09 at 01:18:03, iannucci wrote:
> On 2016/08/09 at 01:06:22, vadimsh wrote:
> > On 2016/08/09 00:53:40, iannucci wrote:
> > > On 2016/08/08 at 20:45:16, bpastene wrote:
> > > > ptal!
> > > > 
> > > > https://www.youtube.com/watch?v=ZSCSB2bzrU8
> > > 
> > > Talked offline and we're going to do the following instead:
> > >   * ping `golang.org/x/mobile` in infra.git as a regular go dependency
> > >   * add `mobile_env.py` in the go folder which:
> > >     * runs env.py
> > >     * runs `gomobile init` to get linker+friends (may require some logic
to
> > > cache the result of this) 
> > >     * sets up mobile-specific PATH/envvars
> > > 
> > > This means that:
> > >   * infra.git remains non-modal (no 'android' gclient variable needed)
> > >   * infra.git's DEPS'd dependencies remain small (doesn't add a 3GB ndk to
DEPS)
> > >   * separate env file keeps things explicitly clear, and doesn't add
mobile deps
> > > to non-mobile work.
> > >   * go mobile apparently supports iOS too, which would be neat if we
actually
> > > want to contemplate some mobile infra apps :).
> > 
> > 3GB NDK is going to make our CI sloooooow in any case, because currently
remote_run nukes everything before each cycle (I don't know why we do this to
ourselves).
> 
> (we do it because "production ready" is not a prerequisite for launching
things into production, apparently)

but practically speaking, the `gomobile init` looks like it might download <<
sizeof(ndk)

[bpastene, 2016-08-09 23:10:44]

ok, pt(another)l !

Added mobile_env.py and a glide deps for gomobile.

On 2016/08/09 01:18:31, iannucci wrote:
> but practically speaking, the `gomobile init` looks like it might download <<
> sizeof(ndk)

Yeah, it fetches ~600mb in ~30s, increasing go/.vendor from .5 gB to 1.1 gB, and
only when building for android. (mac,linux,win is unaffected)

[iannucci, 2016-08-10 01:40:36]

lgtm with some nits. It would be really nice to split out the bulk of the lock
file update from this CL though.

https://codereview.chromium.org/2226873002/diff/20001/go/deps.lock
File go/deps.lock (left):

https://codereview.chromium.org/2226873002/diff/20001/go/deps.lock#oldcode1
go/deps.lock:1: hash:
0967214f967693be0c31d71a9417e7b80a5af7967135a1f004526f9e2be30e80
can we do this lock update in a different CL?

https://codereview.chromium.org/2226873002/diff/20001/go/mobile_env.py
File go/mobile_env.py (right):

https://codereview.chromium.org/2226873002/diff/20001/go/mobile_env.py#newcode20
go/mobile_env.py:20: 'android-ndk-r12b')
is this path variable? Is there a way we could extract it from the gomobile tool
and then record it to a file inside of .vendor?

https://codereview.chromium.org/2226873002/diff/20001/go/mobile_env.py#newcode37
go/mobile_env.py:37: arch = env.get('GOARCH')
does env.py set this? If not, this would be a very atypical thing to have in an
environment.

https://codereview.chromium.org/2226873002/diff/20001/go/mobile_env.py#newcode60
go/mobile_env.py:60: if not os.path.exists(ANDROID_NDK_PATH):
This should be in a `main` function with the `if __name__ == '__main__'` dance
at the bottom. I see that env.py doesn't do this for some reason, but it's
kind-of dangerous/annoying. Right now if you import this python file from
another one, this code will run, which is pretty icky.

https://codereview.chromium.org/2226873002/diff/20001/go/mobile_env.py#newcode72
go/mobile_env.py:72: print subprocess.check_output(cmd, env=new).strip()
we should set cwd == our directory here.

[Vadim Sh., 2016-08-10 01:51:01]

https://codereview.chromium.org/2226873002/diff/20001/go/mobile_env.py
File go/mobile_env.py (right):

https://codereview.chromium.org/2226873002/diff/20001/go/mobile_env.py#newcode60
go/mobile_env.py:60: if not os.path.exists(ANDROID_NDK_PATH):
On 2016/08/10 01:40:35, iannucci wrote:
> This should be in a `main` function with the `if __name__ == '__main__'` dance
> at the bottom. I see that env.py doesn't do this for some reason, but it's
> kind-of dangerous/annoying. Right now if you import this python file from
> another one, this code will run, which is pretty icky.

env.py does "assert __name__ == '__main__'" on top :)

[bpastene, 2016-08-10 22:56:06]

https://codereview.chromium.org/2226873002/diff/20001/go/deps.lock
File go/deps.lock (left):

https://codereview.chromium.org/2226873002/diff/20001/go/deps.lock#oldcode1
go/deps.lock:1: hash:
0967214f967693be0c31d71a9417e7b80a5af7967135a1f004526f9e2be30e80
On 2016/08/10 01:40:35, iannucci wrote:
> can we do this lock update in a different CL?

In case it needs reverting? Ok, done:
https://codereview.chromium.org/2233183002/

https://codereview.chromium.org/2226873002/diff/20001/go/mobile_env.py
File go/mobile_env.py (right):

https://codereview.chromium.org/2226873002/diff/20001/go/mobile_env.py#newcode20
go/mobile_env.py:20: 'android-ndk-r12b')
On 2016/08/10 01:40:36, iannucci wrote:
> is this path variable? Is there a way we could extract it from the gomobile
tool
> and then record it to a file inside of .vendor?

It would probably change as frequently as chromium changes its ndk (once a
year).

The command `gomobile version` should return the relevant information, but it's
complaining that "binary is out of date" for strange reasons, so I just went
with hard coding the path there.

https://codereview.chromium.org/2226873002/diff/20001/go/mobile_env.py#newcode37
go/mobile_env.py:37: arch = env.get('GOARCH')
On 2016/08/10 01:40:35, iannucci wrote:
> does env.py set this? If not, this would be a very atypical thing to have in
an
> environment.

I'd say GOARCH is a very typical thing to have in your environment when
cross-compiling. It's explicitly set in the one place in our infra that
cross-compiles go:
https://chromium.googlesource.com/infra/infra/+/master/recipes/recipes/infra_...

https://codereview.chromium.org/2226873002/diff/20001/go/mobile_env.py#newcode60
go/mobile_env.py:60: if not os.path.exists(ANDROID_NDK_PATH):
On 2016/08/10 01:51:01, Vadim Sh. wrote:
> On 2016/08/10 01:40:35, iannucci wrote:
> > This should be in a `main` function with the `if __name__ == '__main__'`
dance
> > at the bottom. I see that env.py doesn't do this for some reason, but it's
> > kind-of dangerous/annoying. Right now if you import this python file from
> > another one, this code will run, which is pretty icky.
> 
> env.py does "assert __name__ == '__main__'" on top :) 

Added it here anyway.

https://codereview.chromium.org/2226873002/diff/20001/go/mobile_env.py#newcode72
go/mobile_env.py:72: print subprocess.check_output(cmd, env=new).strip()
On 2016/08/10 01:40:35, iannucci wrote:
> we should set cwd == our directory here.

Done.

[iannucci, 2016-08-11 01:20:48]

https://codereview.chromium.org/2226873002/diff/40001/go/mobile_env.py
File go/mobile_env.py (right):

https://codereview.chromium.org/2226873002/diff/40001/go/mobile_env.py#newcode42
go/mobile_env.py:42: 'Specified arch not currently supported on android: %s' %
arch)
I'm still not comfortable with this check. The go toolchain will raise this
error if GOARCH is unsupported, and a very typical usecase that works with
env.py right now is to just put GOOS and GOARCH on the same line as the `go`
command (e.g. `GOARCH=x86 GOOS=windows go build ./...`). I don't think that we
should require users to export this environment variable just to be able to use
this script.

I think a better thing to do would be to print something to standard error after
all the env.py work is done that says something like:

```
To build for android, set the following environment variables when building:
  GOARCH=arm
  GOOS=android
```

[bpastene, 2016-08-11 17:27:54]

https://codereview.chromium.org/2226873002/diff/40001/go/mobile_env.py
File go/mobile_env.py (right):

https://codereview.chromium.org/2226873002/diff/40001/go/mobile_env.py#newcode42
go/mobile_env.py:42: 'Specified arch not currently supported on android: %s' %
arch)
On 2016/08/11 01:20:48, iannucci wrote:
> I'm still not comfortable with this check. The go toolchain will raise this
> error if GOARCH is unsupported, and a very typical usecase that works with
> env.py right now is to just put GOOS and GOARCH on the same line as the `go`
> command (e.g. `GOARCH=x86 GOOS=windows go build ./...`). I don't think that we
> should require users to export this environment variable just to be able to
use
> this script.
> 
> I think a better thing to do would be to print something to standard error
after
> all the env.py work is done that says something like:
> 
> ```
> To build for android, set the following environment variables when building:
>   GOARCH=arm
>   GOOS=android
> ```

GOARCH needs to be specified during the call to mobile_env.py. It sets a number
of other environment variables based on that value below. I'm assuming the go
toolchain does all of this magic itself for GOOS=(win|darwin|linux). There's no
magic for GOOS=android right now. This is that magic :)

Also, you can still inline all the env vars you want, you just have to wrap
mobile_env.py around the go command:
GOOS=android GOARCH=arm64 python mobile_env.py go build ....

[iannucci, 2016-08-11 17:41:46]

On 2016/08/11 at 17:27:54, bpastene wrote:
> https://codereview.chromium.org/2226873002/diff/40001/go/mobile_env.py
> File go/mobile_env.py (right):
> 
>
https://codereview.chromium.org/2226873002/diff/40001/go/mobile_env.py#newcode42
> go/mobile_env.py:42: 'Specified arch not currently supported on android: %s' %
arch)
> On 2016/08/11 01:20:48, iannucci wrote:
> > I'm still not comfortable with this check. The go toolchain will raise this
> > error if GOARCH is unsupported, and a very typical usecase that works with
> > env.py right now is to just put GOOS and GOARCH on the same line as the `go`
> > command (e.g. `GOARCH=x86 GOOS=windows go build ./...`). I don't think that
we
> > should require users to export this environment variable just to be able to
use
> > this script.
> > 
> > I think a better thing to do would be to print something to standard error
after
> > all the env.py work is done that says something like:
> > 
> > ```
> > To build for android, set the following environment variables when building:
> >   GOARCH=arm
> >   GOOS=android
> > ```
> 
> GOARCH needs to be specified during the call to mobile_env.py. It sets a
number of other environment variables based on that value below. I'm assuming
the go toolchain does all of this magic itself for GOOS=(win|darwin|linux).
There's no magic for GOOS=android right now. This is that magic :)
> 
> Also, you can still inline all the env vars you want, you just have to wrap
mobile_env.py around the go command:
> GOOS=android GOARCH=arm64 python mobile_env.py go build ....

Hm ok. In that case lgtm

[bpastene, 2016-08-11 19:55:28]

The CQ bit was checked by bpastene@chromium.org

[commit-bot: I haz the power, 2016-08-11 19:55:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 20:03:58]

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

[commit-bot: I haz the power, 2016-08-11 20:03:59]

Try jobs failed on following builders:
  Infra Mac Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/3093fe4f8fbbb310)
  Infra Win Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/3093fe51c3c98310)

[bpastene, 2016-08-11 20:19:53]

The CQ bit was checked by bpastene@chromium.org

[bpastene, 2016-08-11 20:19:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
iannucci@chromium.org
Link to the patchset: https://codereview.chromium.org/2226873002/#ps60001
(title: "Add a testing exception for the watchdog (for now).")

[commit-bot: I haz the power, 2016-08-11 20:19:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-11 20:44:14]

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

[commit-bot: I haz the power, 2016-08-11 20:44:16]

Try jobs failed on following builders:
  Infra Win Tester on luci.infra.try (JOB_FAILED,
https://luci-milo.appspot.com/swarming/task/30942178baa60e10)

[bpastene, 2016-08-12 00:28:38]

The CQ bit was checked by bpastene@chromium.org

[bpastene, 2016-08-12 00:28:39]

The patchset sent to the CQ was uploaded after l-g-t-m from
iannucci@chromium.org
Link to the patchset: https://codereview.chromium.org/2226873002/#ps80001
(title: "Don't build on windows.")

[commit-bot: I haz the power, 2016-08-12 00:28:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-12 00:45:33]

Description was changed from

==========
Add support for building Go for Android.

With GOOS=android and GOARCH=arm set, this will let you build
binaries that run on devices.

BUG=632895
==========

to

==========
Add support for building Go for Android.

With GOOS=android and GOARCH=arm set, this will let you build
binaries that run on devices.

BUG=632895

Committed:
https://chromium.googlesource.com/infra/infra/+/8efe78cbfaa8f69bd0518dd6dd204...
==========

[commit-bot: I haz the power, 2016-08-12 00:45:35]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/infra/infra/+/8efe78cbfaa8f69bd0518dd6dd204...