juju-core

Merge lp:~jameinel/juju-core/init-1147771 into lp:~juju/juju-core/trunk

  1. init-1147771
  2. Merge into trunk
Proposed by John A Meinel
Status: Merged
Merged at revision: 1002
Proposed branch: lp:~jameinel/juju-core/init-1147771
Merge into: lp:~juju/juju-core/trunk
Diff against target: 76 lines (+14/-5)
4 files modified
cmd/juju/init_test.go (+1/-1)
environs/config.go (+3/-0)
environs/config_test.go (+1/-1)
testing/environ.go (+9/-3)
To merge this branch: bzr merge lp:~jameinel/juju-core/init-1147771
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+153283@code.launchpad.net

Description of the change

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://codereview.appspot.com/7759044/

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :
Download full text (3.4 KiB)

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 @@
...

Read more...

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

LGTM

https://codereview.appspot.com/7759044/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

LGTM

https://codereview.appspot.com/7759044/

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

*** Submitted:

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.

R=dfc, dimitern, rog
CC=
https://codereview.appspot.com/7759044

https://codereview.appspot.com/7759044/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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:55:24 +0000
@@ -15,7 +15,7 @@
15var _ = Suite(&InitSuite{})15var _ = Suite(&InitSuite{})
1616
17func (*InitSuite) TestBoilerPlateEnvironment(c *C) {17func (*InitSuite) TestBoilerPlateEnvironment(c *C) {
18 defer testing.MakeFakeHomeNoEnvironments(c, "empty").Restore()18 defer testing.MakeEmptyFakeHome(c).Restore()
19 // run without an environments.yaml19 // run without an environments.yaml
20 ctx := testing.Context(c)20 ctx := testing.Context(c)
21 code := cmd.Main(&InitCommand{}, ctx, []string{"-w"})21 code := cmd.Main(&InitCommand{}, ctx, []string{"-w"})
2222
=== modified file 'environs/config.go'
--- environs/config.go 2013-02-04 01:20:55 +0000
+++ environs/config.go 2013-03-14 04:55:24 +0000
@@ -153,6 +153,9 @@
153 if err != nil {153 if err != nil {
154 return "", err154 return "", err
155 }155 }
156 if err := os.MkdirAll(filepath.Dir(environsFilepath), 0755); err != nil {
157 return "", err
158 }
156 if err := ioutil.WriteFile(environsFilepath, []byte(fileContents), 0666); err != nil {159 if err := ioutil.WriteFile(environsFilepath, []byte(fileContents), 0666); err != nil {
157 return "", err160 return "", err
158 }161 }
159162
=== 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:55:24 +0000
@@ -141,7 +141,7 @@
141}141}
142142
143func (suite) TestDefaultConfigFile(c *C) {143func (suite) TestDefaultConfigFile(c *C) {
144 defer testing.MakeFakeHomeNoEnvironments(c, "only").Restore()144 defer testing.MakeEmptyFakeHome(c).Restore()
145145
146 env := `146 env := `
147environments:147environments:
148148
=== modified file 'testing/environ.go'
--- testing/environ.go 2013-03-12 23:02:49 +0000
+++ testing/environ.go 2013-03-14 04:55:24 +0000
@@ -50,8 +50,7 @@
50// 'certNames' specified, and the id_rsa.pub file is written to to the .ssh50// 'certNames' specified, and the id_rsa.pub file is written to to the .ssh
51// dir.51// dir.
52func MakeFakeHomeNoEnvironments(c *C, certNames ...string) FakeHome {52func MakeFakeHomeNoEnvironments(c *C, certNames ...string) FakeHome {
53 oldHome := os.Getenv("HOME")53 fake := MakeEmptyFakeHome(c)
54 os.Setenv("HOME", c.MkDir())
5554
56 err := os.Mkdir(HomePath(".juju"), 0755)55 err := os.Mkdir(HomePath(".juju"), 0755)
57 c.Assert(err, IsNil)56 c.Assert(err, IsNil)
@@ -68,7 +67,7 @@
68 err = ioutil.WriteFile(HomePath(".ssh", "id_rsa.pub"), []byte("auth key\n"), 0666)67 err = ioutil.WriteFile(HomePath(".ssh", "id_rsa.pub"), []byte("auth key\n"), 0666)
69 c.Assert(err, IsNil)68 c.Assert(err, IsNil)
7069
71 return FakeHome(oldHome)70 return fake
72}71}
7372
74// MakeFakeHome creates a new temporary directory through the test checker,73// MakeFakeHome creates a new temporary directory through the test checker,
@@ -88,6 +87,13 @@
88 return fake87 return fake
89}88}
9089
90func MakeEmptyFakeHome(c *C) FakeHome {
91 oldHome := os.Getenv("HOME")
92 os.Setenv("HOME", c.MkDir())
93
94 return FakeHome(oldHome)
95}
96
91func HomePath(names ...string) string {97func HomePath(names ...string) string {
92 all := append([]string{os.Getenv("HOME")}, names...)98 all := append([]string{os.Getenv("HOME")}, names...)
93 return filepath.Join(all...)99 return filepath.Join(all...)

Subscribers

People subscribed via source and target branches