This support is not perfect, but should produce more accurate results than
the previous behavior, which is to silently drop those files.
Change-Id: Ia00c042bf303e9ec0fb1cbd579c0fccb29073de0
Reviewed-on: https://go-review.googlesource.com/c/151999
Run-TryBot: Michael Matloob <matloob@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Our tests compare import graphs from go/packages to expected graphs,
and one of the test cases imports math/bits. But in tip math/bits
picked up a dependency on unsafe, which means the expected graph
is different when run against a Go version >= go1.11. Remove that edge
before comparing against the expected graph to work around the breakage.
Change-Id: Ic586a75ba530741d251df9f87d0817a8e37d92ea
Reviewed-on: https://go-review.googlesource.com/c/151657
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
The fallback driver wasn't returning the roots in a
deterministic order because it was using sticking them in
a map and then iterating over that map to get each element.
Put them into a slice instead (and make a few small
associated changes) to preserve behavior.
Fixesgolang/go#28040Fixesgolang/go#28609
Change-Id: Ib8f8c88d65b7a48b2b04ca91e2d3c316d5bb5803
Reviewed-on: https://go-review.googlesource.com/c/148880
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This is based on 147340 and has the same test, but with a different fix.
We now produce a nice error if there is an empty root, instead of
crashing, and the cause of empty roots in go list has been fixed.
The underlying call to go list is returning the same package more than
once, and we only fix the first entry in the root list, so the second
one got left blank.
The fix was to not add the second duplicate copy to the output of the
driver at all.
Change-Id: I9f1b2f0fd63635ba101cdd3c8a5108530e968ba9
Reviewed-on: https://go-review.googlesource.com/c/147440
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
We do insist that drivers are stable (not sorted, just stable) and that
refine is also stable, which allows us to promise a stable output.
I also changed refine so it returns the root set in the same order that
the original root id list was supplied, as this seems to be a strictly
better experience.
Change-Id: I8eb0bffd7547865d14a6c6f18646018b9af140bd
Reviewed-on: https://go-review.googlesource.com/c/145877
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Treat the top-level package of a module specially so that the VCS
working dir name doesn't have to match the package name. Generally we
take the position that directory names should match packages, but for
the top-level package that's a pretty strict requirement. Now we always
check the top-level directory, just in case.
Change-Id: I2e96751bf46736fbaef434fa9788fdc1339c37b9
Reviewed-on: https://go-review.googlesource.com/c/144817
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
The builders are currently flaky failing because of a root order being
wrong.
This causes all root lists to be sorted before being returned, so the
order is always stable no matter which underlying driver is running.
Fixesgolang/go#27594
Change-Id: I09db45c67ad00f23dfaec8e271acbd13fc338888
Reviewed-on: https://go-review.googlesource.com/c/143737
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
This extracts some of the test code from packages, and adds the ability to run
the same test with multiple drivers.
This should generally be useful for all tools that run on top of go/packages as
well when writing tests for them.
Change-Id: I88c596ad07c0782270c5798d92ae29f7549943cf
Reviewed-on: https://go-review.googlesource.com/c/140118
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Can't do module tests on a pre-modules version of Go.
Change-Id: I8e3e22d472f37ebc9f930a68c9a6c0f4b7ba7ea0
Reviewed-on: https://go-review.googlesource.com/c/142700
Run-TryBot: Heschi Kreinick <heschi@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Add an implementation of name= for go list. It will be used to
implement goimports and godoc-like lookups by package name.
Imported a copy of the semver package from the stdlib to do version
comparison, and tweaked the gopathwalk API to include a hint about what
kind of source directory is being traversed.
Note that the tests, despite my best efforts, are not hermetic: go list
insists on doing version lookups in situations where it seems to me like
it shouldn't need to.
I think this implementation is ready for serious use. The one thing I'm
nervous about is that it currently does a substring match when looking
for a package name, so if you look up a package named "a" you will get
a huge number of results. This matches goimports' behavior but I don't
know if it's suitable for general use.
Change-Id: I2b7f823b74571fe30d3bd9c7dfafb4e6a40df5d3
Reviewed-on: https://go-review.googlesource.com/c/138878
Run-TryBot: Heschi Kreinick <heschi@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Get the value of GOARCH from the config instead.
It should have been set to the same as the one from the environment by
default, but may have been overidden by the caller.
Change-Id: If9a6c0ae998c1c72ad2a68fe83c8bb9f5614a189
Reviewed-on: https://go-review.googlesource.com/c/142361
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
We agreed on a different set of invalid queries than what the test
tests. The change got lost in a merge. Fix it.
Change-Id: I812e561d924f5dbd0c29e3a6ec5fb53022d09487
Reviewed-on: https://go-review.googlesource.com/c/142359
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
This change updates the language accepted through the arguments
to packages.Load to make it more consistent. There are now two layers.
A pattern containing an "=" is considered to be a special query, and the
part of the pattern up to the first "=" is considered the query type. All
other patterns are to be interpreted as the build system interprets it.
For now two special queries exist. file= has the behavior that contains:
did: finding packages containing the given file. The query type pattern=
is used to pass through a pattern to be interpreted by the build system.
pattern= is a type of escaping to allow passing through patterns that
contain an "=" to be interpreted by the underlying buildsystem. To allow
for new query types to be introduced, packages.Load will report an
error if the qury type is not understood. We expect name= to be added in
an upcoming change.
"contains:" changes to "file=". A new
Change-Id: I1b208d1c998c67d5556cdc872d7694273cedb7e4
Reviewed-on: https://go-review.googlesource.com/c/141681
Reviewed-by: Alan Donovan <adonovan@google.com>
The typechecker's Sizes function is not currently set correctly.
The correct answer requires information known only to the build
system's query tool (go list, blaze query, etc), and we need to
add a mechanism for it to return this information.
In the meantime, we use this simple workaround: if the GOARCH
environment variable is set, we use that to determine sizes according
to the conventions of gc. Otherwise, we use the architecture for which
the application was compiled. Both could easily be incorrect, but this
is nonetheless progress.
This change should fix the tests of go/analysis/passes/shift,
which are currently broken for GOARCH=386 because the analysistest
driver uses go/packages, which ignores GOARCH.
Change-Id: Iabe3211ad513a9a94eadd6d8f4b2068f7abdd053
Reviewed-on: https://go-review.googlesource.com/c/141757
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This allows users of go/packages to replace the contents of already
existing files, to support use-cases such as unsaved files in editors.
BREAKING CHANGE: This CL changes the signature of the function provided
to Config.ParseFile.
Change-Id: I6ce50336060832679e9f64f8d201b44651772e0b
Reviewed-on: https://go-review.googlesource.com/c/139798
Reviewed-by: Alan Donovan <adonovan@google.com>
This test is unsurprisingly failing on the nocgo builder because
the cgo packages don't exist on those builders.
Updates golang/go#28040
Change-Id: I633b73bb48e76824645e4e8dd141fb42c9adc19f
Reviewed-on: https://go-review.googlesource.com/c/140121
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
TestLoadImportsC just works on Go 1.10. And there's no good reason
that TestLoadAllSyntaxImportErrors shouldn't work, even though
it's currently always skipped.
Change-Id: Icd8d311f12c5731cc635937a00251eab0a3077ec
Reviewed-on: https://go-review.googlesource.com/139117
Reviewed-by: Alan Donovan <adonovan@google.com>
A previous change (golang.org/cl/137096) that made absolute package paths relative to
GOROOT or GOPATH entries also accidentally applied to .go filesnames.
Filter those out of the list of paths considered to make relative to
search path.
(package paths that don't start with './' or '/' are relative to GOROOT or GOPATH,
but filenames are not.)
Change-Id: I67fbd0e5caa7e53f3ab5b77f55d6841fe2132578
Reviewed-on: https://go-review.googlesource.com/c/138880
Reviewed-by: Alan Donovan <adonovan@google.com>
Another change to bring the go1.10 fallback's functionality in line with
the go1.11 go list implementation. The fallback now uses go env to
determine GOPATH and GOROOT and searches them to see if any match an
absolute path, and if so trims the GOPATH/GOROOT entry off the start
of the path.
Fixesgolang/go#27734
Change-Id: Ibd2313fc4301d42fd8c0cd98f1f3e7a313d65eb7
Reviewed-on: https://go-review.googlesource.com/137096
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
The documentation for Load says:
“Load returns an error if any of the patterns was invalid as defined
by the underlying build system. It may return an empty list of
packages without an error, for instance for an empty expansion of a
valid wildcard. Errors associated with a particular package are
recorded in the corresponding Package's Errors list, and do not cause
Load to return an error.”
Therefore, it should not be an error for a pattern to match no
packages. If the pattern is a literal package path that does not
exist, we should prefer to return a *Package for it with an error in
the Errors field.
Change-Id: Iaecfb920097e3b520e763bd52c0e326d2e7a4861
Reviewed-on: https://go-review.googlesource.com/137075
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
For all packages p, q where there's an dependency path of
p.test -> ... -> q -> ... -> p, Go creates test variants
q [p.test] of each q and replaces the dependency on q with
a dependency on q [p.test], and replaces p with the expanded
test variant p[p.test]. Fix the fallback logic to add
these missing test variants. (Before this change, it was
only producing the variant of p: p [p.test].)
Fixesgolang/go#27670
Change-Id: Ic56ba35fadcdf8c5928ec76f5a7b0ebe650c9f02
Reviewed-on: https://go-review.googlesource.com/136176
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Part of the testmain generation logic (sans coverage) has been taken
from go build and put into golist_fallback_testmain.go.
golist_fallback invokes this logic and builds up the package metadata
for the testmain.
The tests checking for testmain are now no longer skipped from
packages_test.go.
Change-Id: I487a947f087f3ad4161ea6c2bed06ebb2f833422
Reviewed-on: https://go-review.googlesource.com/134119
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Errors relating to a particular package (as opposed to the Load
operation as a whole) are now only recorded in the Errors
field of the corresponding Package.
Clients are responsible for printing or otherwise handling errors.
This is a breaking API change.
The PrintErrors function prints all accumulated errors,
dependencies first, and is provided for convenience.
PrintErrors is based on Visit, another helper function, which
visits each Package in an import graph.
Also:
- add an Example documenting typical use.
- update clients to handle errors explicitly.
Depends on https://go-review.googlesource.com/c/tools/+/130576
Change-Id: I39407ab7f46dae2f0dd0fdde21080e172e1258aa
Reviewed-on: https://go-review.googlesource.com/131015
Reviewed-by: Michael Matloob <matloob@golang.org>
Rather than document the range of possible error types, requiring
clumsy client code to extract position information, we now expose a
single concrete type for all errors. Position information (in
standardized string form) is technically optional, but we should
strive for 100%, fixing gaps as they arise.
This change enables us to unify the Package and JSON structs in a
follow-up.
Question: should we eliminate the Config.Error hook and be silent by default?
Pro:
+ most clients suppress it.
Con:
- clients that want to print errors (e.g. vet-like tools) would have
to traverse the entire import graph to find them.
- silence is not the most fail-safe behavior.
Change-Id: Ie92b9fb7641ceda429f00928474b650d1dfadedd
Reviewed-on: https://go-review.googlesource.com/130576
Reviewed-by: Michael Matloob <matloob@golang.org>
This makes Load use the value of GOPACKAGESDRIVER in the current
process's environment if the loader's environment wasn't explicitly
set.
Note that if GOPACKAGESDRIVER is not set, we'll still search PATH in
the current process, regardless of whether PATH is set in the loader's
environment. This has not changed.
Change-Id: I3081a6cb0876a21ccd4d87b4b8d082aa47911b75
Reviewed-on: https://go-review.googlesource.com/131016
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Visit packages in deterministic order for predictable JSON.
(I don't know why the test only appears to fail on darwin;
I would expect it to randomly fail in linux 50% of the time.)
Change-Id: I5270804077fc9ca8f529a1f4657e1c35f0586579
Reviewed-on: https://go-review.googlesource.com/130755
Reviewed-by: Michael Matloob <matloob@golang.org>
This change adds the -buildflag argument to go/packages/gopackages,
which may be passed repeatedly to set config.BuildFlags. I felt -flag
was too vague.
config.Flags is renamed to config.BuildFlags for consistency, and
packages.findExternalDriver now passes -buildflag instead of -flag to
drivers. This will break existing drivers.
Change-Id: Iaed58026373a46e137a236ee9a652eb3a9433ee3
Reviewed-on: https://go-review.googlesource.com/130136
Reviewed-by: Alan Donovan <adonovan@google.com>
The legacy implementation currently always loads from source code, so
packages are more complete than expected.
Change-Id: Ib8c9f7ac590038108dba05c1f47d22e70734945c
Reviewed-on: https://go-review.googlesource.com/130095
Reviewed-by: Michael Matloob <matloob@golang.org>
In ModeTypes, we only need the roots, in ModeSyntax we need the direct
dependancies.
We now do exactly what we need rather than the entire set of transitive
dependancies.
Without this change tools operating on source of a deep dependancy tree are far
to slow.
Change-Id: I3ddbe660709184758ad33ab7cdea860244433ce6
Reviewed-on: https://go-review.googlesource.com/129355
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
For any identity imports to be added to the import map of a package,
make sure these are not already in there. If the package imports "C",
be sure not to overwrite the import map entries for "runtime/cgo", "unsafe",
and "syscall".
Test the import map when test variants are loaded. This tests the fix
in 'go list' made by https://golang.org/cl/128836Fixesgolang/go#26847
Change-Id: I05dff4d3a75fab03f333f6d88128de6a6bf169e6
Reviewed-on: https://go-review.googlesource.com/128876
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
This deletes the raw and golist packatges, instead using the main
Package structure as the serialzied form between the build tool and the
loader.
This requires a few new fields on Package for information that we used
to hide, and some extra json tags on Package.
It also required Package to have custom JSON marshalling methods.
Change-Id: Ib72171bc93e2b494b97f4b0266504acaa9b3f647
Reviewed-on: https://go-review.googlesource.com/128120
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Also:
- make a nil *Config equivalent to new(Config).
- add a test for nil *Config.
- document that Load may return an empty list of packages.
Change-Id: I642133abe18553ca8c7f46b7bd2709a03eda0b28
Reviewed-on: https://go-review.googlesource.com/128875
Reviewed-by: Michael Matloob <matloob@golang.org>
Use export data in LoadTypes mode, when it is available to get the type information.
The lock can be moved back to loadFromExportData, as each package is
loaded in topological order, and all calls to loadFromExportData for a
particular package occur when that package is being loaded.
Fixesgolang/go#26834
Change-Id: Ib6c28eb8642a473cc100d54d0aac7b90644d5d22
Reviewed-on: https://go-review.googlesource.com/128365
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Check for the go-build subdirectory to identify a filepath in the go
build cache. The go command uses a subdirectory of the default os cache
called "go-build" for the build cache.
Additionally, this responds to the comments about function names and tests in the following
CLs:
https://golang.org/cl/125536https://golang.org/cl/125302
Fixes https://github.com/golang/go/issues/26387
Change-Id: I4ec65822e1e178d3de215a773e42f09fcc5bdb0d
Reviewed-on: https://go-review.googlesource.com/128360
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
The fallback was being reset to the incorrect value
after it was set. This change fixes it.
Also add a test that triggers the fallback (a query with both
a contains: line and a regular package path).
Change-Id: I49a9aeb3a0c7d7cc308ac56f4985545315a5bfd2
Reviewed-on: https://go-review.googlesource.com/128356
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
This is mostly straightforward, except that go list -e -compiled
will now return errors if a package can't build. This is a bug.
We need to skip the errors test until that's fixed.
For now, don't try to run go list with no arguments because it will
fail. So when all arguments are contains, we will check for empty
patterns and skip running go list.
Change-Id: I7c92b1bf9448e5dbea22f9ade9fb9429945e3719
Reviewed-on: https://go-review.googlesource.com/127339
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
In Go 1.10 and earlier, the Deps field returned by go list does not
include the dependencies of any test files. To get the dependencies
of the test packages, we need to run go list another time (for a total
of 3 calls to go list), to get the dependencies of the packages the
tests import.
Fixesgolang/go#26753
Change-Id: Id3b424ac935e5ed459b4d4a55630715171bb9710
Reviewed-on: https://go-review.googlesource.com/127457
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
It is a bounds check on an array before an access, the test cannot
continue safely if it fails, so it should be fatal.
Change-Id: Icd081df9cb81d549519d69bd34739d950946ccf5
Reviewed-on: https://go-review.googlesource.com/126975
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
We ask the underlying tool to produce export data if needed, but if it
cant we should fall back to source rather than just failing.
Change-Id: I376d41b4c8ad4d3c1aef3ca4ab4e3d0466b83b56
Reviewed-on: https://go-review.googlesource.com/126875
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
go list has been changed so that when -tests is added it reports the
test packages as well as the test mains in the initial set.
This fixes all the tests that assumed the old behaviour.
I changed the test that checked the initial set to check the entire
graph because the comparison became unreadable with the expanded set,
and this seemd like a nicer standardised way to check the behaviour.
Change-Id: I69404f8ef77cd092b0f02011a7789db71bee9b47
Reviewed-on: https://go-review.googlesource.com/126796
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Add the Flags field to the Config struct in packages to provide a way
for users to pass along additional information to the underlying query
tool.
Since users that need Flags will already know something about the build
system they are using (and flags will vary depending on the underlying
build system), they can pass through the flags that they need for that
build system.
For example, build tags should be passed through using the Flags field in go build,
using "-tags=".
Change-Id: Ia65bf0d003db2f6d9aaad6cd09c602f4bc5bf3e3
Reviewed-on: https://go-review.googlesource.com/125302
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
If a 'word' provided to go/packages' Load function starts with contains,
go/packages will interpret that word as the package containing the given
file.
For example:
packages.Load(config, "contains:/usr/local/go/src/fmt/format.go")
would load the fmt package from the Go installation at /usr/local/go.
This implementation uses "go list ." in the directory the file is
contained in to find the package, but this won't work in the module
cache. We plan to add support to go list directly to help find the
containing package. Then, because we won't need to change directory,
go list will have knowledge of the correct vgo root module, and will
be able to surface correct results.
Change-Id: I6bff62447c12f13dae5e4c0c65f729d9f271c388
Reviewed-on: https://go-review.googlesource.com/126177
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Test that the import graph for packages in the GOPATH that use vendoring
are correct and are keyed by the import path as it appears in the source
file.
Change-Id: Ibf9a516b4bfcc9f2d45f349d2ae49a0dbf958e30
Reviewed-on: https://go-review.googlesource.com/125298
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Paths to files in a Package should exist and be absolute.
Check that both GoFiles and OtherFiles are absolute paths.
Change-Id: I381830425ef313a91fa49be4d1751631d0a3a0c8
Reviewed-on: https://go-review.googlesource.com/125536
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>