code review 5553059: cmd/go: implement go get + bug fixes

cmd/go: implement go get + bug fixes

Move error information into Package struct, so that
a package can be returned even if a dependency failed
to load or did not exist.  This makes it possible to run
'go fix' or 'go fmt' on packages with broken dependencies
or missing imports.  It also enables go get -fix.
The new go list -e flag lets go list process those package
errors as normal data.

Change p.Doc to be first sentence of package doc, not
entire package doc.  Makes go list -json or
go list -f '{{.ImportPath}} {{.Doc}}' much more reasonable.

The go tool now depends on http, which means also
net and crypto/tls, both of which use cgo.  Trying to
make the build scripts that build the go tool understand
and handle cgo is too much work.  Instead, we build
a stripped down version of the go tool, compiled as go_bootstrap,
that substitutes an error stub for the usual HTTP code.
The buildscript builds go_bootstrap, go_bootstrap builds
the standard packages and commands, including the full
including-HTTP-support go tool, and then go_bootstrap
gets deleted.

Also handle the case where the buildscript needs updating
during all.bash: if it fails but a go command can be found on
the current $PATH, try to regenerate it.  This gracefully
handles situations like adding a new file to a package
used by the go tool.

[rsc, 2012-01-21 06:13:01]

Hello golang-dev@googlegroups.com,

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

[r, 2012-01-21 22:40:50]

i didn't look at vcs or get

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

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/list.go#newcode52
src/cmd/go/list.go:52: Incomplete bool            // this package or its
dependencies have errors
'or' is singular. s/have/has/

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/pkg.go
File src/cmd/go/pkg.go (right):

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/pkg.go#newcode83
src/cmd/go/pkg.go:83: // shorterThan returns true if s is shorter than t.
s/s/sp/

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/pkg.go#newcode91
src/cmd/go/pkg.go:91: // If they are the same length, settle ties by using
string ordering.
s/by //

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/pkg.go#newcode221
src/cmd/go/pkg.go:221: return string(b)
var i int
for i = 0; i < len(text); i++ {
  if text[i] == '.' && (i == len(text)-1) || isSpaceByte(text[i+1]) {
    return text[:i+1]
  }
}
return text

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/pkg.go#newcode384
src/cmd/go/pkg.go:384: // cannot be loaded at all (the directory does not exist,
etc),
s/etc/etc./ or, better, /e.g./ or even better /for example/

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/pkg.go#newcode386
src/cmd/go/pkg.go:386: // package in the results.  However, if errors occur
trying
do you care about two spaces?

http://codereview.appspot.com/5553059/diff/7033/src/make.bash
File src/make.bash (right):

http://codereview.appspot.com/5553059/diff/7033/src/make.bash#newcode98
src/make.bash:98: if !which go >/dev/null 2>&1; then
can you depend on 'which' being there? if it is, can you use which -s?
another solution would be to have the go command take a --bootstrap command to
test its presence this way.

but really why not test -x "$GOBIN/go"?

[adg, 2012-01-23 02:26:42]

LGTM

Looking forward to the tests.

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/get.go
File src/cmd/go/get.go (right):

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/get.go#newcode26
src/cmd/go/get.go:26: The -a, -n, -v, -x, and -p flags have the same meaning as
in 'go build'
Please document -fix.

[rsc, 2012-01-23 17:31:58]

Hello golang-dev@googlegroups.com, r@golang.org, adg@golang.org (cc:
golang-dev@googlegroups.com),

Please take another look.

[rsc, 2012-01-23 17:32:14]

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/get.go
File src/cmd/go/get.go (right):

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/get.go#newcode26
src/cmd/go/get.go:26: The -a, -n, -v, -x, and -p flags have the same meaning as
in 'go build'
On 2012/01/23 02:26:43, adg wrote:
> Please document -fix.

Done.

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

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/list.go#newcode52
src/cmd/go/list.go:52: Incomplete bool            // this package or its
dependencies have errors
On 2012/01/21 22:40:50, r wrote:
> 'or' is singular. s/have/has/

