code review 5562045: go: introduce support for "go build" with gccgo.

go: introduce support for "go build" with gccgo.

The use of gccgo is triggered by GC=gccgo in environment. It
still needs the standard distribution to behave properly, but
allows using the test, build, run, install subcommands with
gccgo.

[remyoudompheng, 2012-01-21 17:17:33]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com,
remy@archlinux.org),

I'd like you to review this change to
https://go.googlecode.com/hg/

[remyoudompheng, 2012-01-21 17:23:45]

Hello,

For the moment this looks a bit ugly to me, but I'd like some comments about the
structure of the go tool before trying to do some polishing.

The patch does the following:
- check for an environment variable GC=gccgo (maybe it should be a boolean flag
instead);
- tries to non intrusively call gccgo where applicable, so  I've defined a
toolchain interface where toolchain-dependant methods went.

It does not:
- check appropriately if dependencies are installed (actually it only works if
GOROOT points to an actual Go distribution and GOPATH has no installed packages,
because it makes it thinks the standard library is installed and all GOPATH
dependencies have to be rebuilt).

I've tried it successfully with with of my executables that depends on a simple
cgo module but got bitten by issue 2754.

[rsc, 2012-01-21 17:56:42]

Thanks very much for working on this.
The structure of what you have seems completely reasonable.
I am not sure what the mechanism should be to trigger use of
gccgo, but the GC=gccgo thing is fine for now.

You should be able to determine the correct paths to pass to
the linker (instead of assuming $WORK) by adding the actions
that build those packages to a.deps in the a.link case (grep
for 'if a.link' in (*builder).action).
Then the linker paths for the link action a are just all the a1.target
for each a1 in a.deps.

Russ

[remyoudompheng, 2012-01-21 18:14:45]

Thank you for the link tip, it will be useful for use with the "go test"
usecase. I'll give it a try.

[remyoudompheng, 2012-01-21 19:11:18]

OK, I think I managed to get go build, go test and go install somewhat work. The
only drawback is that $GOPATH/pkg will be littered with import/path/libpackage.a
files because gccgo expects a "lib" prefix.

Probably in the future they should go in a separate path.

Rémy.

[remyoudompheng, 2012-01-21 19:17:03]

