Merge lp:~fwereade/pyjuju/go-unit-commands into lp:pyjuju/go

[William Reade]

Rough proposal for implementation of relation-get and similar commands.

In short: every command is implemented as a symlink to a single "proxy"
executable. This proxy communicates with a cmd.server.Server running in the
unit agent process, which uses a nameless SuperCommand to select and run the
command specified by the original proxy's full command line, and
communicates stderr and stdout back to the original process.

This necessitated a change to cmd, such that a new Env interface has been
added, which is responsible for the command's interactions with its
environment; cmd.DefaultEnv implements the original functionality, while
cmd.server.clientEnv implements the abstracted version which communicates
back to the proxy process. Env has ended up needing to be threaded through
much of cmd, but the impact on calling code is minimal.

https://codereview.appspot.com/5695082/

[lp:~fwereade/pyjuju/go-unit-commands]

80. By William Reade on 2012-02-28

add tests for command server

81. By William Reade on 2012-02-28

added tests for request/response

82. By William Reade on 2012-02-28

doc tweaks

[Gustavo Niemeyer (niemeyer)]

Can this be broken down in smaller chunks?

The past changes have all been similarly large in scope. I thought they
might be hard to break down, but I'm getting more skeptical that every
single change needs to touch 20 files. Reviewing, applying comments, and
getting them through the pipeline in general is a lot more traumatic
that way.

https://codereview.appspot.com/5695082/

[William Reade (fwereade)]

On Tue, 2012-02-28 at 18:22 +0000, Gustavo Niemeyer wrote:
> Can this be broken down in smaller chunks?
>
> The past changes have all been similarly large in scope. I thought they
> might be hard to break down, but I'm getting more skeptical that every
> single change needs to touch 20 files. Reviewing, applying comments, and
> getting them through the pipeline in general is a lot more traumatic
> that way.

Fair point, and this will definitely break down cleanly into 2 parts;
more might be a little tricky, but I'll see what I can do.

>
> https://codereview.appspot.com/5695082/
>

[Gustavo Niemeyer (niemeyer)]

> Fair point, and this will definitely break down cleanly into 2 parts;
> more might be a little tricky, but I'll see what I can do.

I suspect there are many more independent changes packed into that stack.

--
Gustavo Niemeyer
http://niemeyer.net
http://niemeyer.net/plus
http://niemeyer.net/twitter
http://niemeyer.net/blog

-- I'm not absolutely sure of anything.

Unmerged revisions

82. By William Reade on 2012-02-28

doc tweaks

81. By William Reade on 2012-02-28

added tests for request/response

80. By William Reade on 2012-02-28

add tests for command server

79. By William Reade on 2012-02-27

moved packages again; added max request size check; moved InitOutput from SuperCommand to Env, to prevent potential pollution of server process settings

78. By William Reade on 2012-02-27

rearrange packages

77. By William Reade on 2012-02-27

moved response handling next to request handling

76. By William Reade on 2012-02-27

merge parent

75. By William Reade on 2012-02-27

docs

74. By William Reade on 2012-02-27

we have sometests,and apparently they work...

73. By William Reade on 2012-02-27

added testable entry point for Main

=== modified file 'cmd/command.go'
--- cmd/command.go	2012-02-15 10:29:05 +0000
+++ cmd/command.go	2012-02-28 17:13:19 +0000
@@ -2,8 +2,10 @@
 
 import (
 	"fmt"
+	"io"
 	"launchpad.net/gnuflag"
 	"launchpad.net/juju/go/log"
+	stdlog "log"
 	"os"
 	"strings"
 )
@@ -32,8 +34,7 @@
 	return fmt.Sprintf("%s %s", i.Name, i.Args)
 }
 
-// Command is implemented by types that interpret any command-line arguments
-// passed to the "juju" command.
+// Command is implemented by types that interpret command-line arguments.
 type Command interface {
 	// Info returns information about the command.
 	Info() *Info
@@ -44,40 +45,85 @@
 
 	// ParsePositional is called by Parse to allow the Command to handle
 	// positional command-line arguments.
-	ParsePositional(args []string) error
+	ParsePositional(e Env, args []string) error
 
 	// Run will execute the command according to the options and positional
 	// arguments interpreted by a call to Parse.
-	Run() error
+	Run(e Env) error
+}
+
+// Env represents an environment in which a Command can be run.
+type Env interface {
+	Stderr() io.Writer
+	Stdout() io.Writer
+	Exit(int)
+	InitOutput(bool, bool, string) error
+}
+
+// DefaultEnv is the environment in which normal command-line commands are run.
+type DefaultEnv struct{}
+
+func (e *DefaultEnv) Stdout() io.Writer {
+	return os.Stdout
+}
+
+func (e *DefaultEnv) Stderr() io.Writer {
+	return os.Stderr
+}
+
+func (e *DefaultEnv) Exit(code int) {
+	os.Exit(code)
+}
+
+func (e *DefaultEnv) InitOutput(verbose bool, debug bool, logfile string) error {
+	if debug {
+		log.Debug = true
+	}
+	var target io.Writer
+	if logfile != "" {
+		var err error
+		target, err = os.OpenFile(logfile, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644)
+		if err != nil {
+			return err
+		}
+	} else if verbose || debug {
+		target = e.Stderr()
+	}
+	if target != nil {
+		log.Target = stdlog.New(target, "", stdlog.LstdFlags)
+	}
+	return nil
 }
 
 // NewFlagSet returns a FlagSet initialized for use with c.
