code review 103480044: go/build: Easier cross-compiling with gccgo

go/build: Easier cross-compiling with gccgo

Setting CROSS_COMPILE to xxx-yyy-zzz- causes go build to:

- use gccgo rather than gc toolchain (similar to passing -compiler
  gccgo, except it doesn't whine when the architecture is something
  other than x86 or arm)
- set GOARCH=xxx
- set GOOS=yyy
- enable cgo (and pass actual arch and os to cgo via GOARCH and GOOS)
- prepend xxx-yyy-zzz- default compiler names, e.g. xxx-yyy-zzz-gccgo
  instead of gccgo

You can still override these parameters individually by setting GOARCH,
GOOS, CGO_ENABLED, CC, and so on. The goal is to make cross-compiling
as transparent as possible, requiring only the CROSS_COMPILE
environment variable to be set. If CROSS_COMPILE is empty or unset, go
build works as usual.

[eswierk, 2014-06-17 03:15:30]

Hello golang-codereviews@googlegroups.com,

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

[eswierk, 2014-06-17 04:26:04]

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

Please take another look.

[iant, 2014-06-17 13:37:56]

Thanks, but this gives us yet another environment variable to document and use. 
It's not necessary, since people can set CC and CXX, in a shell script if
appropriate.  I would rather not add this.

[aram, 2014-06-17 13:53:15]

NOT LGTM

Where is the discussion around this?

Why do we need this new variable? This seems wrong, we already have CC and CXX
and all that jazz.

Maybe there indeed is a problem with cross-compiling when using gccgo. If that's
the case we should fix our broken assumptions instead of introducing another
variable, another tweak, and another special-case.

https://codereview.appspot.com/103480044/diff/60001/src/cmd/cgo/main.go
File src/cmd/cgo/main.go (right):

https://codereview.appspot.com/103480044/diff/60001/src/cmd/cgo/main.go#newco...
src/cmd/cgo/main.go:136: "mips":  4,
This is unrelated to the purpose of this CL.

https://codereview.appspot.com/103480044/diff/60001/src/cmd/cgo/main.go#newco...
src/cmd/cgo/main.go:143: "mips":  4,
Ditto.

https://codereview.appspot.com/103480044/diff/60001/src/cmd/go/build.go
File src/cmd/go/build.go (right):

https://codereview.appspot.com/103480044/diff/60001/src/cmd/go/build.go#newco...
src/cmd/go/build.go:2158: cgoenv = append(cgoenv, "GOARCH="+goarch,
"GOOS="+goos)
Why is this needed?

https://codereview.appspot.com/103480044/diff/60001/src/pkg/go/build/build.go
File src/pkg/go/build/build.go (right):

https://codereview.appspot.com/103480044/diff/60001/src/pkg/go/build/build.go...
src/pkg/go/build/build.go:289: gccgoCross := os.Getenv("CROSS_COMPILE")
Why is cross-gcc special from regular gcc?

https://codereview.appspot.com/103480044/diff/60001/src/pkg/go/build/build.go...
src/pkg/go/build/build.go:291: cs := strings.SplitN(gccgoCross, "-", 3)
I don't understand why should guess, but in any case we should respect if the
user sets these variables rather than silently (!) overriding them.

[eswierk, 2014-06-17 16:43:32]

On 2014/06/17 13:53:15, aram wrote:
> NOT LGTM
> 
> Where is the discussion around this?

https://groups.google.com/forum/#!topic/golang-dev/AGGMtWHpXi0

The feedback wasn't deafening :-)

> Why do we need this new variable? This seems wrong, we already have CC and CXX
> and all that jazz.

CC and CXX specify the executable names of the C and C++ compilers. I don't see
a way to specify the Go compiler executable name; it's hardcoded to gccgo. And
the go front-end is hardcoded to accept only x86 and arm architecture unless you
build a special gccgo-only version.

> Maybe there indeed is a problem with cross-compiling when using gccgo. If
that's
> the case we should fix our broken assumptions instead of introducing another
> variable, another tweak, and another special-case.

I agree adding yet another variable isn't ideal, and confess ignorance of the
design decisions that informed the existing mechanisms.

I'd be happy with any solution that lets me

* use a single instance of the go front-end for native gc, cross gc, and gccgo
(rather than a whole separate go front-end built just for gccgo),

* work with a "standard" gcc cross toolchain, where the compiler for target
architecture foo is installed as foo-linux-gnu-gcc (at least for Linux), and

* choose the compiler and target architecture/OS with a single variable or flag
(rather than a handful of environment variables, plus PATH tweaks or wrapper
scripts, and "all that jazz" :-) ).

As for the specific feedback:

> https://codereview.appspot.com/103480044/diff/60001/src/cmd/cgo/main.go
> File src/cmd/cgo/main.go (right):
> 
>
https://codereview.appspot.com/103480044/diff/60001/src/cmd/cgo/main.go#newco...
> src/cmd/cgo/main.go:136: "mips":  4,
> This is unrelated to the purpose of this CL.
> 
>
https://codereview.appspot.com/103480044/diff/60001/src/cmd/cgo/main.go#newco...
> src/cmd/cgo/main.go:143: "mips":  4,
> Ditto.

Indeed, will split this out to a separate change.

> https://codereview.appspot.com/103480044/diff/60001/src/cmd/go/build.go
> File src/cmd/go/build.go (right):
> 
>
https://codereview.appspot.com/103480044/diff/60001/src/cmd/go/build.go#newco...
> src/cmd/go/build.go:2158: cgoenv = append(cgoenv, "GOARCH="+goarch,
> "GOOS="+goos)
> Why is this needed?

cgo needs to be told the target architecture and OS to select the right data
types.

> https://codereview.appspot.com/103480044/diff/60001/src/pkg/go/build/build.go
> File src/pkg/go/build/build.go (right):
> 
>
https://codereview.appspot.com/103480044/diff/60001/src/pkg/go/build/build.go...
> src/pkg/go/build/build.go:289: gccgoCross := os.Getenv("CROSS_COMPILE")
> Why is cross-gcc special from regular gcc?

It's not. You can set CROSS_COMPILE to a gcc toolchain that happens to target
the native architecture, in which case it's something of a misnomer. I chose
this terminology for consistency with other projects (e.g. the Linux kernel)
that use CROSS_COMPILE to choose the toolchain.

>
https://codereview.appspot.com/103480044/diff/60001/src/pkg/go/build/build.go...
> src/pkg/go/build/build.go:291: cs := strings.SplitN(gccgoCross, "-", 3)
> I don't understand why should guess, but in any case we should respect if the
> user sets these variables rather than silently (!) overriding them.

That's exactly what envOr does--if the user sets GOARCH and GOOS explicitly then
those take precedence.

[eswierk, 2014-06-17 16:51:37]

Hello golang-codereviews@googlegroups.com, iant@golang.org, aram@mgk.ro (cc:
golang-codereviews@googlegroups.com),

Please take another look.

[eswierk, 2014-06-17 17:25:28]

On 2014/06/17 13:37:56, iant wrote:
> Thanks, but this gives us yet another environment variable to document and
use. 
> It's not necessary, since people can set CC and CXX, in a shell script if
> appropriate.  I would rather not add this.

Here's an example to make the use case more concrete. With this patch, I can use
a single go front-end to build gopacket/afpacket using either gc for the native
architecture or with gccgo for any target architecture. I set just one
environment variable.