Done.

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/pkg.go
File src/cmd/go/pkg.go (right):

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/pkg.go#newcode83
src/cmd/go/pkg.go:83: // shorterThan returns true if s is shorter than t.
On 2012/01/21 22:40:50, r wrote:
> s/s/sp/

Done.

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/pkg.go#newcode91
src/cmd/go/pkg.go:91: // If they are the same length, settle ties by using
string ordering.
On 2012/01/21 22:40:50, r wrote:
> s/by //

Done.

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/pkg.go#newcode221
src/cmd/go/pkg.go:221: return string(b)
The existing code is handling things like rewriting \n and \t 
to space and cutting runs of spaces into single spaces, just
as godoc would.  I added that info to the comment.

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/pkg.go#newcode384
src/cmd/go/pkg.go:384: // cannot be loaded at all (the directory does not exist,
etc),
On 2012/01/21 22:40:50, r wrote:
> s/etc/etc./ or, better, /e.g./ or even better /for example/

Done.

http://codereview.appspot.com/5553059/diff/7033/src/cmd/go/pkg.go#newcode386
src/cmd/go/pkg.go:386: // package in the results.  However, if errors occur
trying
On 2012/01/21 22:40:50, r wrote:
> do you care about two spaces?

not really, no.  especially now that godoc fixes it.

http://codereview.appspot.com/5553059/diff/7033/src/make.bash
File src/make.bash (right):

http://codereview.appspot.com/5553059/diff/7033/src/make.bash#newcode98
src/make.bash:98: if !which go >/dev/null 2>&1; then
On 2012/01/21 22:40:50, r wrote:
> can you depend on 'which' being there? if it is, can you use which -s?
> another solution would be to have the go command take a --bootstrap command to
> test its presence this way.
> 
> but really why not test -x "$GOBIN/go"?

Done.

[r, 2012-01-23 18:34:29]

LGTM or rather LGEFN (looks good enough for now)

http://codereview.appspot.com/5553059/diff/13001/src/cmd/go/pkg.go
File src/cmd/go/pkg.go (right):

http://codereview.appspot.com/5553059/diff/13001/src/cmd/go/pkg.go#newcode225
src/cmd/go/pkg.go:225: }
this can stay, but for the record:
1) i hate state machines implemented as loops. they're hard to understand and
debug
2) we have a strings library
3) there are no tests for this tricky code
4) there are no tests for any of this code

i realize the urgency for progress, though

[rsc, 2012-01-23 20:16:54]

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

cmd/go: implement go get + bug fixes

Move error information into Package struct, so that
a package can be returned even if a dependency failed
to load or did not exist.  This makes it possible to run
'go fix' or 'go fmt' on packages with broken dependencies
or missing imports.  It also enables go get -fix.
The new go list -e flag lets go list process those package
errors as normal data.

Change p.Doc to be first sentence of package doc, not
entire package doc.  Makes go list -json or
go list -f '{{.ImportPath}} {{.Doc}}' much more reasonable.

The go tool now depends on http, which means also
net and crypto/tls, both of which use cgo.  Trying to
make the build scripts that build the go tool understand
and handle cgo is too much work.  Instead, we build
a stripped down version of the go tool, compiled as go_bootstrap,
that substitutes an error stub for the usual HTTP code.
The buildscript builds go_bootstrap, go_bootstrap builds
the standard packages and commands, including the full
including-HTTP-support go tool, and then go_bootstrap
gets deleted.

Also handle the case where the buildscript needs updating
during all.bash: if it fails but a go command can be found on
the current $PATH, try to regenerate it.  This gracefully
handles situations like adding a new file to a package
used by the go tool.

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

[tux21b, 2012-01-23 20:47:03]

http://codereview.appspot.com/5553059/diff/11002/src/cmd/go/vcs.go
File src/cmd/go/vcs.go (right):

http://codereview.appspot.com/5553059/diff/11002/src/cmd/go/vcs.go#newcode290
src/cmd/go/vcs.go:290: check:  noVCSSuffix,
I think there is a vcs: "git" entry missing. At least I am unable to fetch any
github repos at the moment.