code review 109390043: misc/nacl: add go_nacl_arm_exec, update README

misc/nacl: add go_nacl_arm_exec, update README

[minux, 2014-07-02 00:31:01]

Hello rsc@golang.org, iant@golang.org (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[dave_cheney.net, 2014-07-02 00:55:05]

LGTM

On Wed, Jul 2, 2014 at 10:31 AM,  <minux@golang.org> wrote:
> Reviewers: rsc, iant,
>
> Message:
> Hello rsc@golang.org, iant@golang.org (cc:
> golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go
>
>
> Description:
> misc/nacl: add go_nacl_arm_exec, update README.
>
> Please review this at https://codereview.appspot.com/109390043/
>
> Affected files (+17, -3 lines):
>   M misc/nacl/README
>   A misc/nacl/go_nacl_arm_exec
>
>
> Index: misc/nacl/README
> ===================================================================
> --- a/misc/nacl/README
> +++ b/misc/nacl/README
> @@ -3,10 +3,11 @@
>
>  This document outlines the basics of building and developing the Go runtime
> and programs in the Native Client (NaCl) environment.
>
> -Go 1.3 supports two architectures
> +Go 1.3 supports three architectures
>
>   * nacl/386 which is standard 386.
>   * nacl/amd64p32 which is a 64 bit architecture, where the address space is
> limited to a 4gb window.
> + * nacl/arm which is 32-bit ARMv7A architecture with 1GB address space.
>
>  For background it is recommended that you read
> http://golang.org/s/go13nacl.
>
> @@ -28,12 +29,14 @@
>         % cd /opt/nacl_sdk
>         % ./naclsdk update
>
> -At this time pepper_33 is the stable version. If naclsdk downloads a later
> version, please adjust accordingly.
> +At this time pepper_33 is the stable version. If naclsdk downloads a later
> version, please adjust accordingly. As of June 2014, only the canary sdk
> provides support for nacl/arm.
>
> -The cmd/go helper scripts expect that the runtime loaders,
> sel_ldr_x86_{32,64} are in your path. I find it easiest to make a symlink
> from the NaCl distribution to my $GOPATH/bin directory.
> +The cmd/go helper scripts expect that the runtime loaders,
> sel_ldr_{x86_{32,64},arm} and nacl_helper_bootstrap_arm are in your path. I
> find it easiest to make a symlink from the NaCl distribution to my
> $GOPATH/bin directory.
>
>         % ln -nfs /opt/nacl_sdk/pepper_33/tools/sel_ldr_x86_32
> $GOPATH/bin/sel_ldr_x86_32
>         % ln -nfs /opt/nacl_sdk/pepper_33/tools/sel_ldr_x86_64
> $GOPATH/bin/sel_ldr_x86_64
> +       % ln -nfs /opt/nacl_sdk/pepper_canary/tools/sel_ldr_arm
> $GOPATH/bin/sel_ldr_arm
> +       % ln -nfs
> /opt/nacl_sdk/pepper_canary/tools/nacl_helper_bootstrap_arm
> $GOPATH/bin/nacl_helper_bootstrap_arm # only required for NaCl/ARM.
>
>  Support scripts
>  ---------------
> @@ -42,6 +45,7 @@
>
>         % ln -nfs $GOROOT/go/misc/nacl/go_nacl_amd64p32_exec
> $GOPATH/bin/go_nacl_amd64p32_exec
>         % ln -nfs $GOROOT/go/misc/nacl/go_nacl_386_exec
> $GOPATH/bin/go_nacl_386_exec
> +       % ln -nfs $GOROOT/go/misc/nacl/go_nacl_arm_exec
> $GOPATH/bin/go_nacl_arm_exec
>
>  Building and testing
>  --------------------
> Index: misc/nacl/go_nacl_arm_exec
> ===================================================================
> new file mode 100644
> --- /dev/null
> +++ b/misc/nacl/go_nacl_arm_exec
> @@ -0,0 +1,10 @@
> +#!/bin/bash
> +
> +eval $(go env)
> +
> +export NACLENV_GOARCH=$GOARCH
> +export NACLENV_GOOS=$GOOS
> +export NACLENV_GOROOT=/go
> +export NACLENV_NACLPWD=$(pwd | sed "s;$GOROOT;/go;")
> +
> +exec nacl_helper_bootstrap_arm ~/bin/sel_ldr_arm
> --reserved_at_zero=0xXXXXXXXXXXXXXXXX -l /dev/null -S -e "$@"
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[rsc, 2014-07-02 03:20:28]

LGTM

[dave_cheney.net, 2014-07-09 09:53:04]

On 2014/07/02 03:20:28, rsc wrote:
> LGTM

I just noticed that the mode on go_nacl_arm_exec was not +x, if possible, could
you fix that.

[minux, 2014-07-09 14:54:17]

On 2014/07/09 09:53:04, dfc wrote:
> I just noticed that the mode on go_nacl_arm_exec was not +x, if possible,
could
> you fix that.
Thanks, fixed.

[rsc, 2014-07-09 17:36:23]

Does the canary sdk work yet?

https://codereview.appspot.com/109390043/diff/50001/misc/nacl/go_nacl_arm_exec
File misc/nacl/go_nacl_arm_exec (right):

https://codereview.appspot.com/109390043/diff/50001/misc/nacl/go_nacl_arm_exe...
misc/nacl/go_nacl_arm_exec:10: exec nacl_helper_bootstrap_arm ~/bin/sel_ldr_arm
--reserved_at_zero=0xXXXXXXXXXXXXXXXX -l /dev/null -S -e "$@"
Please remove ~/bin/ from the sel_ldr_path.
The other scripts do not use a direct path.
If a full path is needed, use $(which sel_ldr_arm).

[minux, 2014-07-09 18:28:20]

On Wed, Jul 9, 2014 at 1:36 PM, <rsc@golang.org> wrote:

> Does the canary sdk work yet?
>
I just checked sel_ldr_arm in pepper_36 (beta) is still buggy, but the one
in pepper_canary is ok.

>
> https://codereview.appspot.com/109390043/diff/50001/misc/
> nacl/go_nacl_arm_exec
> File misc/nacl/go_nacl_arm_exec (right):
>
> https://codereview.appspot.com/109390043/diff/50001/misc/
> nacl/go_nacl_arm_exec#newcode10
> misc/nacl/go_nacl_arm_exec:10: exec nacl_helper_bootstrap_arm
> ~/bin/sel_ldr_arm --reserved_at_zero=0xXXXXXXXXXXXXXXXX -l /dev/null -S
> -e "$@"
> Please remove ~/bin/ from the sel_ldr_path.
> The other scripts do not use a direct path.
> If a full path is needed, use $(which sel_ldr_arm).
>
yes, it needs full path. Done.

[minux, 2014-07-10 19:15:35]

*** Submitted as https://code.google.com/p/go/source/detail?r=3a7398f59c30 ***

misc/nacl: add go_nacl_arm_exec, update README

LGTM=dave, rsc
R=rsc, iant, dave
CC=golang-codereviews
https://codereview.appspot.com/109390043

[gobot, 2014-07-10 21:28:45]

This CL appears to have broken the netbsd-amd64-bsiegert builder.
See http://build.golang.org/log/69e8fbe7febec3372a5bfd24c58ec3805b8fa045

[minux, 2014-07-10 21:29:47]

On Thu, Jul 10, 2014 at 5:28 PM, <gobot@golang.org> wrote:

> This CL appears to have broken the netbsd-amd64-bsiegert builder.
> See http://build.golang.org/log/69e8fbe7febec3372a5bfd24c58ec3805b8fa045
>
> https://codereview.appspot.com/109390043/
>
flaky test, or builder, or both.

# GOMAXPROCS=2 runtime -cpu=1,2,4
--- FAIL: TestFinalizerType-4 (4.13s)
 mfinal_test.go:61: finalizer for type func(*int) didn't run
FAIL