code review 7095067: cmd/go: fix build -n for cgo enabled packages

cmd/go: fix build -n for cgo enabled packages

[ality, 2013-01-21 02:41:47]

Hello golang-dev@googlegroups.com,

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

[minux1, 2013-01-21 02:50:53]

Aha, buildscripts, definitely helpful when porting Go to another platform.
I wish the cmd/dist command also have this feature.

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

https://codereview.appspot.com/7095067/diff/4001/src/cmd/go/build.go#newcode1714
src/cmd/go/build.go:1714: cgoLibGccFile, err = b.libgcc(p)
I wonder if it's better to do this:
cgoLibgccFile = "$LIBGCC"

and write
LIBGCC=`gcc -print-libgcc-file-name`
in the buildscript?

[ality, 2013-01-21 03:04:48]

minux.ma@gmail.com once said:
> I wish the cmd/dist command also have this feature.

Me too.

> I wonder if it's better to do this:
> cgoLibgccFile = "$LIBGCC"
> 
> and write
> LIBGCC=`gcc -print-libgcc-file-name`
> in the buildscript?

I thought about that but then I'd have to deal
with differences in how each shell does variable
assignment and "backtick" commands. Or at the
very least modify them afterwards. Right now
the buildscripts are so simple they work in a
variety of shells.

  Anthony

[minux1, 2013-01-21 04:15:54]

LGTM. Please wait for rsc.

[rsc, 2013-01-22 18:47:19]

NOT LGTM

The -n flag means "do not run commands". I think that definition trumps
"maybe generate a working script". That may mean it cannot tell you the
exact commands that are necessary in this case. I think we're going to have
to live with that. If you want a true script there is always -x.

Russ

[ality, 2013-01-30 06:29:09]

PTAL

I changed it to use minux's suggestion
instead of toggling the -n flag. Does
that seem reasonable?

  Anthony

[rsc, 2013-01-30 16:00:31]

LGTM

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

https://codereview.appspot.com/7095067/diff/11001/src/cmd/go/build.go#newcode...
src/cmd/go/build.go:1554: s := fmt.Sprintf("LIBGCC = `%s`\n",
buf.Next(buf.Len()-1))
please use $(%s) instead of `%s`.

[ality, 2013-01-30 23:09:58]

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

cmd/go: fix build -n for cgo enabled packages

R=golang-dev, minux.ma, rsc
CC=golang-dev
https://codereview.appspot.com/7095067