Code review comment for lp:~jameinel/juju-core/init-1147771

Revision history for this message
John A Meinel (jameinel) wrote :

Reviewers: mp+153283_code.launchpad.net,

Message:
Please take a look.

Description:
environs/config: ensure ~/.juju exists

When doing WriteEnviron we would create ~/.juju/environments.yaml
but we weren't creating the containing dir. (Which meant actually
running 'juju init -w' wouldn't work for new users.)

This adds an unconditional MkdirAll() before writing environments.yaml,
but that should be ok, as it is a no-op if the dir already exists.

https://code.launchpad.net/~jameinel/juju-core/init-1147771/+merge/153283

(do not edit description out of merge proposal)

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

Affected files:
   A [revision details]
   M cmd/juju/init_test.go
   M environs/config.go
   M environs/config_test.go
   M testing/environ.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: environs/config.go
=== modified file 'environs/config.go'
--- environs/config.go 2013-02-04 01:20:55 +0000
+++ environs/config.go 2013-03-14 04:51:54 +0000
@@ -153,6 +153,9 @@
   if err != nil {
    return "", err
   }
+ if err := os.MkdirAll(filepath.Dir(environsFilepath), 0755); err != nil {
+ return "", err
+ }
   if err := ioutil.WriteFile(environsFilepath, []byte(fileContents), 0666);
err != nil {
    return "", err
   }

Index: environs/config_test.go
=== modified file 'environs/config_test.go'
--- environs/config_test.go 2013-03-12 23:02:49 +0000
+++ environs/config_test.go 2013-03-14 04:51:54 +0000
@@ -141,7 +141,7 @@
  }

  func (suite) TestDefaultConfigFile(c *C) {
- defer testing.MakeFakeHomeNoEnvironments(c, "only").Restore()
+ defer testing.MakeEmptyFakeHome(c).Restore()

   env := `
  environments:

Index: testing/environ.go
=== modified file 'testing/environ.go'
--- testing/environ.go 2013-03-12 23:02:49 +0000
+++ testing/environ.go 2013-03-14 04:51:54 +0000
@@ -50,8 +50,7 @@
  // 'certNames' specified, and the id_rsa.pub file is written to to the .ssh
  // dir.
  func MakeFakeHomeNoEnvironments(c *C, certNames ...string) FakeHome {
- oldHome := os.Getenv("HOME")
- os.Setenv("HOME", c.MkDir())
+ fake := MakeEmptyFakeHome(c)

   err := os.Mkdir(HomePath(".juju"), 0755)
   c.Assert(err, IsNil)
@@ -68,7 +67,7 @@
   err = ioutil.WriteFile(HomePath(".ssh", "id_rsa.pub"), []byte("auth
key\n"), 0666)
   c.Assert(err, IsNil)

- return FakeHome(oldHome)
+ return fake
  }

  // MakeFakeHome creates a new temporary directory through the test checker,
@@ -88,6 +87,13 @@
   return fake
  }

+func MakeEmptyFakeHome(c *C) FakeHome {
+ oldHome := os.Getenv("HOME")
+ os.Setenv("HOME", c.MkDir())
+
+ return FakeHome(oldHome)
+}
+
  func HomePath(names ...string) string {
   all := append([]string{os.Getenv("HOME")}, names...)
   return filepath.Join(all...)

Index: cmd/juju/init_test.go
=== modified file 'cmd/juju/init_test.go'
--- cmd/juju/init_test.go 2013-03-12 23:34:53 +0000
+++ cmd/juju/init_test.go 2013-03-14 04:51:54 +0000
@@ -15,7 +15,7 @@
  var _ = Suite(&InitSuite{})

  func (*InitSuite) TestBoilerPlateEnvironment(c *C) {
- defer testing.MakeFakeHomeNoEnvironments(c, "empty").Restore()
+ defer testing.MakeEmptyFakeHome(c).Restore()
   // run without an environments.yaml
   ctx := testing.Context(c)
   code := cmd.Main(&InitCommand{}, ctx, []string{"-w"})

« Back to merge proposal