$ go build -x
WORK=/tmp/go-build478241547
mkdir -p $WORK/code.google.com/p/gopacket/_obj/
mkdir -p $WORK/code.google.com/p/
cd /bobo/gopacket
/usr/lib/go/pkg/tool/linux_amd64/6g -o
$WORK/code.google.com/p/gopacket/_obj/_go_.6 -p code.google.com/p/gopacket
-complete -D _/bobo/gopacket -I $WORK ./base.go ./decode.go ./doc.go ./flows.go
./layerclass.go ./layertype.go ./packet.go ./parser.go ./writer.go
/usr/lib/go/pkg/tool/linux_amd64/pack grcP $WORK
$WORK/code.google.com/p/gopacket.a $WORK/code.google.com/p/gopacket/_obj/_go_.6
mkdir -p $WORK/_/bobo/gopacket/afpacket/_obj/
mkdir -p $WORK/_/bobo/gopacket/
cd /bobo/gopacket/afpacket
/usr/lib/go/pkg/tool/linux_amd64/cgo -objdir
$WORK/_/bobo/gopacket/afpacket/_obj/ -- -I $WORK/_/bobo/gopacket/afpacket/_obj/
afpacket.go header.go options.go
/usr/lib/go/pkg/tool/linux_amd64/6c -F -V -w -I
$WORK/_/bobo/gopacket/afpacket/_obj/ -I /usr/lib/go/pkg/linux_amd64 -o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_defun.6 -D GOOS_linux -D GOARCH_amd64
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_defun.c
gcc -I . -g -O2 -fPIC -m64 -pthread -print-libgcc-file-name
gcc -I . -g -O2 -fPIC -m64 -pthread -I $WORK/_/bobo/gopacket/afpacket/_obj/ -o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_main.o -c
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_main.c
gcc -I . -g -O2 -fPIC -m64 -pthread -I $WORK/_/bobo/gopacket/afpacket/_obj/ -o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_export.o -c
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_export.c
gcc -I . -g -O2 -fPIC -m64 -pthread -I $WORK/_/bobo/gopacket/afpacket/_obj/ -o
$WORK/_/bobo/gopacket/afpacket/_obj/afpacket.cgo2.o -c
$WORK/_/bobo/gopacket/afpacket/_obj/afpacket.cgo2.c
gcc -I . -g -O2 -fPIC -m64 -pthread -I $WORK/_/bobo/gopacket/afpacket/_obj/ -o
$WORK/_/bobo/gopacket/afpacket/_obj/header.cgo2.o -c
$WORK/_/bobo/gopacket/afpacket/_obj/header.cgo2.c
gcc -I . -g -O2 -fPIC -m64 -pthread -I $WORK/_/bobo/gopacket/afpacket/_obj/ -o
$WORK/_/bobo/gopacket/afpacket/_obj/options.cgo2.o -c
$WORK/_/bobo/gopacket/afpacket/_obj/options.cgo2.c
gcc -I . -g -O2 -fPIC -m64 -pthread -o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_.o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_main.o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_export.o
$WORK/_/bobo/gopacket/afpacket/_obj/afpacket.cgo2.o
$WORK/_/bobo/gopacket/afpacket/_obj/header.cgo2.o
$WORK/_/bobo/gopacket/afpacket/_obj/options.cgo2.o
/usr/lib/go/pkg/tool/linux_amd64/cgo -objdir
$WORK/_/bobo/gopacket/afpacket/_obj/ -dynimport
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_.o -dynout
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_import.c
/usr/lib/go/pkg/tool/linux_amd64/6c -F -V -w -I
$WORK/_/bobo/gopacket/afpacket/_obj/ -I /usr/lib/go/pkg/linux_amd64 -o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_import.6 -D GOOS_linux -D GOARCH_amd64
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_import.c
gcc -I . -g -O2 -fPIC -m64 -pthread -o
$WORK/_/bobo/gopacket/afpacket/_obj/_all.o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_export.o
$WORK/_/bobo/gopacket/afpacket/_obj/afpacket.cgo2.o
$WORK/_/bobo/gopacket/afpacket/_obj/header.cgo2.o
$WORK/_/bobo/gopacket/afpacket/_obj/options.cgo2.o -Wl,-r -nostdlib
/usr/lib/gcc/x86_64-linux-gnu/4.9/libgcc.a
/usr/lib/go/pkg/tool/linux_amd64/6g -o
$WORK/_/bobo/gopacket/afpacket/_obj/_go_.6 -p _/bobo/gopacket/afpacket -D
_/bobo/gopacket/afpacket -I $WORK -I /foo/go/pkg/linux_amd64
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_gotypes.go
$WORK/_/bobo/gopacket/afpacket/_obj/afpacket.cgo1.go
$WORK/_/bobo/gopacket/afpacket/_obj/header.cgo1.go
$WORK/_/bobo/gopacket/afpacket/_obj/options.cgo1.go
/usr/lib/go/pkg/tool/linux_amd64/pack grcP $WORK
$WORK/_/bobo/gopacket/afpacket.a $WORK/_/bobo/gopacket/afpacket/_obj/_go_.6
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_import.6
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_defun.6
$WORK/_/bobo/gopacket/afpacket/_obj/_all.o


