Make agent/tools and remove state.Tools

Make agent/tools and remove state.Tools

This branch makes an agent/tools package that is all things tools.
The environs/tools package is merged in there.  There is a test
to ensure that we don't bring in extra package dependencies to
tools, right now just "utils/set" and "version".

https://code.launchpad.net/~thumper/juju-core/agent-tools/+merge/176111

(do not edit description out of merge proposal)

[thumper, 2013-07-22 04:32:36]

Please take a look.

[wallyworld, 2013-07-22 04:52:03]

LGTM

https://codereview.appspot.com/11561044/diff/1/agent/tools/storage.go
File agent/tools/storage.go (right):

https://codereview.appspot.com/11561044/diff/1/agent/tools/storage.go#newcode84
agent/tools/storage.go:84: // environs.Storage; it exists to avoid a dependency
on environs.
Perhaps this is no longer relevant with the refactoring?

[thumper, 2013-07-22 04:55:38]

https://codereview.appspot.com/11561044/diff/1/agent/tools/storage.go
File agent/tools/storage.go (right):

https://codereview.appspot.com/11561044/diff/1/agent/tools/storage.go#newcode84
agent/tools/storage.go:84: // environs.Storage; it exists to avoid a dependency
on environs.
On 2013/07/22 04:52:03, wallyworld wrote:
> Perhaps this is no longer relevant with the refactoring?

It is still valid.  We don't want to bring in the environs package as a
dependency.

[jtv.canonical, 2013-07-23 05:41:55]

LGTM — of course I can't comment on the actual change, because my brain stopped
working after a thousand lines of collateral damage.  This may be a fun language
for re-inventing wheels in, but it's not good for engineering.

As discussed on IRC, the test against new dependencies needs commenting because
adding a dependency _can_ be a bug but in general is merely undesirable.

The loop in FindJujuCoreImports is an abomination.  Is there some better way of
phrasing this?  I realize Go forces your hand to some extent, but there must be
*something* less like assembly language.  For penance, re-read Goto Considered
Harmful.

[rog, 2013-07-23 16:34:45]

LGTM with some suggestions below.

I think this is a good example of how we can
refactor without fear given a statically typed
language.

https://codereview.appspot.com/11561044/diff/1/agent/tools/diskmanager.go
File agent/tools/diskmanager.go (right):

https://codereview.appspot.com/11561044/diff/1/agent/tools/diskmanager.go#new...
agent/tools/diskmanager.go:29: return UnpackTools(d.dataDir, tools, r)
nice change

https://codereview.appspot.com/11561044/diff/1/agent/tools/list_test.go
File agent/tools/list_test.go (right):

https://codereview.appspot.com/11561044/diff/1/agent/tools/list_test.go#newcode7
agent/tools/list_test.go:7: gc "launchpad.net/gocheck"
in this already huge CL, it would perhaps be nice to keep unnecessary changes to
a minimum, though I appreciate these are fairly automatic.

https://codereview.appspot.com/11561044/diff/1/agent/tools/tools_test.go
File agent/tools/tools_test.go (right):

https://codereview.appspot.com/11561044/diff/1/agent/tools/tools_test.go#newc...
agent/tools/tools_test.go:13: gc "launchpad.net/gocheck"
again, it would be nice to restrict changes to those directly relevant to this
CL.

https://codereview.appspot.com/11561044/diff/1/testing/imports.go
File testing/imports.go (right):

https://codereview.appspot.com/11561044/diff/1/testing/imports.go#newcode16
testing/imports.go:16: // imported by the packageName parameter.  The resulting
list removes the
// imported by the package with the given name.
?

https://codereview.appspot.com/11561044/diff/1/testing/imports.go#newcode19
testing/imports.go:19: var imports []string
i don't think you need to traverse SrcDirs manually.

i think you probably want something like the following:

pkg, err := build.Import(packageName)
if err != nil {
   c.Fatalf("cannot import %q: %v", packageName, err)
}
[...]
for _, name := range pkg.Imports {
[...]

https://codereview.appspot.com/11561044/diff/1/testing/imports.go#newcode31
testing/imports.go:31: c.Logf(packageName + " not found")
probably c.Fatalf, but no need if you make the change above.