Merge lp:~fwereade/pyjuju/go-cmd-context into lp:pyjuju/go

[William Reade]

add cmd.Context to allow for commands with non-global output/logging

prereq: lp:~fwereade/juju/go-log-type

The top-level Main, Run, Parse and NewFlagSet function have all become methods
on the new Context type, as has SuperCommand.initOutput. Main and Parse, which
are both used by code outside this package, have been retained for
convenience's sake and act as they did before.

The Command interface has changed slightly; Run now expects a *Context, and
ParsePositional can now return a []string to indicate that the Command should
be Parsed again with the returned args; this allows us to move the re-Parsing
logic to Context.Parse; even though SuperCommand is the only user, this
allows us to avoid passing a Context into ParsePositional, which feels quite
inappropriate. (Given that every Command has to implement ParsePositional,
it's better to add additional nil return values everywhere than to pass an
unused Context around everywhere).

This doesn't feel like an overwhelmingly large change, but it doesn't diff
very nicely. Most of the changes should be explained by the above; there's
also an unrelated type rename in supercommand_test.go because, er, it seemed
like a good idea at the time (and still doesn't feel like a bad one, even if
it adds to the noise a bit).

https://codereview.appspot.com/5719050/

[William Reade (fwereade)]

Reviewers: mp+95602_code.launchpad.net,

Message:
Please take a look.

Description:
prereq: lp:~fwereade/juju/go-log-type

The top-level Main, Run, Parse and NewFlagSet function have all become
methods
on the new Context type, as has SuperCommand.initOutput. Main and Parse,
which
are both used by code outside this package, have been retained for
convenience's sake and act as they did before.

The Command interface has changed slightly; Run now expects a *Context,
and
ParsePositional can now return a []string to indicate that the Command
should
be Parsed again with the returned args; this allows us to move the
re-Parsing
logic to Context.Parse; even though SuperCommand is the only user, this
allows us to avoid passing a Context into ParsePositional, which feels
quite
inappropriate. (Given that every Command has to implement
ParsePositional,
it's better to add additional nil return values everywhere than to pass
an
unused Context around everywhere).

This doesn't feel like an overwhelmingly large change, but it doesn't
diff
very nicely. Most of the changes should be explained by the above;
there's
also an unrelated type rename in supercommand_test.go because, er, it
seemed
like a good idea at the time (and still doesn't feel like a bad one,
even if
it adds to the noise a bit).

https://code.launchpad.net/~fwereade/juju/go-cmd-context/+merge/95602

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/5719050/

Affected files:
   M cmd/command.go
   A cmd/context.go
   A cmd/context_test.go
   M cmd/juju/bootstrap.go
   M cmd/juju/main.go
   M cmd/jujud/agent.go
   M cmd/jujud/initzk.go
   M cmd/jujud/machine.go
   M cmd/jujud/main.go
   M cmd/jujud/provisioning.go
   M cmd/jujud/unit.go
   M cmd/supercommand.go
   M cmd/supercommand_test.go
   M log/log.go
   M log/log_test.go
   M store/store_test.go

Unmerged revisions

83. By William Reade on 2012-03-02

merge parent

82. By William Reade on 2012-03-02

added direct tests for Context

81. By William Reade on 2012-03-02

rearranged cmd output to avoid reprinting errors/usage

80. By William Reade on 2012-03-01

er, remove the Exit field itself...

79. By William Reade on 2012-03-01

Exit doesn't really want to be part of Context, I think...

78. By William Reade on 2012-03-01

added cmd.Context, which now exposes most of the original top-level functions, and SuperCommand's erstwhile initOutput method; retained top-level Parse and Main functions, which act as before, but by acting via a default Context rather than by directly hitting os.Exit, etc

77. By William Reade on 2012-03-01

add log.Log type, with an instance always available as log.Global

=== modified file 'cmd/command.go'
--- cmd/command.go	2012-02-15 10:29:05 +0000
+++ cmd/command.go	2012-03-02 16:10:26 +0000
@@ -3,9 +3,7 @@
 import (
 	"fmt"
 	"launchpad.net/gnuflag"
-	"launchpad.net/juju/go/log"
 	"os"
-	"strings"
 )
 
 // Info holds everything necessary to describe a Command's intent and usage.
