code review 5495068: go: build runtime

go: build runtime

[rsc, 2011-12-16 21:40:31]

Hello golang-dev@googlegroups.com,

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

[r, 2011-12-16 21:48:28]

LGTM but adg and iant should take a look

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

http://codereview.appspot.com/5495068/diff/6001/src/cmd/go/build.go#newcode382
src/cmd/go/build.go:382: // to names using GOOS and GOARCH.
sentence is hard to follow. an example would help.
// example: asm_linux_amd64.h becomes asm_LINUX_AMD64.h

[adg1, 2011-12-16 22:55:54]

LGTM

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

http://codereview.appspot.com/5495068/diff/2012/src/cmd/go/build.go#newcode373
src/cmd/go/build.go:373: gcargs = append(gcargs, "-+")
should really document this flag in 6g

http://codereview.appspot.com/5495068/diff/2012/src/cmd/go/build.go#newcode409
src/cmd/go/build.go:409: // The same convention applies for .s files.
so in a cgo package .s files are built by gcc? doesn't make sense to me.
although I've not encountered a package with cgo and .s files before. i don'
think the assemble step should be inside this if block, correct me if I'm wrong.

http://codereview.appspot.com/5495068/diff/2012/src/cmd/go/list.go
File src/cmd/go/list.go (right):

http://codereview.appspot.com/5495068/diff/2012/src/cmd/go/list.go#newcode40
src/cmd/go/list.go:40: HFiles []string // .h source files
this isn't standard gofmt formatting

http://codereview.appspot.com/5495068/diff/2012/src/cmd/go/main.go
File src/cmd/go/main.go (left):

http://codereview.appspot.com/5495068/diff/2012/src/cmd/go/main.go#oldcode256
src/cmd/go/main.go:256: switch path {
nice.

[rsc, 2011-12-16 23:06:40]

> http://codereview.appspot.com/5495068/diff/2012/src/cmd/go/build.go#newcode373
> src/cmd/go/build.go:373: gcargs = append(gcargs, "-+")
> should really document this flag in 6g

it is documented in the source code.
i'd rather not brag about it in public.

> http://codereview.appspot.com/5495068/diff/2012/src/cmd/go/build.go#newcode409
> src/cmd/go/build.go:409: // The same convention applies for .s files.
> so in a cgo package .s files are built by gcc? doesn't make sense to me.
> although I've not encountered a package with cgo and .s files before. i
> don' think the assemble step should be inside this if block, correct me
> if I'm wrong.

if the c convention makes sense, i don't see why the s wouldn't.
it hasn't come up yet but i think it's more consistent this way.

[rsc, 2011-12-20 15:28:08]

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

go: build runtime

R=golang-dev, r, adg
CC=golang-dev
http://codereview.appspot.com/5495068