code review 5732062: go/build: add dependency test

go/build: add dependency test

This exercises the Import function but more importantly
gives us a place to write down the policy for dependencies
within the Go tree.  It also forces us to look at the dependencies,
which may lead to adjustments.

Surprises:
 - go/doc imports text/template, for HTMLEscape (could fix)
 - it is impossible to use math/big without fmt (unfixable)
 - it is impossible to use crypto/rand without math/big (unfixable)

[rsc, 2012-03-05 19:01:48]

Hello golang-dev@googlegroups.com,

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

[bradfitz, 2012-03-05 19:43:32]

LGTM

On Mon, Mar 5, 2012 at 11:01 AM, <rsc@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> go/build: add dependency test
>
> This exercises the Import function but more importantly
> gives us a place to write down the policy for dependencies
> within the Go tree.  It also forces us to look at the dependencies,
> which may lead to adjustments.
>
> Surprises:
>  - go/doc imports text/template, for HTMLEscape (could fix)
>  - it is impossible to use math/big without fmt (unfixable)
>  - it is impossible to use crypto/rand without math/big (unfixable)
>
> Please review this at
http://codereview.appspot.com/**5732062/<http://codereview.appspot.com/5732062/>
>
> Affected files:
>  A src/pkg/go/build/deps_test.go
>
>
>

[gri, 2012-03-05 19:49:30]

LGTM

(code, not a judgement of the dep. hierarchy)

http://codereview.appspot.com/5732062/diff/1002/src/pkg/go/build/deps_test.go
File src/pkg/go/build/deps_test.go (right):

http://codereview.appspot.com/5732062/diff/1002/src/pkg/go/build/deps_test.go...
src/pkg/go/build/deps_test.go:328: m[p] = true
this is only needed for non-macros, no?

[rsc, 2012-03-05 20:13:52]

On Mon, Mar 5, 2012 at 14:49,  <gri@golang.org> wrote:
> this is only needed for non-macros, no?

in theory yes, but it also keeps circular macro definitions
from causing infinite recursion.

[rsc, 2012-03-05 22:08:00]

ping r

[r, 2012-03-06 03:22:23]

LGTM

http://codereview.appspot.com/5732062/diff/1002/src/pkg/go/build/deps_test.go
File src/pkg/go/build/deps_test.go (right):

http://codereview.appspot.com/5732062/diff/1002/src/pkg/go/build/deps_test.go...
src/pkg/go/build/deps_test.go:20: // The map contains two kinds of entries:
lower-case keys are
inconsistent formatting, also hard to parse. fix:

entries:

1) Lower-case ...
2) Upper-case ...

http://codereview.appspot.com/5732062/diff/1002/src/pkg/go/build/deps_test.go...
src/pkg/go/build/deps_test.go:25: // DO NOT CHANGE THIS DATA TO FIX BUILDS.
how will we know to update this table if things are deleted? maybe nothing but
discipline

perhaps codereview or some such could print a message whenever a CL changes the
dependencies of a package.

http://codereview.appspot.com/5732062/diff/1002/src/pkg/go/build/deps_test.go...
src/pkg/go/build/deps_test.go:57: "unicode/utf16": {"unicode"},
CL sent; can delete.

http://codereview.appspot.com/5732062/diff/1002/src/pkg/go/build/deps_test.go...
src/pkg/go/build/deps_test.go:58: "unicode/utf8":  {"unicode"},
not any more!

http://codereview.appspot.com/5732062/diff/1002/src/pkg/go/build/deps_test.go...
src/pkg/go/build/deps_test.go:224: // TODO: Trim dependencies for package net
down to L0.
surely syscall will always be a dependency.

[rsc, 2012-03-06 03:54:54]

PTAL

Updated table for unicode/* but more importantly
extended test to try all goos/goarch/cgo combinations,
so that go test behaves consistently on all systems
(go test -short will not; it only tests the local configuration).

[r, 2012-03-06 03:57:28]

LGTM

http://codereview.appspot.com/5732062/diff/11002/src/pkg/go/build/deps_test.go
File src/pkg/go/build/deps_test.go (right):

http://codereview.appspot.com/5732062/diff/11002/src/pkg/go/build/deps_test.g...
src/pkg/go/build/deps_test.go:345: var gooses = []string{"darwin", "freebsd",
"linux", "netbsd", "openbsd", "plan9", "windows"}
surely geese

[bradfitz, 2012-03-06 04:00:10]

LGTM

but maybe spell out net's dependencies rather than just "L2", so we can
watch them shrink bit at a time?

On Mon, Mar 5, 2012 at 7:54 PM, Russ Cox <rsc@golang.org> wrote:

> PTAL
>
> Updated table for unicode/* but more importantly
> extended test to try all goos/goarch/cgo combinations,
> so that go test behaves consistently on all systems
> (go test -short will not; it only tests the local configuration).
>

[bradfitz, 2012-03-06 04:01:31]

http://codereview.appspot.com/5732062/diff/11002/src/pkg/go/build/deps_test.go
File src/pkg/go/build/deps_test.go (right):

http://codereview.appspot.com/5732062/diff/11002/src/pkg/go/build/deps_test.g...
src/pkg/go/build/deps_test.go:23: // 2) Upper-case keys define aliases for
package sets, which can then
unless it's "C" ?

[rsc, 2012-03-06 04:13:04]

*** Submitted as e219970cdf9f ***

go/build: add dependency test

This exercises the Import function but more importantly
gives us a place to write down the policy for dependencies
within the Go tree.  It also forces us to look at the dependencies,
which may lead to adjustments.

Surprises:
 - go/doc imports text/template, for HTMLEscape (could fix)
 - it is impossible to use math/big without fmt (unfixable)
 - it is impossible to use crypto/rand without math/big (unfixable)

R=golang-dev, bradfitz, gri, r
CC=golang-dev
http://codereview.appspot.com/5732062

[rog, 2012-03-06 11:18:52]

On 5 March 2012 19:01,  <rsc@golang.org> wrote:
> Surprises:
>  - it is impossible to use math/big without fmt (unfixable)

while the type dependency is unfixable, i wonder if it
might be worth avoiding the calls to fmt.Fprintf.

then one could use math/big without linking in
all of fmt and reflect.

[rsc, 2012-03-06 14:25:33]

On Tue, Mar 6, 2012 at 06:18, roger peppe <rogpeppe@gmail.com> wrote:
> while the type dependency is unfixable, i wonder if it
> might be worth avoiding the calls to fmt.Fprintf.
>
> then one could use math/big without linking in
> all of fmt and reflect.

I'm not worried about it.