@@ -32,8 +30,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
@@ -43,41 +40,13 @@
 	InitFlagSet(f *gnuflag.FlagSet)
 
 	// ParsePositional is called by Parse to allow the Command to handle
-	// positional command-line arguments.
-	ParsePositional(args []string) error
+	// positional command-line arguments; if any arguments are returned, the
+	// Command will be reparsed using those arguments.
+	ParsePositional(args []string) ([]string, error)
 
-	// Run will execute the command according to the options and positional
+	// Run will execute the Command according to the options and positional
 	// arguments interpreted by a call to Parse.
-	Run() error
-}
-
-// 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) }
-	c.InitFlagSet(f)
-	return f
-}
-
-// PrintUsage prints usage information for c to stderr.
-func PrintUsage(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()
-	if i.Doc != "" {
-		fmt.Fprintf(os.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)
-	if err := f.Parse(c.Info().Intersperse, args); err != nil {
-		return err
-	}
-	return c.ParsePositional(f.Args())
+	Run(ctx *Context) error
 }
 
 // CheckEmpty is a utility function that returns an error if args is not empty.
@@ -88,17 +57,14 @@
 	return nil
 }
 
-// Main will Parse and Run a Command, and exit appropriately.
+// Parse parses args on c in a default context suitable for a command-line tool.
+// This must be called before c is Run.
+func Parse(c Command, args []string) error {
+	return NewContext().Parse(c, args)
+}
+
+// Main will Parse and Run a Command in a default context suitable for a
+// command-line tool, up to and including calling os.Exit.
 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)
-	}
-	if err := c.Run(); err != nil {
-		log.Debugf("%s command failed: %s\n", c.Info().Name, err)
-		fmt.Fprintf(os.Stderr, "%v\n", err)
-		os.Exit(1)
-	}
-	os.Exit(0)
+	os.Exit(NewContext().Main(c, args))
 }
 
=== added file 'cmd/context.go'
--- cmd/context.go	1970-01-01 00:00:00 +0000
+++ cmd/context.go	2012-03-02 16:10:26 +0000
@@ -0,0 +1,117 @@
+package cmd
+
+import (
+	"fmt"
+	"io"
+	"launchpad.net/gnuflag"
+	"launchpad.net/juju/go/log"
+	stdlog "log"
+	"os"
+	"strings"
+)
+
+// sink is an io.Writer that doesn't write anything. This allows us to suppress
+// gnuflag output when necessary (f.SetOutput(nil) will actually cause f to
+// write to os.Stderr).
+type sink struct{}
+
+func (*sink) Write(data []byte) (int, error) {
+	return len(data), nil
+}
+
+// Context holds those parts of the environment that it is useful to abstract
+// out to allow well-behaved Commands to be executed as part of another process.
+type Context struct {
+	Stdout io.Writer
+	Stderr io.Writer
+	Log    *log.Log
+}
+
+// NewContext returns a Context suitable for a normal command-line tool.
+func NewContext() *Context {
+	return &Context{
+		Stdout: os.Stdout,
+		Stderr: os.Stderr,
+		Log:    log.Global,
+	}
+}
+
+// InitLog sets up logging to a file or to ctx.Stderr as directed.
+func (ctx *Context) InitLog(verbose bool, debug bool, logfile string) error {
+	if debug {
+		ctx.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 = ctx.Stderr
+	}
+	if target != nil {
+		ctx.Log.Target = stdlog.New(target, "", stdlog.LstdFlags)
+	}
+	return nil
+}
+
+// newFlagSet returns a FlagSet initialized for use with c.
+func (ctx *Context) newFlagSet(c Command, output io.Writer) *gnuflag.FlagSet {
+	f := gnuflag.NewFlagSet(c.Info().Name, gnuflag.ContinueOnError)
+	f.SetOutput(output)
+	f.Usage = func() { ctx.PrintUsage(c, output) }
+	c.InitFlagSet(f)
+	return f
+}
+
+// PrintUsage prints usage information for c to output. If output is nil, usage
+// information is printed to ctx.Stderr (for consistency with gnuflag.FlagSet's
+// SetOutput method).
+func (ctx *Context) PrintUsage(c Command, output io.Writer) {
+	if output == nil {
+		output = ctx.Stderr
+	}
+	i := c.Info()
+	fmt.Fprintf(output, "usage: %s\n", i.Usage())
+	fmt.Fprintf(output, "purpose: %s\n", i.Purpose)
+	fmt.Fprintf(output, "\noptions:\n")
+	f := ctx.newFlagSet(c, output)
+	f.PrintDefaults()
+	if i.Doc != "" {
+		fmt.Fprintf(output, "\n%s\n", strings.TrimSpace(i.Doc))
+	}
+}
+
+// Parse parses args on c. This must be called before c is Run.
+func (ctx *Context) Parse(c Command, args []string) error {
+	f := ctx.newFlagSet(c, &sink{})
+	if err := f.Parse(c.Info().Intersperse, args); err != nil {
+		return err
+	}
+	reparse, err := c.ParsePositional(f.Args())
+	if err != nil {
+		return err
+	}
+	if reparse != nil {
+		return ctx.Parse(c, reparse)
+	}
+	return nil
+}
+
+// Main will Parse and Run a Command, and return a process exit code. args
+// should contain flags and arguments only (and not the top-level command name).
+func (ctx *Context) Main(c Command, args []string) int {
+	if err := ctx.Parse(c, args); err != nil {
+		fmt.Fprintf(ctx.Stderr, "%v\n", err)
+		ctx.PrintUsage(c, nil)
+		return 2
+	}
+	if err := c.Run(ctx); err != nil {
+		ctx.Log.Debugf("%s command failed: %s\n", c.Info().Name, err)
+		fmt.Fprintf(ctx.Stderr, "%v\n", err)
+		return 1
+	}
+	return 0
+}
 
