cmd: No longer panic if cwd is deleted

cmd: No longer panic if cwd is deleted

This adds error return value for cmd.DefaultContext() function:

func DefaultContext() (*Context, error)

The previous implemenation causes panic if cmd.DefaultContext() was called with deleted current directory.

https://code.launchpad.net/~klyachin/juju-core/100-lp-1227074-panic-in-deleted-directory/+merge/212182

(do not edit description out of merge proposal)

[vladislav.klyachin, 2014-03-21 16:33:12]

Please take a look.

[dimitern, 2014-03-21 16:45:59]

LGTM with the suggestions below.

https://codereview.appspot.com/78660045/diff/1/cmd/cmd_test.go
File cmd/cmd_test.go (right):

https://codereview.appspot.com/78660045/diff/1/cmd/cmd_test.go#newcode128
cmd/cmd_test.go:128: c.Assert(err, gc.NotNil)
Please, check the actual error message. You can use gc.ErrorMatches for a
regular expression match against the err.Error() message.

Like this:
c.Assert(err, gc.ErrorMatches, `error: getwd: <whatever it reports>`)

https://codereview.appspot.com/78660045/diff/1/cmd/juju/cmd_test.go
File cmd/juju/cmd_test.go (right):

https://codereview.appspot.com/78660045/diff/1/cmd/juju/cmd_test.go#newcode126
cmd/juju/cmd_test.go:126: ctx, _ := cmd.DefaultContext()
It's bad to ignore errors like this, how about:

ctx, err := cmd.DefaultContext()
c.Assert(err, gc.IsNil)

and take c *gc.C as an argument.

https://codereview.appspot.com/78660045/diff/1/cmd/juju/main.go
File cmd/juju/main.go (right):

https://codereview.appspot.com/78660045/diff/1/cmd/juju/main.go#newcode41
cmd/juju/main.go:41: if err = juju.InitJujuHome(); err != nil {
I'd prefer s/if err =/if err :=/ here.

[dave_cheney.net, 2014-03-23 23:29:35]

Why is all this necessary. Why does cmd.DefaultContext() need the CWD ? What
happens if it isn't available, why not just default to "/", then you don't need
to return an error at all

[fwereade, 2014-03-24 10:23:42]

On 2014/03/23 23:29:35, dfc wrote:
> Why is all this necessary. Why does cmd.DefaultContext() need the CWD ? What
> happens if it isn't available, why not just default to "/", then you don't
need
> to return an error at all

Absolute paths are calculated relative to the cmd.Context, and should be tested
as such, lest we end up with scattered code fragments that use the "real"
working directory. We don't want a different mode in which paths are silently
calculated from a surprising place: we want to fail in a way that makes sense.

It *might* be better to fix Context.AbsPath() but I'm not sure that's the *only*
place that uses the Context's WD -- would you check please? I have a nasty
feeling it's a struct and anyone can look at that field directly, in which case
it might be smartest to put it behind an interface.

I think it's best if we can make sure WD access goes through a single
bottleneck, but defer erroring out until someone actually tries to use the WD.

[fwereade, 2014-03-24 10:23:54]

https://codereview.appspot.com/78660045/diff/1/cmd/cmd_test.go
File cmd/cmd_test.go (right):

https://codereview.appspot.com/78660045/diff/1/cmd/cmd_test.go#newcode128
cmd/cmd_test.go:128: c.Assert(err, gc.NotNil)
On 2014/03/21 16:46:00, dimitern wrote:
> Please, check the actual error message. You can use gc.ErrorMatches for a
> regular expression match against the err.Error() message.
> 
> Like this:
> c.Assert(err, gc.ErrorMatches, `error: getwd: <whatever it reports>`)

+100

[vladislav.klyachin, 2014-03-24 10:44:52]

Please take a look.

[vladislav.klyachin, 2014-03-24 11:12:58]

On 2014/03/21 16:45:59, dimitern wrote:
> LGTM with the suggestions below.
> 
> https://codereview.appspot.com/78660045/diff/1/cmd/cmd_test.go
> File cmd/cmd_test.go (right):
> 
> https://codereview.appspot.com/78660045/diff/1/cmd/cmd_test.go#newcode128
> cmd/cmd_test.go:128: c.Assert(err, gc.NotNil)
> Please, check the actual error message. You can use gc.ErrorMatches for a
> regular expression match against the err.Error() message.
> 
> Like this:
> c.Assert(err, gc.ErrorMatches, `error: getwd: <whatever it reports>`)
Done
> 
> https://codereview.appspot.com/78660045/diff/1/cmd/juju/cmd_test.go
> File cmd/juju/cmd_test.go (right):
> 
> https://codereview.appspot.com/78660045/diff/1/cmd/juju/cmd_test.go#newcode126
> cmd/juju/cmd_test.go:126: ctx, _ := cmd.DefaultContext()
> It's bad to ignore errors like this, how about:
> 
> ctx, err := cmd.DefaultContext()
> c.Assert(err, gc.IsNil)
> 
> and take c *gc.C as an argument.
Done
> 
> https://codereview.appspot.com/78660045/diff/1/cmd/juju/main.go
> File cmd/juju/main.go (right):
> 
> https://codereview.appspot.com/78660045/diff/1/cmd/juju/main.go#newcode41
> cmd/juju/main.go:41: if err = juju.InitJujuHome(); err != nil {
> I'd prefer s/if err =/if err :=/ here.
Done