Hello golang-dev@googlegroups.com, rsc@golang.org (cc:
golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[rsc, 2012-01-21 19:20:57]

On Sat, Jan 21, 2012 at 14:11,  <remyoudompheng@gmail.com> wrote:
> OK, I think I managed to get go build, go test and go install somewhat
> work. The only drawback is that $GOPATH/pkg will be littered with
> import/path/libpackage.a files because gccgo expects a "lib" prefix.

That surprises me.  It's true that if you say -lfoo linkers want libfoo.a,
but if you pass a full path name instead I would expect it to work.

Ian, is there something we're missing?

Thanks.
Russ

[remyoudompheng, 2012-01-21 19:25:17]

On 2012/01/21 19:20:57, rsc wrote:
> That surprises me.  It's true that if you say -lfoo linkers want libfoo.a,
> but if you pass a full path name instead I would expect it to work.
> 
> Ian, is there something we're missing?

Archives full paths are passed for linking, but gccgo also needs to find the
file at compile time: to find export information it tries
$INCLUDEDIR/import/path/libpackage.a, for folders mentioned with the -I flag (a
bit like gc).

[rsc, 2012-01-21 19:31:30]

On Sat, Jan 21, 2012 at 14:25,  <remyoudompheng@gmail.com> wrote:
> Archives full paths are passed for linking, but gccgo also needs to find
> the file at compile time: to find export information it tries
> $INCLUDEDIR/import/path/libpackage.a, for folders mentioned with the -I
> flag (a bit like gc).

I see.  It seems fine if the pkg tree is slightly different
when you're using gccgo vs gc.  However, maybe it
should be in pkg/gccgo/linux_amd64/... instead of
pkg/linux_amd64/..., to avoid any possible confusion
with gc objects.

Russ

[remyoudompheng, 2012-01-21 19:50:29]

On 2012/01/21 19:31:30, rsc wrote:
> I see.  It seems fine if the pkg tree is slightly different
> when you're using gccgo vs gc.  However, maybe it
> should be in pkg/gccgo/linux_amd64/... instead of
> pkg/linux_amd64/..., to avoid any possible confusion
> with gc objects.
> 
> Russ

I was afraid it would be horrible, but hooking into includeArgs and scanPackage
made it.

Rémy.

[remyoudompheng, 2012-01-21 19:50:45]

Hello golang-dev@googlegroups.com, rsc@golang.org (cc:
golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[iant2, 2012-01-21 22:43:19]

Russ Cox <rsc@golang.org> writes:

> On Sat, Jan 21, 2012 at 14:25,  <remyoudompheng@gmail.com> wrote:
>> Archives full paths are passed for linking, but gccgo also needs to find
>> the file at compile time: to find export information it tries
>> $INCLUDEDIR/import/path/libpackage.a, for folders mentioned with the -I
>> flag (a bit like gc).
>
> I see.  It seems fine if the pkg tree is slightly different
> when you're using gccgo vs gc.  However, maybe it
> should be in pkg/gccgo/linux_amd64/... instead of
> pkg/linux_amd64/..., to avoid any possible confusion
> with gc objects.

That is probably wise, although it points out another reason that gccgo
looks for libfoo.a: it permits the gc and gccgo trees to overlap, since
gc will create and look for foo.a.

Ian

[albert.strasheim, 2012-01-22 05:27:00]

Hello

On 2012/01/21 17:17:33, remyoudompheng wrote:
> I'd like you to review this change to
> https://go.googlecode.com/hg/

Just a general comment: what about -fgo-prefix? We have packages like foo/os and
foo/syscall in our project and I think this code needs to be built with
-fgo-prefix to work.

Regards

Albert

[albert.strasheim, 2012-01-22 05:42:43]

On 2012/01/22 05:27:00, albert.strasheim wrote:
> Hello
> On 2012/01/21 17:17:33, remyoudompheng wrote:
> > I'd like you to review this change to
> > https://go.googlecode.com/hg/
> Just a general comment: what about -fgo-prefix? We have packages like foo/os
and
> foo/syscall in our project and I think this code needs to be built with
> -fgo-prefix to work.

Any maybe more importantly, foo/bar/server and foo/baz/server.

[remyoudompheng, 2012-01-22 11:23:20]

Done for -fgo-prefix. I had horrible headaches trying to get the linking order
right, especially when trying to make "go test" work with e.g. the standard sort
package.

I finally resorted to using -Wl,-( -Wl,-), and using different go-prefixes for
fake packages, but it still looks a bit ugly to me.

[remyoudompheng, 2012-01-22 11:25:13]

Hello golang-dev@googlegroups.com, rsc@golang.org, iant@google.com,
fullung@gmail.com (cc: golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[albert.strasheim, 2012-01-22 12:33:33]

I have one more question about flags. I'll try to test this on a big codebase
today or tomorrow.

http://codereview.appspot.com/5562045/diff/4005/src/cmd/go/build.go
File src/cmd/go/build.go (right):

http://codereview.appspot.com/5562045/diff/4005/src/cmd/go/build.go#newcode240
src/cmd/go/build.go:240: // atexit(func() { os.RemoveAll(b.work) })
Debugging leftover?

http://codereview.appspot.com/5562045/diff/4005/src/cmd/go/build.go#newcode1052
src/cmd/go/build.go:1052: return b.run(p.Dir, p.ImportPath, "gcc", "-Wall",
"-g",
I have questions about flags here. This is perhaps an issue that needs to be
addressed at a higher level in the go tool:

- Does it work to compile without -g? Would users want to turn it off or is
stripping the binaries afterwards good enough?

- How would I specify -On for the compile step?

- Ian still needs to look at issue 1408, but how would I think if you want LTO
you need to pass -flto to the compiler and the linker. And maybe that means the
linker also needs -On.

- I think -fplugin=./dragonegg.so (and other plugins) could be interesting

I think there's a general problem of how the go tool deals with flags to
commands it runs. The 6g toolset nicely dodged this issue by not having too many
flags. :-) -B to 6g is maybe one exception. I think someone asked about this in
another CL or issue a few days ago.

[remyoudompheng, 2012-01-22 12:57:28]

http://codereview.appspot.com/5562045/diff/4005/src/cmd/go/build.go
File src/cmd/go/build.go (right):

http://codereview.appspot.com/5562045/diff/4005/src/cmd/go/build.go#newcode240
src/cmd/go/build.go:240: // atexit(func() { os.RemoveAll(b.work) })
On 2012/01/22 12:33:33, albert.strasheim wrote:
> Debugging leftover?

Yes, sorry.

http://codereview.appspot.com/5562045/diff/4005/src/cmd/go/build.go#newcode1052
src/cmd/go/build.go:1052: return b.run(p.Dir, p.ImportPath, "gcc", "-Wall",
"-g",
On 2012/01/22 12:33:33, albert.strasheim wrote:
> I have questions about flags here. This is perhaps an issue that needs to be
> addressed at a higher level in the go tool:

I'd say only the minimal set of flags should be hardcoded, and other user
defined flags should be specified in GCFLAGS. Maybe this should be adjusted
after a working gccgo support is added.

I added "-g" because I thought the absence of debugging symbols had broken the
reporting of line numbers but it's actually not implemented.

[remyoudompheng, 2012-01-22 13:11:21]

Hello golang-dev@googlegroups.com, rsc@golang.org, iant@google.com,
fullung@gmail.com (cc: golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[rsc, 2012-01-27 22:01:22]

LGTM

[rsc, 2012-01-27 22:05:56]

*** Submitted as http://code.google.com/p/go/source/detail?r=d6a14e6fac0c ***

go: introduce support for "go build" with gccgo.

The use of gccgo is triggered by GC=gccgo in environment. It
still needs the standard distribution to behave properly, but
allows using the test, build, run, install subcommands with
gccgo.

R=rsc, iant, fullung
CC=golang-dev, remy
http://codereview.appspot.com/5562045

Committer: Russ Cox <rsc@golang.org>