+cbook, kemp

Did the gcc or go compilers change recently?

I would suggest yunlian to do a clean sync and try again. I just tested "test" ap-wireless on Gale and Whirlwind, with croswork_on start and stop, didn't see issue.

The cros-go_src_test() function in the cros-go.eclass tries to build and run the Go tests on the host, but when emerge is run like this:

$ CC=armv7a-cros-linux-gnueabi-gcc FEATURES=test emerge-whirlwind ap-wireless

'go test' gets confused since CC is pointing to cross-gcc for arm. I'll try to fix this in the eclass.

seems like when you cross-compile, the code should be built for the target and then executed there.  it's what we do for all other packages that are C/C++.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/4c1fc6a7b9fb3624344f4d72ca3aa9878906cc88

commit 4c1fc6a7b9fb3624344f4d72ca3aa9878906cc88
Author: Rahul Chaudhry <rahulchaudhry@chromium.org>
Date: Sat Nov 19 00:57:51 2016

dev-lang/go: add host wrapper.

'x86_64-pc-linux-gnu-go' is now installed as a wrapper
that sets proper environment variables for host builds.

BUG= chromium:660591 
TEST='sudo emerge dev-lang/go' and inspected 'x86_64-pc-linux-gnu-go'.

Change-Id: I28508190703e5f6772203dc7a31fa4f21b97cbb0
Reviewed-on: https://chromium-review.googlesource.com/412995
Commit-Ready: Rahul Chaudhry <rahulchaudhry@chromium.org>
Tested-by: Rahul Chaudhry <rahulchaudhry@chromium.org>
Reviewed-by: Luis Lozano <llozano@chromium.org>

[modify] https://crrev.com/4c1fc6a7b9fb3624344f4d72ca3aa9878906cc88/dev-lang/go/go-1.7.3.ebuild
[add] https://crrev.com/4c1fc6a7b9fb3624344f4d72ca3aa9878906cc88/dev-lang/go/files/host_wrapper.py
[add] https://crrev.com/4c1fc6a7b9fb3624344f4d72ca3aa9878906cc88/dev-lang/go/go-1.7.3-r1.ebuild

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/4269fb715b24997ff832c5d5b856fd2a03ad5d41

commit 4269fb715b24997ff832c5d5b856fd2a03ad5d41
Author: Rahul Chaudhry <rahulchaudhry@chromium.org>
Date: Sat Nov 19 01:34:51 2016

eclass: use $(tc-getBUILD_GO) in cros-go_src_test().

BUG= chromium:660591 
TEST='CC=armv7a-cros-linux-gnueabi-gcc FEATURES=test emerge-whirlwind ap-wireless' works.

Change-Id: I4931d9ea17f6900a6508fdaa9e3b59836d69cc5b
Reviewed-on: https://chromium-review.googlesource.com/412996
Commit-Ready: Nicolas Boichat <drinkcat@chromium.org>
Tested-by: Rahul Chaudhry <rahulchaudhry@chromium.org>
Tested-by: Yunlian Jiang <yunlian@chromium.org>
Reviewed-by: Caroline Tice <cmtice@chromium.org>
Reviewed-by: Luis Lozano <llozano@chromium.org>

[modify] https://crrev.com/4269fb715b24997ff832c5d5b856fd2a03ad5d41/eclass/toolchain-funcs.eclass
[modify] https://crrev.com/4269fb715b24997ff832c5d5b856fd2a03ad5d41/eclass/cros-go.eclass

my question in comment #5 doesn't seem to have been answered anywhere ... it's why i didn't approve any of the recent go/CC related CLs

Oh.. Sorry to have missed that.

The background is that we considered these choices a few months ago, when support for unit-testing Go packages was first implemented.

The main thinking was that the unit tests in individual packages are meant to test the logic in those packages (and are low value for testing the cross-compilers themselves). It does not really matter where those unit tests are run. It is definitely easier and more convenient (no remote device or qemu setup needed) to simply build and run the unit tests locally. This is easier and makes sense for Go for another reason: there is no (easy way to put) architecture specific logic in Go packages themselves (no #ifdef processing on architecture macros supported by the language).

As for testing the cross-compilers themselves, we do that every time when we upgrade the compilers: by cross-compiling all the compiler and standard library tests for all supported architectures and running them on actual chromeos devices. Not much value is added by cross-compiling these unit tests and running them under qemu. It is more than likely to hit a qemu specific issue than an actual bug in the cross-compiler.

Happy to learn more about why testing is done the way it is for C/C++ packages. If there is a doc/discussion somewhere that goes over the choices and provides some background, I'll be happy to read it. And if there's substantial added value for Go unit tests to be run that way, I'll be happy to implement that too.

there is plenty of ways arch-specific code can go wrong that is independent of the cross-compiler or qemu.  even just building the code can find bugs.  alignment/padding in structs, or unexpected mismatch in named or common types can be pretty common.  mishandling of arch-specific funcs (like syscalls) has come up too.

how much this applies to Go as a higher level language is debatable.  if that is the route currently being taken, it should be documented in the eclass and the dev.chromium.org guide.

keep in mind that just because you have a package being built for a board doesn't mean that the package has been installed in the sdk, so there's no guarantee its dependencies are available.

Both the eclass (https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/master/eclass/cros-go.eclass) and the developer guide (http://www.chromium.org/chromium-os/developer-guide/go-in-chromium-os) mention that "Package tests are always built and run locally on host".

Wrt. the build dependencies, the cros-go_src_test() function builds and runs the Go unit tests on the host, but picks up all Go dependencies from the board SYSROOT (it rebuilds them for host in a temporary directory). i.e. the unit tests are built using the same environment that the cross-compiler used to build the actual package.

Regarding the other concerns, many do not apply to Go, but some do. Alignment/padding related information is hidden and code cannot easily discover it or directly depend on it (without importing package "unsafe"). Size of int types is different between 32-bit and 64-bit architectures, so any bugs triggered by overflowing a 32-bit integer or trying to allocate a slice with more than 2^32 items may remain undetected when the tests are run on a 64-bit architecture.

One solution may be to also build and run the unit tests on the host in 32-bit mode (GOARCH=386). Note that this still won't catch any issues that happen only on arm for example, like code making direct syscalls.

One additional thing that can be done is to cross-compile the unit tests for target, but do not run them. This would take catch any issues that may be simple build related.

Marking this as fixed. No further action item here, as far as I can see.