-func NewFlagSet(c Command) *gnuflag.FlagSet {
-	f := gnuflag.NewFlagSet(c.Info().Name, gnuflag.ExitOnError)
-	f.Usage = func() { PrintUsage(c) }
+func NewFlagSet(e Env, c Command) *gnuflag.FlagSet {
+	f := gnuflag.NewFlagSet(c.Info().Name, gnuflag.ContinueOnError)
+	f.SetOutput(e.Stderr())
+	f.Usage = func() { PrintUsage(e, c) }
 	c.InitFlagSet(f)
 	return f
 }
 
 // PrintUsage prints usage information for c to stderr.
-func PrintUsage(c Command) {
+func PrintUsage(e Env, c Command) {
 	i := c.Info()
-	fmt.Fprintf(os.Stderr, "usage: %s\n", i.Usage())
-	fmt.Fprintf(os.Stderr, "purpose: %s\n", i.Purpose)
-	fmt.Fprintf(os.Stderr, "\noptions:\n")
-	NewFlagSet(c).PrintDefaults()
+	stderr := e.Stderr()
+	fmt.Fprintf(stderr, "usage: %s\n", i.Usage())
+	fmt.Fprintf(stderr, "purpose: %s\n", i.Purpose)
+	fmt.Fprintf(stderr, "\noptions:\n")
+	NewFlagSet(e, c).PrintDefaults()
 	if i.Doc != "" {
-		fmt.Fprintf(os.Stderr, "\n%s\n", strings.TrimSpace(i.Doc))
+		fmt.Fprintf(stderr, "\n%s\n", strings.TrimSpace(i.Doc))
 	}
 }
 
 // Parse parses args on c. This must be called before c is Run.
-func Parse(c Command, args []string) error {
-	f := NewFlagSet(c)
+func Parse(e Env, c Command, args []string) error {
+	f := NewFlagSet(e, c)
 	if err := f.Parse(c.Info().Intersperse, args); err != nil {
 		return err
 	}
-	return c.ParsePositional(f.Args())
+	return c.ParsePositional(e, f.Args())
 }
 
 // CheckEmpty is a utility function that returns an error if args is not empty.
@@ -89,16 +135,19 @@
 }
 
 // Main will Parse and Run a Command, and exit appropriately.
-func Main(c Command, args []string) {
-	if err := Parse(c, args[1:]); err != nil {
-		fmt.Fprintf(os.Stderr, "%v\n", err)
-		PrintUsage(c)
-		os.Exit(2)
+func Main(e Env, c Command, args []string) {
+	stderr := e.Stderr()
+	if err := Parse(e, c, args); err != nil {
+		fmt.Fprintf(stderr, "%v\n", err)
+		PrintUsage(e, c)
+		e.Exit(2)
+		return
 	}
-	if err := c.Run(); err != nil {
+	if err := c.Run(e); err != nil {
 		log.Debugf("%s command failed: %s\n", c.Info().Name, err)
-		fmt.Fprintf(os.Stderr, "%v\n", err)
-		os.Exit(1)
+		fmt.Fprintf(stderr, "%v\n", err)
+		e.Exit(1)
+		return
 	}
-	os.Exit(0)
+	e.Exit(0)
 }

=== modified file 'cmd/juju/bootstrap.go'
--- cmd/juju/bootstrap.go	2012-02-09 11:35:24 +0000
+++ cmd/juju/bootstrap.go	2012-02-28 17:13:19 +0000
@@ -30,13 +30,13 @@
 
 // ParsePositional checks that no unexpected extra command-line arguments have
 // been specified.
-func (c *BootstrapCommand) ParsePositional(args []string) error {
+func (c *BootstrapCommand) ParsePositional(e cmd.Env, args []string) error {
 	return cmd.CheckEmpty(args)
 }
 
 // Run connects to the environment specified on the command line and bootstraps
 // a juju in that environment if none already exists.
-func (c *BootstrapCommand) Run() error {
+func (c *BootstrapCommand) Run(e cmd.Env) error {
 	conn, err := juju.NewConn(c.Environment)
 	if err != nil {
 		return err

=== modified file 'cmd/juju/bootstrap_test.go'
--- cmd/juju/bootstrap_test.go	2012-02-08 11:29:32 +0000
+++ cmd/juju/bootstrap_test.go	2012-02-28 17:13:19 +0000
@@ -12,7 +12,7 @@
 
 func checkEnv(c *C, args []string, expect string) {
 	bc := &main.BootstrapCommand{}
-	err := cmd.Parse(bc, args)
+	err := cmd.Parse(&cmd.DefaultEnv{}, bc, args)
 	c.Assert(err, IsNil)
 	c.Assert(bc.Environment, Equals, expect)
 }
@@ -20,7 +20,7 @@
 func (s *BootstrapSuite) TestEnvironment(c *C) {
 	bc := &main.BootstrapCommand{}
 	c.Assert(bc.Environment, Equals, "")
-	err := cmd.Parse(bc, []string{"hotdog"})
+	err := cmd.Parse(&cmd.DefaultEnv{}, bc, []string{"hotdog"})
 	c.Assert(err, ErrorMatches, `unrecognised args: \[hotdog\]`)
 	c.Assert(bc.Environment, Equals, "")
 

=== modified file 'cmd/juju/main.go'
--- cmd/juju/main.go	2012-02-14 16:42:42 +0000
+++ cmd/juju/main.go	2012-02-28 17:13:19 +0000
@@ -18,7 +18,7 @@
 func Main(args []string) {
 	jc := cmd.NewSuperCommand("juju", jujuDoc)
 	jc.Register(&BootstrapCommand{})
-	cmd.Main(jc, args)
+	cmd.Main(&cmd.DefaultEnv{}, jc, args[1:])
 }
 
 func main() {

=== modified file 'cmd/jujud/agent.go'
--- cmd/jujud/agent.go	2012-02-21 15:13:47 +0000
+++ cmd/jujud/agent.go	2012-02-28 17:13:19 +0000
@@ -36,7 +36,7 @@
 
 // ParsePositional checks that there are no unwanted arguments, and that all
 // required flags have been set.
-func (c *agentConf) ParsePositional(args []string) error {
+func (c *agentConf) ParsePositional(e cmd.Env, args []string) error {
 	if c.jujuDir == "" {
 		return requiredError("juju-directory")
 	}

=== modified file 'cmd/jujud/initzk.go'
--- cmd/jujud/initzk.go	2012-02-21 15:13:47 +0000
+++ cmd/jujud/initzk.go	2012-02-28 17:13:19 +0000
@@ -32,7 +32,7 @@
 
 // ParsePositional checks that there are no unwanted arguments, and that all
 // required flags have been set.
-func (c *InitzkCommand) ParsePositional(args []string) error {
+func (c *InitzkCommand) ParsePositional(e cmd.Env, args []string) error {
 	if c.StateInfo.Addrs == nil {
 		return requiredError("zookeeper-servers")
 	}
@@ -46,7 +46,7 @@
 }
 
 // Run initializes zookeeper state for an environment.
-func (c *InitzkCommand) Run() error {
+func (c *InitzkCommand) Run(e cmd.Env) error {
 	// TODO connect to zookeeper; call State.Initialize
 	return fmt.Errorf("InitzkCommand.Run not implemented")
 }

=== modified file 'cmd/jujud/initzk_test.go'
--- cmd/jujud/initzk_test.go	2012-02-27 08:48:13 +0000
+++ cmd/jujud/initzk_test.go	2012-02-28 17:13:19 +0000
@@ -12,7 +12,7 @@
 
 func parseInitzkCommand(args []string) (*main.InitzkCommand, error) {
 	c := &main.InitzkCommand{}
-	err := cmd.Parse(c, args)
+	err := cmd.Parse(&cmd.DefaultEnv{}, c, args)
 	return c, err
 }
 

=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go	2012-02-15 12:28:26 +0000
+++ cmd/jujud/machine.go	2012-02-28 17:13:19 +0000
@@ -3,6 +3,7 @@
 import (
 	"fmt"
 	"launchpad.net/gnuflag"
+	"launchpad.net/juju/go/cmd"
 )
 
 // MachineAgent is a cmd.Command responsible for running a machine agent.
@@ -23,14 +24,14 @@
 
 // ParsePositional checks that there are no unwanted arguments, and that all
 // required flags have been set.
-func (a *MachineAgent) ParsePositional(args []string) error {
+func (a *MachineAgent) ParsePositional(e cmd.Env, args []string) error {
 	if a.MachineId < 0 {
 		return fmt.Errorf("--machine-id option must be set, and expects a non-negative integer")
 	}
-	return a.agentConf.ParsePositional(args)
+	return a.agentConf.ParsePositional(e, args)
 }
 
 // Run runs a machine agent.
-func (a *MachineAgent) Run() error {
+func (a *MachineAgent) Run(e cmd.Env) error {
 	return fmt.Errorf("MachineAgent.Run not implemented")
 }

=== modified file 'cmd/jujud/main.go'
--- cmd/jujud/main.go	2012-02-15 10:10:38 +0000
+++ cmd/jujud/main.go	2012-02-28 17:13:19 +0000
@@ -21,7 +21,7 @@
 	jc.Register(NewUnitAgent())
 	jc.Register(NewMachineAgent())
 	jc.Register(NewProvisioningAgent())
-	cmd.Main(jc, args)
+	cmd.Main(&cmd.DefaultEnv{}, jc, args[1:])
 }
 
 func main() {

=== modified file 'cmd/jujud/provisioning.go'
--- cmd/jujud/provisioning.go	2012-02-15 10:29:05 +0000
+++ cmd/jujud/provisioning.go	2012-02-28 17:13:19 +0000
@@ -1,6 +1,9 @@
 package main
 
-import "fmt"
+import (
+	"fmt"
+	"launchpad.net/juju/go/cmd"
+)
 
 // ProvisioningAgent is a cmd.Command responsible for running a provisioning agent.
 type ProvisioningAgent struct {
@@ -12,6 +15,6 @@
 }
 
 // Run runs a provisioning agent.
-func (a *ProvisioningAgent) Run() error {
-	return fmt.Errorf("MachineAgent.Run not implemented")
+func (a *ProvisioningAgent) Run(e cmd.Env) error {
+	return fmt.Errorf("ProvisioningAgent.Run not implemented")
 }

=== modified file 'cmd/jujud/unit.go'
--- cmd/jujud/unit.go	2012-02-17 08:03:35 +0000
+++ cmd/jujud/unit.go	2012-02-28 17:13:19 +0000
@@ -3,6 +3,7 @@
 import (
 	"fmt"
 	"launchpad.net/gnuflag"
+	"launchpad.net/juju/go/cmd"
 	"launchpad.net/juju/go/juju"
 )
 
@@ -24,17 +25,17 @@
 
 // ParsePositional checks that there are no unwanted arguments, and that all
 // required flags have been set.
-func (a *UnitAgent) ParsePositional(args []string) error {
+func (a *UnitAgent) ParsePositional(e cmd.Env, args []string) error {
 	if a.UnitName == "" {
 		return requiredError("unit-name")
 	}
 	if !juju.ValidUnit.MatchString(a.UnitName) {
 		return fmt.Errorf(`--unit-name option expects "<service>/<n>" argument`)
 	}
-	return a.agentConf.ParsePositional(args)
+	return a.agentConf.ParsePositional(e, args)
 }
 
 // Run runs a unit agent.
-func (a *UnitAgent) Run() error {
+func (a *UnitAgent) Run(e cmd.Env) error {
 	return fmt.Errorf("UnitAgent.Run not implemented")
 }

=== modified file 'cmd/jujud/util_test.go'
--- cmd/jujud/util_test.go	2012-02-27 08:48:13 +0000
+++ cmd/jujud/util_test.go	2012-02-28 17:13:19 +0000
@@ -13,23 +13,23 @@
 // command pre-parsed with the always-required options and whatever others
 // are necessary to allow parsing to succeed (specified in args).
 func CheckAgentCommand(c *C, create acCreator, args []string) main.AgentCommand {
-	err := cmd.Parse(create(), args)
+	err := cmd.Parse(&cmd.DefaultEnv{}, create(), args)
 	c.Assert(err, ErrorMatches, "--zookeeper-servers option must be set")
 	args = append(args, "--zookeeper-servers", "zk1:2181,zk2:2181")
 
-	err = cmd.Parse(create(), args)
+	err = cmd.Parse(&cmd.DefaultEnv{}, create(), args)
 	c.Assert(err, ErrorMatches, "--session-file option must be set")
 	args = append(args, "--session-file", "sf")
 
 	ac := create()
-	c.Assert(cmd.Parse(ac, args), IsNil)
+	c.Assert(cmd.Parse(&cmd.DefaultEnv{}, ac, args), IsNil)
 	c.Assert(ac.StateInfo().Addrs, DeepEquals, []string{"zk1:2181", "zk2:2181"})
 	c.Assert(ac.SessionFile(), Equals, "sf")
 	c.Assert(ac.JujuDir(), Equals, "/var/lib/juju")
 	args = append(args, "--juju-directory", "jd")
 
 	ac = create()
-	c.Assert(cmd.Parse(ac, args), IsNil)
+	c.Assert(cmd.Parse(&cmd.DefaultEnv{}, ac, args), IsNil)
 	c.Assert(ac.StateInfo().Addrs, DeepEquals, []string{"zk1:2181", "zk2:2181"})
 	c.Assert(ac.SessionFile(), Equals, "sf")
 	c.Assert(ac.JujuDir(), Equals, "jd")
@@ -44,5 +44,5 @@
 		"--session-file", "sf",
 		"--juju-directory", "jd",
 	}
-	return cmd.Parse(ac, append(common, args...))
+	return cmd.Parse(&cmd.DefaultEnv{}, ac, append(common, args...))
 }

=== added directory 'cmd/proxy'
=== added file 'cmd/proxy/main.go'
--- cmd/proxy/main.go	1970-01-01 00:00:00 +0000
+++ cmd/proxy/main.go	2012-02-28 17:13:19 +0000
@@ -0,0 +1,42 @@
+package main
+
+import (
+	"fmt"
+	"os"
+)
+
+// requireEnv returns an environment variable, and calls os.Exit(2) if it's not
+// set.
+func requireEnv(name string) string {
+	value := os.Getenv(name)
+	if value == "" {
+		fmt.Fprintf(os.Stderr, "%s not set\n", name)
+		os.Exit(2)
+	}
+	return value
+}
+
+// Main implements every command that calls back into a juju unit agent by using
+// a Proxy to ask the unit agent to execute the actual command for us and pipe
+// back stdout/stderr. This function is not redundant with main, because it
+// provides an entry point for testing with arbitrary command line arguments.
+// Individual commands should be exposed by symlinking the command name to this
+// executable.
+func Main(args []string) {
+	p := &Proxy{
+		AgentSock: requireEnv("JUJU_AGENT_SOCKET"),
+		ClientId:  requireEnv("JUJU_CLIENT_ID"),
+		Stdout:    os.Stdout,
+		Stderr:    os.Stderr,
+	}
+	result, err := p.Run(args)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "%s command failed: %v\n", args[0], err)
+		os.Exit(1)
+	}
+	os.Exit(result)
+}
+
+func main() {
+	Main(os.Args)
+}

=== added file 'cmd/proxy/proxy.go'
--- cmd/proxy/proxy.go	1970-01-01 00:00:00 +0000
+++ cmd/proxy/proxy.go	2012-02-28 17:13:19 +0000
@@ -0,0 +1,103 @@
+package main
+
+import (
+	"fmt"
+	"io"
+	"io/ioutil"
+	"launchpad.net/juju/go/cmd/server"
+	"net"
+	"os"
+	"path/filepath"
+)
+
+// Proxy is responsible for asking a running Server to execute a Command, and
+// for propagating the Command's output and return code.
+type Proxy struct {
+	AgentSock string
+	ClientId  string
+	Stdout    io.Writer
+	Stderr    io.Writer
+}
+
+// connect listens for a connection on sock, and echoes anything received to dst.
+func connect(dst io.Writer, sock string) (<-chan bool, error) {
+	l, err := net.Listen("unix", sock)
+	if err != nil {
+		return nil, err
+	}
+	done := make(chan bool)
+	go func() {
+		defer close(done)
+		defer os.Remove(sock)
+		defer l.Close()
+		conn, err := l.Accept()
+		if err != nil {
+			return
+		}
+		defer conn.Close()
+		_, err = io.Copy(dst, conn)
+		if err != nil {
+			return
+		}
+		done <- true
+	}()
+	return done, nil
+}
+
+// Run connects to a Server listening on p.AgentSock, asks it to run the Command
+// specified by args, forwards the Command's stdout/stderr to p.Stdout/p.Stderr,
+// and returns the Command's result code.
+func (p *Proxy) Run(args []string) (int, error) {
+	// Create tempdir for stdout/stderr sockets.
+	sockDir, err := ioutil.TempDir("", "juju-proxy")
+	if err != nil {
+		return 0, err
+	}
+	defer os.RemoveAll(sockDir)
+
+	// Pipe data from sockets into stdout/stderr.
+	outSock := filepath.Join(sockDir, "stdout.sock")
+	outDone, err := connect(p.Stdout, outSock)
+	if err != nil {
+		return 0, err
+	}
+	errSock := filepath.Join(sockDir, "stderr.sock")
+	errDone, err := connect(p.Stderr, errSock)
+	if err != nil {
+		return 0, err
+	}
+
+	// Connect to the agent socket.
+	conn, err := net.Dial("unix", p.AgentSock)
+	if err != nil {
+		return 0, err
+	}
+	defer conn.Close()
+
+	// Tell the agent to run the command specified by args (with the command
+	// name suitably trimmed) and pipe its output back to our sockets.
+	args[0] = filepath.Base(args[0])
+	req := &server.Request{p.ClientId, args, outSock, errSock}
+	_, err = req.WriteTo(conn)
+	if err != nil {
+		return 0, err
+	}
+
+	// Extract response code from agent connection.
+	var response server.Response
+	_, err = response.ReadFrom(conn)
+	if err != nil {
+		return 0, err
+	}
+
+	// Wait for streams to close before returning.
+	_, ok := <-outDone
+	if !ok {
+		return 0, fmt.Errorf("stdout connection failed on %s", outSock)
+	}
+	_, ok = <-errDone
+	if !ok {
+		return 0, fmt.Errorf("stderr connection failed on %s", errSock)
+	}
+	return int(response), nil
+}

=== added file 'cmd/proxy/proxy_test.go'
--- cmd/proxy/proxy_test.go	1970-01-01 00:00:00 +0000
+++ cmd/proxy/proxy_test.go	2012-02-28 17:13:19 +0000
@@ -0,0 +1,136 @@
+package main_test
+
+import (
+	"bytes"
+	"fmt"
+	"launchpad.net/gnuflag"
+	. "launchpad.net/gocheck"
+	"launchpad.net/juju/go/cmd"
+	main "launchpad.net/juju/go/cmd/proxy"
+	"launchpad.net/juju/go/cmd/server"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func Test(t *testing.T) { TestingT(t) }
+
+type TestCommand struct {
+	Value string
+}
+
+func (c *TestCommand) Info() *cmd.Info {
+	return &cmd.Info{"magic", "[options]", "do magic", "blah doc", true}
+}
+
+func (c *TestCommand) InitFlagSet(f *gnuflag.FlagSet) {
+	f.StringVar(&c.Value, "value", "", "doc")
+}
+
+func (c *TestCommand) ParsePositional(e cmd.Env, args []string) error {
+	if len(args) != 0 {
+		return fmt.Errorf("BADARGS: %s", args)
+	}
+	return nil
+}
+
+func (c *TestCommand) Run(e cmd.Env) error {
+	if c.Value != "zyxxy" {
+		return fmt.Errorf("insufficiently magic")
+	}
+	e.Stdout().Write([]byte("eye of newt"))
+	e.Stderr().Write([]byte("toe of frog"))
+	return nil
+}
+
+type ProxySuite struct {
+	sock   string
+	server *server.Server
+}
+
+var _ = Suite(&ProxySuite{})
+
+func (s *ProxySuite) SetUpSuite(c *C) {
+	factory := func(clientId string) (cmd.Command, error) {
+		if clientId != "jeremiah" {
+			return nil, fmt.Errorf("unknown client")
+		}
+		sc := cmd.NewSuperCommand("", "")
+		sc.Register(&TestCommand{})
+		return sc, nil
+	}
+	s.sock = filepath.Join(c.MkDir(), "test.sock")
+	var err error
+	s.server, err = server.NewServer(factory, s.sock)
+	c.Assert(err, IsNil)
+}
+
+func (s *ProxySuite) TearDownSuite(c *C) {
+	c.Assert(s.server.Stop(), IsNil)
+}
+
+func (s *ProxySuite) AssertCall(c *C, clientId string, args []string) (int, string, string) {
+	stdout := bytes.NewBuffer([]byte{})
+	stderr := bytes.NewBuffer([]byte{})
+	p := &main.Proxy{
+		AgentSock: s.sock,
+		ClientId:  clientId,
+		Stdout:    stdout,
+		Stderr:    stderr,
+	}
+	result, err := p.Run(args)
+	c.Assert(err, IsNil)
+	return result, stdout.String(), stderr.String()
+}
+
+func (s *ProxySuite) TestSuccess(c *C) {
+	result, stdout, stderr := s.AssertCall(
+		c, "jeremiah", []string{"magic", "--value", "zyxxy"})
+	c.Assert(result, Equals, 0)
+	c.Assert(stdout, Equals, "eye of newt")
+	c.Assert(stderr, Equals, "toe of frog")
+}
+
+func (s *ProxySuite) TestCommandPath(c *C) {
+	result, stdout, stderr := s.AssertCall(
+		c, "jeremiah", []string{"some/path/to/magic", "--value", "zyxxy"})
+	c.Assert(result, Equals, 0)
+	c.Assert(stdout, Equals, "eye of newt")
+	c.Assert(stderr, Equals, "toe of frog")
+}
+
+func (s *ProxySuite) TestBadRun(c *C) {
+	result, stdout, stderr := s.AssertCall(
+		c, "jeremiah", []string{"magic", "--value", "wrong"})
+	c.Assert(result, Equals, 1)
+	c.Assert(stdout, Equals, "")
+	errLines := strings.Split(stderr, "\n")
+	c.Assert(errLines[0], Equals, "insufficiently magic")
+}
+
+func (s *ProxySuite) TestBadClient(c *C) {
+	result, stdout, stderr := s.AssertCall(
+		c, "methuselah", []string{"magic"})
+	c.Assert(result, Equals, 1)
+	c.Assert(stdout, Equals, "")
+	errLines := strings.Split(stderr, "\n")
+	c.Assert(errLines[0], Equals, "command unavailable: unknown client")
+}
+
+func (s *ProxySuite) TestNonsenseArgs(c *C) {
+	result, stdout, stderr := s.AssertCall(
+		c, "jeremiah", []string{"magic", "--cheese"})
+	c.Assert(result, Equals, 2)
+	c.Assert(stdout, Equals, "")
+	errLines := strings.Split(stderr, "\n")
+	c.Assert(errLines[0], Equals, "flag provided but not defined: --cheese")
+}
+
+func (s *ProxySuite) TestUnknownCommand(c *C) {
+	result, stdout, stderr := s.AssertCall(
+		c, "jeremiah", []string{"witchcraft"})
+	c.Assert(result, Equals, 2)
+	c.Assert(stdout, Equals, "")
+	errLines := strings.Split(stderr, "\n")
+	c.Assert(errLines[0], Equals, "unrecognised command: witchcraft")
+}

=== added directory 'cmd/server'
=== added file 'cmd/server/protocol.go'
--- cmd/server/protocol.go	1970-01-01 00:00:00 +0000
+++ cmd/server/protocol.go	2012-02-28 17:13:19 +0000
@@ -0,0 +1,86 @@
+package server
+
+import (
+	"encoding/binary"
+	"fmt"
+	"io"
+	"launchpad.net/goyaml"
+)
+
+// Request contains all the information necessary for a Server to run a Command
+// on behalf of a Proxy.
+type Request struct {
+	ClientId string
+	Args     []string
+	OutSock  string
+	ErrSock  string
+}
+
+var (
+	bo      = binary.BigEndian
+	codeS   = int64(binary.Size(int32(0)))
+	sizeS   = int64(binary.Size(uint32(0)))
+	maxSize = uint32(1024 * 1024)
+)
+
+// ReadFrom fills in a Request's fields using data read from src (which is
+// expected in the format written by WriteTo).
+func (r *Request) ReadFrom(src io.Reader) (count int64, err error) {
+	var size uint32
+	err = binary.Read(src, bo, &size)
+	if err != nil {
+		return
+	}
+	count += sizeS
+	if size > maxSize {
+		err = fmt.Errorf("unreasonably large request: %d bytes", size)
+		return
+	}
+	data := make([]byte, size)
+	n, err := io.ReadFull(src, data[:])
+	count += int64(n)
+	if err != nil {
+		return
+	}
+	err = goyaml.Unmarshal(data, r)
+	return
+}
+
+// WriteTo writes a Request's data to dst, in a format suitable for
+// reconstruction by ReadFrom.
+func (r *Request) WriteTo(dst io.Writer) (int64, error) {
+	yaml, err := goyaml.Marshal(r)
+	if err != nil {
+		return 0, err
+	}
+	bytes := []byte(yaml)
+	err = binary.Write(dst, bo, uint32(len(bytes)))
+	if err != nil {
+		return 0, err
+	}
+	n, err := dst.Write(bytes)
+	return int64(n) + sizeS, err
+}
+
+// Response is a command's return code, with extra methods to make it convenient
+// to transmit and interpret.
+type Response int32
+
+// ReadFrom decodes a Response from src (expected in the format written by WriteTo).
+func (r *Response) ReadFrom(src io.Reader) (int64, error) {
+	err := binary.Read(src, bo, r)
+	if err != nil {
+		return 0, err
+	}
+	return codeS, nil
+}
+
+// WriteTo encodes a Response to dst, in a format suitable for reconstruction by
+// ReadFrom.
+func (r *Response) WriteTo(dst io.Writer) (int64, error) {
+	err := binary.Write(dst, bo, r)
+	if err != nil {
+		return 0, err
+	}
+	return codeS, nil
+}

=== added file 'cmd/server/protocol_test.go'
--- cmd/server/protocol_test.go	1970-01-01 00:00:00 +0000
+++ cmd/server/protocol_test.go	2012-02-28 17:13:19 +0000
@@ -0,0 +1,38 @@
+package server_test
+
+import (
+	"bytes"
+	. "launchpad.net/gocheck"
+	"launchpad.net/juju/go/cmd/server"
+)
+
+type ProtocolSuite struct{}
+
+var _ = Suite(&ProtocolSuite{})
+
+func (s *ProtocolSuite) TestWriteReadRequest(c *C) {
+	buf := bytes.NewBuffer([]byte{})
+	req0 := &server.Request{"client", []string{"command", "arg"}, "out", "err"}
+	count0, err := req0.WriteTo(buf)
+	c.Assert(err, IsNil)
+	var req1 server.Request
+	count1, err := req1.ReadFrom(buf)
+	c.Assert(err, IsNil)
+	c.Assert(count1, Equals, count0)
+	c.Assert(req1.ClientId, Equals, "client")
+	c.Assert(req1.Args, DeepEquals, []string{"command", "arg"})
+	c.Assert(req1.OutSock, Equals, "out")
+	c.Assert(req1.ErrSock, Equals, "err")
+}
+
+func (s *ProtocolSuite) TestWriteReadResponse(c *C) {
+	buf := bytes.NewBuffer([]byte{})
+	resp0 := server.Response(-123)
+	count0, err := resp0.WriteTo(buf)
+	c.Assert(err, IsNil)
+	var resp1 server.Response
+	count1, err := resp1.ReadFrom(buf)
+	c.Assert(err, IsNil)
+	c.Assert(count1, Equals, count0)
+	c.Assert(resp1, Equals, resp0)
+}

=== added file 'cmd/server/server.go'
--- cmd/server/server.go	1970-01-01 00:00:00 +0000
+++ cmd/server/server.go	2012-02-28 17:13:19 +0000
@@ -0,0 +1,149 @@
+package server
+
+import (
+	"fmt"
+	"io"
+	"launchpad.net/juju/go/cmd"
+	"launchpad.net/tomb"
+	"net"
+)
+
+// clientEnv implements cmd.Env so as to act for the Proxy which communicated a
+// Request to a Server.
+type clientEnv struct {
+	stdout io.WriteCloser
+	stderr io.WriteCloser
+	ret    io.WriteCloser
+}
+
+// newClientEnv returns a clientEnv whose stdout/stderr are connected to the
+// sockets specified in req, and whose Exit() will write the return code to conn.
+func newClientEnv(conn net.Conn, req *Request) (*clientEnv, error) {
+	stdout, err := net.Dial("unix", req.OutSock)
+	if err != nil {
+		return nil, err
+	}
+	stderr, err := net.Dial("unix", req.ErrSock)
+	if err != nil {
+		return nil, err
+	}
+	return &clientEnv{stdout, stderr, conn}, nil
+}
+
+func (e *clientEnv) Stdout() io.Writer {
+	return e.stdout
+}
+
+func (e *clientEnv) Stderr() io.Writer {
+	return e.stderr
+}
+
+func (e *clientEnv) Exit(code int) {
+	response := Response(code)
+	response.WriteTo(e.ret)
+	e.ret.Close()
+	e.stdout.Close()
+	e.stderr.Close()
+}
+
+func (e *clientEnv) InitOutput(verbose bool, debug bool, logfile string) error {
+	// TODO move away from global logging to enable this to do something
+	// helpful? (We don't want to override this process's logging setup with
+	// the proxy command's, but I think it's still worth exposing the flags).
+	return nil
+}
+
+// CmdFactory returns an appropriate Command for the context specified by
+// clientId.
+type CmdFactory func(clientId string) (cmd.Command, error)
+
+// Server is a bare-bones "RPC" ("RCC"?) server which accepts serialized Request
+// structs and runs the specified Command, piping its output to the sockets
+// specified in the Request and writing its return code back to the original
+// connection.
+type Server struct {
+	cmdFactory CmdFactory
+	listener   net.Listener
+	reqs       chan bool
+	*tomb.Tomb
+}
+
+var (
+	maxReqs = 4
+)
+
+// NewServer starts a Server listening on sock and exposing the Command
+// returned by f.
+func NewServer(f CmdFactory, sock string) (*Server, error) {
+	s := &Server{f, nil, make(chan bool, maxReqs), tomb.New()}
+	if err := s.serve(sock); err != nil {
+		return nil, err
+	}
+	return s, nil
+}
+
+// serve starts listening for Requests sent on sock.
+func (s *Server) serve(sock string) error {
+	var err error
+	s.listener, err = net.Listen("unix", sock)
+	if err != nil {
+		return err
+	}
+	go func() {
+		<-s.Dying
+		s.listener.Close()
+		for i := 0; i < maxReqs; i++ {
+			s.reqs <- true
+		}
+		s.Done()
+	}()
+	go func() {
+		for {
+			s.reqs <- true
+			conn, err := s.listener.Accept()
+			if err != nil {
+				// if err *is* tomb.Stop, it implies we Stop~ed the listener;
+				// and that the error is an expected connection-lost complaint,
+				// which we should quietly suppress. I feel that this bit could
+				// maybe be done better..?
+				if s.Err() != tomb.Stop {
+					s.Fatal(err)
+				}
+				<-s.reqs
+				return
+			}
+			go s.handle(conn)
+		}
+	}()
+	return nil
+}
+
+// handle reads and handles a single Request from conn.
+func (s *Server) handle(conn net.Conn) {
+	defer conn.Close()
+	defer func() { <-s.reqs }()
+
+	req := &Request{}
+	_, err := req.ReadFrom(conn)
+	if err != nil {
+		return
+	}
+	env, err := newClientEnv(conn, req)
+	if err != nil {
+		return
+	}
+	c, err := s.cmdFactory(req.ClientId)
+	if err != nil {
+		fmt.Fprintf(env.Stderr(), "command unavailable: %s", err)
+		env.Exit(1)
+		return
+	}
+	cmd.Main(env, c, req.Args)
+}
+
+// Stop stops the server from accepting new requests, and returns once all
+// current requests have completed.
+func (s *Server) Stop() error {
+	s.Fatal(tomb.Stop)
+	return s.Wait()
+}

=== added file 'cmd/server/server_test.go'
--- cmd/server/server_test.go	1970-01-01 00:00:00 +0000
+++ cmd/server/server_test.go	2012-02-28 17:13:19 +0000
@@ -0,0 +1,205 @@
+package server_test
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"launchpad.net/gnuflag"
+	. "launchpad.net/gocheck"
+	"launchpad.net/juju/go/cmd"
+	proxy "launchpad.net/juju/go/cmd/proxy"
+	"launchpad.net/juju/go/cmd/server"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func Test(t *testing.T) { TestingT(t) }
+
+type TestCommand struct {
+	Value string
+}
+
+func (c *TestCommand) Info() *cmd.Info {
+	return &cmd.Info{"magic", "[options]", "do magic", "blah doc", true}
+}
+
+func (c *TestCommand) InitFlagSet(f *gnuflag.FlagSet) {
+	f.StringVar(&c.Value, "value", "", "doc")
+}
+
+func (c *TestCommand) ParsePositional(e cmd.Env, args []string) error {
+	return cmd.CheckEmpty(args)
+}
+
+func (c *TestCommand) Run(e cmd.Env) error {
+	if c.Value != "zyxxy" {
+		return fmt.Errorf("insufficiently magic")
+	}
+	e.Stdout().Write([]byte("eye of newt"))
+	e.Stderr().Write([]byte("toe of frog"))
+	return nil
+}
+
+type WaitCommand struct {
+	unblock chan bool
+}
+
+func (c *WaitCommand) Info() *cmd.Info {
+	return &cmd.Info{"wait", "", "wait until unblocked", "blah doc", true}
+}
+
+func (c *WaitCommand) InitFlagSet(f *gnuflag.FlagSet) {}
+
+func (c *WaitCommand) ParsePositional(e cmd.Env, args []string) error {
+	return cmd.CheckEmpty(args)
+}
+
+func (c *WaitCommand) Run(e cmd.Env) error {
+	fmt.Fprintf(e.Stderr(), "waiting...\n")
+	_, ok := <-c.unblock
+	fmt.Fprintf(e.Stderr(), "unblocked; ok = %s\n", ok)
+	if !ok {
+		return fmt.Errorf("oh no, command failed")
+	}
+	return nil
+}
+
+type ServerSuite struct{}
+
+var _ = Suite(&ServerSuite{})
+
+func startServer(c *C, register func(*cmd.SuperCommand)) (string, *server.Server) {
+	factory := func(clientId string) (cmd.Command, error) {
+		if clientId != "jeremiah" {
+			return nil, fmt.Errorf("unknown client")
+		}
+		sc := cmd.NewSuperCommand("", "")
+		register(sc)
+		return sc, nil
+	}
+	sock := filepath.Join(c.MkDir(), "test.sock")
+	server, err := server.NewServer(factory, sock)
+	c.Assert(err, IsNil)
+	return sock, server
+}
+
+func newProxy(sock string, stdout io.Writer, stderr io.Writer) *proxy.Proxy {
+	if stdout == nil {
+		stdout = bytes.NewBuffer([]byte{})
+	}
+	if stderr == nil {
+		stderr = bytes.NewBuffer([]byte{})
+	}
+	return &proxy.Proxy{
+		AgentSock: sock,
+		ClientId:  "jeremiah",
+		Stdout:    stdout,
+		Stderr:    stderr,
+	}
+}
+
+func (s *ServerSuite) TestStartStop(c *C) {
+	sock, server := startServer(c, func(sc *cmd.SuperCommand) {
+		sc.Register(&TestCommand{})
+	})
+
+	stdout := bytes.NewBuffer([]byte{})
+	stderr := bytes.NewBuffer([]byte{})
+	p := newProxy(sock, stdout, stderr)
+	args := []string{"magic", "--value", "zyxxy"}
+	result, err := p.Run(args)
+	c.Assert(err, IsNil)
+	c.Assert(result, Equals, 0)
+	c.Assert(stdout.String(), Equals, "eye of newt")
+	c.Assert(stderr.String(), Equals, "toe of frog")
+
+	server.Stop()
+	stdout.Reset()
+	stderr.Reset()
+	_, err = p.Run(args)
+	c.Assert(err, NotNil)
+}
+
+func (s *ServerSuite) TestConcurrentStop(c *C) {
+	concurrency := 4
+	unblockOne := make(chan bool)
+	gotOneRequest := make(chan bool)
+	sock, server := startServer(c, func(sc *cmd.SuperCommand) {
+		sc.Register(&WaitCommand{unblockOne})
+		gotOneRequest <- true
+	})
+
+	args := []string{"wait"}
+	oneRunComplete := make(chan bool)
+	for i := 0; i < concurrency; i++ {
+		go func() {
+			result, err := newProxy(sock, nil, nil).Run(args)
+			c.Assert(err, IsNil)
+			c.Assert(result, Equals, 0)
+			oneRunComplete <- true
+		}()
+		<-gotOneRequest
+	}
+
+	stopComplete := make(chan bool)
+	go func() {
+		err := server.Stop()
+		c.Assert(err, IsNil)
+		stopComplete <- true
+	}()
+
+	for i := 0; i < concurrency; i++ {
+		select {
+		case <-server.Dying:
+			// Jolly good; check it's not accepting new requests.
+			_, err := newProxy(sock, nil, nil).Run(args)
+			c.Assert(err, NotNil)
+		case <-server.Dead:
+			c.Fatal("server did not finish processing requests")
+		}
+
+		unblockOne <- true
+		<-oneRunComplete
+	}
+	<-server.Dead
+	<-stopComplete
+}
+
+func (s *ServerSuite) TestIgnoresHugeRequests(c *C) {
+	sock, server := startServer(c, func(sc *cmd.SuperCommand) {
+		c.Fatal("should never call this")
+	})
+	defer server.Stop()
+
+	p := newProxy(sock, nil, nil)
+	stupidArgs := [1e6]string{}
+	_, err := p.Run(stupidArgs[:])
+	c.Assert(err, NotNil)
+}
+
+func (s *ServerSuite) TestBadCommands(c *C) {
+	sock, server := startServer(c, func(sc *cmd.SuperCommand) {
+		sc.Register(&TestCommand{})
+	})
+	defer server.Stop()
+
+	assertBadCmd := func(clientId string, args []string, expect int, stderr0 string) {
+		stderr := bytes.NewBuffer([]byte{})
+		p := newProxy(sock, nil, stderr)
+		p.ClientId = clientId
+		result, err := p.Run(args)
+		c.Assert(err, IsNil)
+		c.Assert(result, Equals, expect)
+		lines := strings.Split(stderr.String(), "\n")
+		c.Assert(lines[0], Equals, stderr0)
+	}
+	assertBadCmd(
+		"jeremiah", []string{"witchcraft"}, 2, "unrecognised command: witchcraft")
+	assertBadCmd(
+		"jeremiah", []string{"magic", "--ham"}, 2, "flag provided but not defined: --ham")
+	assertBadCmd(
+		"jeremiah", []string{"magic", "--value", "wrong"}, 1, "insufficiently magic")
+	assertBadCmd(
+		"jeff", []string{"magic", "--value", "zyxxy"}, 1, "command unavailable: unknown client")
+}

=== modified file 'cmd/supercommand.go'
--- cmd/supercommand.go	2012-02-17 08:03:35 +0000
+++ cmd/supercommand.go	2012-02-28 17:13:19 +0000
@@ -2,11 +2,7 @@
 
 import (
 	"fmt"
-	"io"
 	"launchpad.net/gnuflag"
-	"launchpad.net/juju/go/log"
-	stdlog "log"
-	"os"
 	"sort"
 	"strings"
 )
@@ -72,7 +68,9 @@
 	var info *Info
 	if c.subcmd != nil {
 		info = c.subcmd.Info()
-		info.Name = fmt.Sprintf("%s %s", c.Name, info.Name)
+		if c.Name != "" {
+			info.Name = fmt.Sprintf("%s %s", c.Name, info.Name)
+		}
 		return info
 	}
 	return &Info{
@@ -89,7 +87,7 @@
 	if c.subcmd != nil {
 		c.subcmd.InitFlagSet(f)
 	}
-	// SuperCommand's flags are always added to subcommands./ Note that the
+	// SuperCommand's flags are always added to subcommands'. Note that the
 	// flag defaults come from the SuperCommand itself, so that ParsePositional
 	// can call Parse twice on the same SuperCommand without losing information.
 	f.StringVar(&c.LogFile, "log-file", c.LogFile, "path to write log to")
@@ -101,9 +99,9 @@
 
 // ParsePositional selects the subcommand specified by subargs and uses it to
 // Parse any remaining unconsumed command-line arguments.
-func (c *SuperCommand) ParsePositional(subargs []string) error {
+func (c *SuperCommand) ParsePositional(e Env, subargs []string) error {
 	if c.subcmd != nil {
-		return c.subcmd.ParsePositional(subargs)
+		return c.subcmd.ParsePositional(e, subargs)
 	}
 	if len(subargs) == 0 {
 		return fmt.Errorf("no command specified")
@@ -112,38 +110,16 @@
 	if c.subcmd, found = c.subcmds[subargs[0]]; !found {
 		return fmt.Errorf("unrecognised command: %s", subargs[0])
 	}
-	return Parse(c, subargs[1:])
-}
-
-// initOutput sets up logging to a file or to stderr depending on what's been
-// requested on the command line.
-func (c *SuperCommand) initOutput() error {
-	if c.Debug {
-		log.Debug = true
-	}
-	var target io.Writer
-	if c.LogFile != "" {
-		var err error
-		target, err = os.OpenFile(c.LogFile, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0644)
-		if err != nil {
-			return err
-		}
-	} else if c.Verbose || c.Debug {
-		target = os.Stderr
-	}
-	if target != nil {
-		log.Target = stdlog.New(target, "", stdlog.LstdFlags)
-	}
-	return nil
+	return Parse(e, c, subargs[1:])
 }
 
 // Run executes the subcommand that was selected when Parse was called.
-func (c *SuperCommand) Run() error {
-	if err := c.initOutput(); err != nil {
+func (c *SuperCommand) Run(e Env) error {
+	if err := e.InitOutput(c.Verbose, c.Debug, c.LogFile); err != nil {
 		return err
 	}
 	if c.subcmd == nil {
 		panic("Run: missing subcommand; Parse failed or not called")
 	}
-	return c.subcmd.Run()
+	return c.subcmd.Run(e)
 }

=== modified file 'cmd/supercommand_test.go'
--- cmd/supercommand_test.go	2012-02-09 11:35:24 +0000
+++ cmd/supercommand_test.go	2012-02-28 17:13:19 +0000
@@ -31,20 +31,20 @@
 	f.StringVar(&c.Value, "value", "", "doc")
 }
 
-func (c *TestCommand) ParsePositional(args []string) error {
+func (c *TestCommand) ParsePositional(e cmd.Env, args []string) error {
 	if len(args) != 0 {
 		return fmt.Errorf("BADARGS: %s", args)
 	}
 	return nil
 }
 
-func (c *TestCommand) Run() error {
+func (c *TestCommand) Run(e cmd.Env) error {
 	return fmt.Errorf("BORKEN: value is %s.", c.Value)
 }
 
 func parseEmpty(args []string) (*cmd.SuperCommand, error) {
 	jc := cmd.NewSuperCommand("jujutest", "")
-	err := cmd.Parse(jc, args)
+	err := cmd.Parse(&cmd.DefaultEnv{}, jc, args)
 	return jc, err
 }
 
@@ -52,7 +52,7 @@
 	jc := cmd.NewSuperCommand("jujutest", "")
 	tc := &TestCommand{Name: "defenestrate"}
 	jc.Register(tc)
-	err := cmd.Parse(jc, args)
+	err := cmd.Parse(&cmd.DefaultEnv{}, jc, args)
 	return jc, tc, err
 }
 
@@ -161,7 +161,7 @@
 	jc, _, err := parseDefenestrate(args)
 	c.Assert(err, IsNil)
 
-	err = jc.Run()
+	err = jc.Run(&cmd.DefaultEnv{})
 	c.Assert(err, ErrorMatches, "BORKEN: value is cheese.")
 
 	c.Assert(log.Debug, Equals, debug)