=== added file 'cmd/context_test.go'
--- cmd/context_test.go	1970-01-01 00:00:00 +0000
+++ cmd/context_test.go	2012-03-02 16:10:26 +0000
@@ -0,0 +1,148 @@
+package cmd_test
+
+import (
+	"bytes"
+	"errors"
+	"io"
+	"io/ioutil"
+	"launchpad.net/gnuflag"
+	. "launchpad.net/gocheck"
+	cmd "launchpad.net/juju/go/cmd"
+	"launchpad.net/juju/go/log"
+	"path/filepath"
+)
+
+func getContext() *cmd.Context {
+	return &cmd.Context{
+		bytes.NewBuffer([]byte{}), bytes.NewBuffer([]byte{}), &log.Log{}}
+}
+
+func str(stream io.Writer) string {
+	return stream.(*bytes.Buffer).String()
+}
+
+type CtxCommand struct {
+	Value string
+}
+
+func (c *CtxCommand) Info() *cmd.Info {
+	return &cmd.Info{"cmd-name", "[options]", "cmd-purpose", "cmd-doc", true}
+}
+
+func (c *CtxCommand) InitFlagSet(f *gnuflag.FlagSet) {
+	f.StringVar(&c.Value, "opt", "", "opt-doc")
+}
+
+func (c *CtxCommand) ParsePositional(args []string) ([]string, error) {
+	return nil, cmd.CheckEmpty(args)
+}
+
+func (c *CtxCommand) Run(ctx *cmd.Context) error {
+	if c.Value == "error" {
+		return errors.New("oh noes!")
+	}
+	ctx.Stdout.Write([]byte("hello stdout: " + c.Value))
+	ctx.Stderr.Write([]byte("hello stderr: " + c.Value))
+	return nil
+}
+
+var usage = `usage: cmd-name [options]
+purpose: cmd-purpose
+
+options:
+--opt (= "")
+    opt-doc
+
+cmd-doc
+`
+
+type ContextSuite struct{}
+
+func (s *CommandSuite) TestPrintUsageBuf(c *C) {
+	buf := bytes.NewBuffer([]byte{})
+	getContext().PrintUsage(&CtxCommand{}, buf)
+	c.Assert(buf.String(), Equals, usage)
+}
+
+func (s *CommandSuite) TestPrintUsageNil(c *C) {
+	ctx := getContext()
+	ctx.PrintUsage(&CtxCommand{}, nil)
+	c.Assert(ctx.Stderr.(*bytes.Buffer).String(), Equals, usage)
+}
+
+func (s *CommandSuite) TestParseIsSilent(c *C) {
+	ctx := getContext()
+	err := ctx.Parse(&CtxCommand{}, []string{"--unknown"})
+	c.Assert(err, ErrorMatches, "flag provided but not defined: --unknown")
+	c.Assert(str(ctx.Stdout), Equals, "")
+	c.Assert(str(ctx.Stderr), Equals, "")
+}
+
+func (s *CommandSuite) TestMainIsNotSilent(c *C) {
+	ctx := getContext()
+	result := ctx.Main(&CtxCommand{}, []string{"--unknown"})
+	c.Assert(result, Equals, 2)
+	c.Assert(str(ctx.Stdout), Equals, "")
+	expected := "flag provided but not defined: --unknown\n" + usage
+	c.Assert(str(ctx.Stderr), Equals, expected)
+}
+
+func (s *CommandSuite) TestParseSuccess(c *C) {
+	ctx := getContext()
+	err := ctx.Parse(&CtxCommand{}, []string{})
+	c.Assert(err, IsNil)
+	c.Assert(str(ctx.Stdout), Equals, "")
+	c.Assert(str(ctx.Stderr), Equals, "")
+}
+
+func (s *CommandSuite) TestMainBadRun(c *C) {
+	ctx := getContext()
+	result := ctx.Main(&CtxCommand{}, []string{"--opt", "error"})
+	c.Assert(result, Equals, 1)
+	c.Assert(str(ctx.Stdout), Equals, "")
+	c.Assert(str(ctx.Stderr), Equals, "oh noes!\n")
+}
+
+func (s *CommandSuite) TestMainSuccess(c *C) {
+	ctx := getContext()
+	result := ctx.Main(&CtxCommand{}, []string{"--opt", "success!"})
+	c.Assert(result, Equals, 0)
+	c.Assert(str(ctx.Stdout), Equals, "hello stdout: success!")
+	c.Assert(str(ctx.Stderr), Equals, "hello stderr: success!")
+}
+
+func AssertInitLog(c *C, verbose bool, debug bool, logfile string, logre string) {
+	ctx := getContext()
+	err := ctx.InitLog(verbose, debug, logfile)
+	c.Assert(err, IsNil)
+	ctx.Log.Printf("hello log")
+	ctx.Log.Debugf("hello debug")
+	c.Assert(str(ctx.Stdout), Equals, "")
+
+	var log string
+	if logfile == "" {
+		log = str(ctx.Stderr)
+	} else {
+		c.Assert(str(ctx.Stderr), Equals, "")
+		raw, err := ioutil.ReadFile(logfile)
+		c.Assert(err, IsNil)
+		log = string(raw)
+	}
+	c.Assert(log, Matches, logre)
+}
+
+func (s *CommandSuite) TestInitLog(c *C) {
+	printfre := ".* JUJU hello log\n"
+	debugfre := ".* JUJU:DEBUG hello debug\n"
+
+	AssertInitLog(c, false, false, "", "")
+	AssertInitLog(c, true, false, "", printfre)
+	AssertInitLog(c, false, true, "", printfre+debugfre)
+	AssertInitLog(c, true, true, "", printfre+debugfre)
+
+	tmp := c.MkDir()
+	AssertInitLog(c, false, false, filepath.Join(tmp, "1.log"), printfre)
+	AssertInitLog(c, true, false, filepath.Join(tmp, "2.log"), printfre)
+	AssertInitLog(c, false, true, filepath.Join(tmp, "3.log"), printfre+debugfre)
+	AssertInitLog(c, true, true, filepath.Join(tmp, "4.log"), printfre+debugfre)
+}
 