$ CROSS_COMPILE=mips-linux-gnu- go build -x
WORK=/tmp/go-build742338952
mkdir -p $WORK/code.google.com/p/gopacket/_obj/
mkdir -p $WORK/code.google.com/p/
cd /bobo/gopacket
mips-linux-gnu-gccgo -I $WORK -c -g -fgo-pkgpath=code.google.com/p/gopacket
-fgo-relative-import-path=_/bobo/gopacket -o
$WORK/code.google.com/p/gopacket/_obj/gopacket.o ./base.go ./decode.go ./doc.go
./flows.go ./layerclass.go ./layertype.go ./packet.go ./parser.go ./writer.go
ar cru $WORK/code.google.com/p/libgopacket.a
$WORK/code.google.com/p/gopacket/_obj/gopacket.o
mkdir -p $WORK/_/bobo/gopacket/afpacket/_obj/
mkdir -p $WORK/_/bobo/gopacket/
cd /bobo/gopacket/afpacket
/usr/lib/go/pkg/tool/linux_amd64/cgo -objdir
$WORK/_/bobo/gopacket/afpacket/_obj/ -gccgo
-gccgopkgpath=_/bobo/gopacket/afpacket -- -I
$WORK/_/bobo/gopacket/afpacket/_obj/ afpacket.go header.go options.go
mips-linux-gnu-gcc -Wall -g -I $WORK/_/bobo/gopacket/afpacket/_obj/ -I
/usr/lib/go/pkg/linux_mips -o $WORK/_/bobo/gopacket/afpacket/_obj/_cgo_defun.o
-D GOOS_linux -D GOARCH_mips -D "GOPKGPATH=\"__bobo_gopacket_afpacket\"" -c
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_defun.c
mips-linux-gnu-gcc -I . -g -O2 -fPIC -pthread -print-libgcc-file-name
mips-linux-gnu-gcc -I . -g -O2 -fPIC -pthread -I
$WORK/_/bobo/gopacket/afpacket/_obj/ -o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_main.o -c
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_main.c
mips-linux-gnu-gcc -I . -g -O2 -fPIC -pthread -I
$WORK/_/bobo/gopacket/afpacket/_obj/ -o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_export.o -c
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_export.c
mips-linux-gnu-gcc -I . -g -O2 -fPIC -pthread -I
$WORK/_/bobo/gopacket/afpacket/_obj/ -o
$WORK/_/bobo/gopacket/afpacket/_obj/afpacket.cgo2.o -c
$WORK/_/bobo/gopacket/afpacket/_obj/afpacket.cgo2.c
mips-linux-gnu-gcc -I . -g -O2 -fPIC -pthread -I
$WORK/_/bobo/gopacket/afpacket/_obj/ -o
$WORK/_/bobo/gopacket/afpacket/_obj/header.cgo2.o -c
$WORK/_/bobo/gopacket/afpacket/_obj/header.cgo2.c
mips-linux-gnu-gcc -I . -g -O2 -fPIC -pthread -I
$WORK/_/bobo/gopacket/afpacket/_obj/ -o
$WORK/_/bobo/gopacket/afpacket/_obj/options.cgo2.o -c
$WORK/_/bobo/gopacket/afpacket/_obj/options.cgo2.c
mips-linux-gnu-gcc -I . -g -O2 -fPIC -pthread -o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_.o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_main.o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_export.o
$WORK/_/bobo/gopacket/afpacket/_obj/afpacket.cgo2.o
$WORK/_/bobo/gopacket/afpacket/_obj/header.cgo2.o
$WORK/_/bobo/gopacket/afpacket/_obj/options.cgo2.o
mips-linux-gnu-gccgo -I $WORK -I /foo/go/pkg/gccgo_linux_mips -c -g
-fgo-pkgpath=_/bobo/gopacket/afpacket
-fgo-relative-import-path=_/bobo/gopacket/afpacket -o
$WORK/_/bobo/gopacket/afpacket/_obj/afpacket.o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_gotypes.go
$WORK/_/bobo/gopacket/afpacket/_obj/afpacket.cgo1.go
$WORK/_/bobo/gopacket/afpacket/_obj/header.cgo1.go
$WORK/_/bobo/gopacket/afpacket/_obj/options.cgo1.go
ar cru $WORK/_/bobo/gopacket/libafpacket.a
$WORK/_/bobo/gopacket/afpacket/_obj/afpacket.o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_defun.o
$WORK/_/bobo/gopacket/afpacket/_obj/_cgo_export.o
$WORK/_/bobo/gopacket/afpacket/_obj/afpacket.cgo2.o
$WORK/_/bobo/gopacket/afpacket/_obj/header.cgo2.o
$WORK/_/bobo/gopacket/afpacket/_obj/options.cgo2.o

[iant, 2014-06-17 17:44:02]

On Tue, Jun 17, 2014 at 10:25 AM,  <eswierk@gmail.com> wrote:
> On 2014/06/17 13:37:56, iant wrote:
>>
>> Thanks, but this gives us yet another environment variable to document
>
> and use.
>>
>> It's not necessary, since people can set CC and CXX, in a shell script
>
> if
>>
>> appropriate.  I would rather not add this.
>
>
> Here's an example to make the use case more concrete. With this patch, I
> can use a single go front-end to build gopacket/afpacket using either gc
> for the native architecture or with gccgo for any target architecture. I
> set just one environment variable.

I think I understand the use case.  I think that setting just one
environment variable is a non-goal.  I agree that we need to make this
work.  Perhaps adding a new environment variable is the way to go, but
I am not yet convinced.

For example, people install cross-compilers using various schemes;
your environment variable works for just one.  It is admittedly the
default one for GCC, but even for GCC there are many different ways to
install.

This should go back to golang-dev for discussion, I think.

Ian

[gobot, 2014-07-16 11:00:18]

R=close (assigned by dave@cheney.net)