=== modified file 'cmd/juju/bootstrap.go'
--- cmd/juju/bootstrap.go	2012-02-09 11:35:24 +0000
+++ cmd/juju/bootstrap.go	2012-03-02 16:10:26 +0000
@@ -30,13 +30,13 @@
 
 // ParsePositional checks that no unexpected extra command-line arguments have
 // been specified.
-func (c *BootstrapCommand) ParsePositional(args []string) error {
-	return cmd.CheckEmpty(args)
+func (c *BootstrapCommand) ParsePositional(args []string) ([]string, error) {
+	return nil, 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(ctx *cmd.Context) error {
 	conn, err := juju.NewConn(c.Environment)
 	if err != nil {
 		return err
 
=== modified file 'cmd/juju/main.go'
--- cmd/juju/main.go	2012-02-14 16:42:42 +0000
+++ cmd/juju/main.go	2012-03-02 16:10:26 +0000
@@ -18,7 +18,7 @@
 func Main(args []string) {
 	jc := cmd.NewSuperCommand("juju", jujuDoc)
 	jc.Register(&BootstrapCommand{})
-	cmd.Main(jc, args)
+	cmd.Main(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-03-02 16:10:26 +0000
@@ -36,15 +36,15 @@
 
 // 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(args []string) ([]string, error) {
 	if c.jujuDir == "" {
-		return requiredError("juju-directory")
+		return nil, requiredError("juju-directory")
 	}
 	if c.stateInfo.Addrs == nil {
-		return requiredError("zookeeper-servers")
+		return nil, requiredError("zookeeper-servers")
 	}
 	if c.sessionFile == "" {
-		return requiredError("session-file")
+		return nil, requiredError("session-file")
 	}
-	return cmd.CheckEmpty(args)
+	return nil, cmd.CheckEmpty(args)
 }
 
=== modified file 'cmd/jujud/initzk.go'
--- cmd/jujud/initzk.go	2012-02-21 15:13:47 +0000
+++ cmd/jujud/initzk.go	2012-03-02 16:10:26 +0000
@@ -32,21 +32,21 @@
 
 // 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(args []string) ([]string, error) {
 	if c.StateInfo.Addrs == nil {
-		return requiredError("zookeeper-servers")
+		return nil, requiredError("zookeeper-servers")
 	}
 	if c.InstanceId == "" {
-		return requiredError("instance-id")
+		return nil, requiredError("instance-id")
 	}
 	if c.EnvType == "" {
-		return requiredError("env-type")
+		return nil, requiredError("env-type")
 	}
-	return cmd.CheckEmpty(args)
+	return nil, cmd.CheckEmpty(args)
 }
 
 // Run initializes zookeeper state for an environment.
-func (c *InitzkCommand) Run() error {
+func (c *InitzkCommand) Run(ctx *cmd.Context) error {
 	// TODO connect to zookeeper; call State.Initialize
 	return fmt.Errorf("InitzkCommand.Run not implemented")
 }
 
=== modified file 'cmd/jujud/machine.go'
--- cmd/jujud/machine.go	2012-02-15 12:28:26 +0000
+++ cmd/jujud/machine.go	2012-03-02 16:10:26 +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(args []string) ([]string, error) {
 	if a.MachineId < 0 {
-		return fmt.Errorf("--machine-id option must be set, and expects a non-negative integer")
+		return nil, fmt.Errorf("--machine-id option must be set, and expects a non-negative integer")
 	}
 	return a.agentConf.ParsePositional(args)
 }
 
 // Run runs a machine agent.
-func (a *MachineAgent) Run() error {
+func (a *MachineAgent) Run(ctx *cmd.Context) 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-03-02 16:10:26 +0000
@@ -21,7 +21,7 @@
 	jc.Register(NewUnitAgent())
 	jc.Register(NewMachineAgent())
 	jc.Register(NewProvisioningAgent())
-	cmd.Main(jc, args)
+	cmd.Main(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-03-02 16:10:26 +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 {
+func (a *ProvisioningAgent) Run(ctx *cmd.Context) error {
 	return fmt.Errorf("MachineAgent.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-03-02 16:10:26 +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(args []string) ([]string, error) {
 	if a.UnitName == "" {
-		return requiredError("unit-name")
+		return nil, requiredError("unit-name")
 	}
 	if !juju.ValidUnit.MatchString(a.UnitName) {
-		return fmt.Errorf(`--unit-name option expects "<service>/<n>" argument`)
+		return nil, fmt.Errorf(`--unit-name option expects "<service>/<n>" argument`)
 	}
 	return a.agentConf.ParsePositional(args)
 }
 
 // Run runs a unit agent.
-func (a *UnitAgent) Run() error {
+func (a *UnitAgent) Run(ctx *cmd.Context) error {
 	return fmt.Errorf("UnitAgent.Run not implemented")
 }
 
=== modified file 'cmd/supercommand.go'
--- cmd/supercommand.go	2012-03-02 16:10:26 +0000
+++ cmd/supercommand.go	2012-03-02 16:10:26 +0000
@@ -2,11 +2,7 @@
 
 import (
 	"fmt"
-	"io"
 	"launchpad.net/gnuflag"
-	"launchpad.net/juju/go/log"
-	stdlog "log"
-	"os"
 	"sort"
 	"strings"
 )
@@ -99,51 +95,32 @@
 	f.BoolVar(&c.Debug, "debug", c.Debug, "if set, log debugging messages")
 }
 
-// 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 {
+// ParsePositional selects the subcommand specified by subargs and returns the
+// remaining args that it expects to be Parsed again; that is, those that would
+// be passed to the subcommand had the subcommand itself been passed directly
+// to Main. When a subcommand has been selected, c itself takes on its relevant
+// properties, and once reparsing is complete Running c will Run the subcommand.
+func (c *SuperCommand) ParsePositional(subargs []string) ([]string, error) {
 	if c.subcmd != nil {
 		return c.subcmd.ParsePositional(subargs)
 	}
 	if len(subargs) == 0 {
-		return fmt.Errorf("no command specified")
+		return nil, fmt.Errorf("no command specified")
 	}
 	found := false
 	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.Global.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.Global.Target = stdlog.New(target, "", stdlog.LstdFlags)
-	}
-	return nil
+		return nil, fmt.Errorf("unrecognised command: %s", subargs[0])
+	}
+	return subargs[1:], nil
 }
 
 // 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(ctx *Context) error {
+	if err := ctx.InitLog(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(ctx)
 }
 
=== modified file 'cmd/supercommand_test.go'
--- cmd/supercommand_test.go	2012-03-02 16:10:26 +0000
+++ cmd/supercommand_test.go	2012-03-02 16:10:26 +0000
@@ -13,12 +13,12 @@
 
 func Test(t *testing.T) { TestingT(t) }
 
-type TestCommand struct {
+type SubCommand struct {
 	Name  string
 	Value string
 }
 
-func (c *TestCommand) Info() *cmd.Info {
+func (c *SubCommand) Info() *cmd.Info {
 	return &cmd.Info{
 		c.Name, "[options]",
 		fmt.Sprintf("%s the juju", c.Name),
@@ -27,18 +27,18 @@
 	}
 }
 
-func (c *TestCommand) InitFlagSet(f *gnuflag.FlagSet) {
+func (c *SubCommand) InitFlagSet(f *gnuflag.FlagSet) {
 	f.StringVar(&c.Value, "value", "", "doc")
 }
 
-func (c *TestCommand) ParsePositional(args []string) error {
+func (c *SubCommand) ParsePositional(args []string) ([]string, error) {
 	if len(args) != 0 {
-		return fmt.Errorf("BADARGS: %s", args)
+		return nil, fmt.Errorf("BADARGS: %s", args)
 	}
-	return nil
+	return nil, nil
 }
 
-func (c *TestCommand) Run() error {
+func (c *SubCommand) Run(ctx *cmd.Context) error {
 	return fmt.Errorf("BORKEN: value is %s.", c.Value)
 }
 
@@ -48,9 +48,9 @@
 	return jc, err
 }
 
-func parseDefenestrate(args []string) (*cmd.SuperCommand, *TestCommand, error) {
+func parseDefenestrate(args []string) (*cmd.SuperCommand, *SubCommand, error) {
 	jc := cmd.NewSuperCommand("jujutest", "")
-	tc := &TestCommand{Name: "defenestrate"}
+	tc := &SubCommand{Name: "defenestrate"}
 	jc.Register(tc)
 	err := cmd.Parse(jc, args)
 	return jc, tc, err
@@ -83,10 +83,10 @@
 
 func (s *CommandSuite) TestRegister(c *C) {
 	jc := cmd.NewSuperCommand("jujutest", "")
-	jc.Register(&TestCommand{Name: "flip"})
-	jc.Register(&TestCommand{Name: "flap"})
+	jc.Register(&SubCommand{Name: "flip"})
+	jc.Register(&SubCommand{Name: "flap"})
 
-	badCall := func() { jc.Register(&TestCommand{Name: "flap"}) }
+	badCall := func() { jc.Register(&SubCommand{Name: "flap"}) }
 	c.Assert(badCall, PanicMatches, "command already registered: flap")
 
 	cmds := jc.DescribeCommands()
@@ -161,7 +161,7 @@
 	jc, _, err := parseDefenestrate(args)
 	c.Assert(err, IsNil)
 
-	err = jc.Run()
+	err = jc.Run(cmd.NewContext())
 	c.Assert(err, ErrorMatches, "BORKEN: value is cheese.")
 
 	c.Assert(log.Global.Debug, Equals, debug)