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
1=== modified file 'cmd/juju/init_test.go'
2--- cmd/juju/init_test.go 2013-03-12 23:34:53 +0000
3+++ cmd/juju/init_test.go 2013-03-14 04:55:24 +0000
4@@ -15,7 +15,7 @@
5 var _ = Suite(&InitSuite{})
6
7 func (*InitSuite) TestBoilerPlateEnvironment(c *C) {
8- defer testing.MakeFakeHomeNoEnvironments(c, "empty").Restore()
9+ defer testing.MakeEmptyFakeHome(c).Restore()
10 // run without an environments.yaml
11 ctx := testing.Context(c)
12 code := cmd.Main(&InitCommand{}, ctx, []string{"-w"})
13
14=== modified file 'environs/config.go'
15--- environs/config.go 2013-02-04 01:20:55 +0000
16+++ environs/config.go 2013-03-14 04:55:24 +0000
17@@ -153,6 +153,9 @@
18 if err != nil {
19 return "", err
20 }
21+ if err := os.MkdirAll(filepath.Dir(environsFilepath), 0755); err != nil {
22+ return "", err
23+ }
24 if err := ioutil.WriteFile(environsFilepath, []byte(fileContents), 0666); err != nil {
25 return "", err
26 }
27
28=== modified file 'environs/config_test.go'
29--- environs/config_test.go 2013-03-12 23:02:49 +0000
30+++ environs/config_test.go 2013-03-14 04:55:24 +0000
31@@ -141,7 +141,7 @@
32 }
33
34 func (suite) TestDefaultConfigFile(c *C) {
35- defer testing.MakeFakeHomeNoEnvironments(c, "only").Restore()
36+ defer testing.MakeEmptyFakeHome(c).Restore()
37
38 env := `
39 environments:
40
41=== modified file 'testing/environ.go'
42--- testing/environ.go 2013-03-12 23:02:49 +0000
43+++ testing/environ.go 2013-03-14 04:55:24 +0000
44@@ -50,8 +50,7 @@
45 // 'certNames' specified, and the id_rsa.pub file is written to to the .ssh
46 // dir.
47 func MakeFakeHomeNoEnvironments(c *C, certNames ...string) FakeHome {
48- oldHome := os.Getenv("HOME")
49- os.Setenv("HOME", c.MkDir())
50+ fake := MakeEmptyFakeHome(c)
51
52 err := os.Mkdir(HomePath(".juju"), 0755)
53 c.Assert(err, IsNil)
54@@ -68,7 +67,7 @@
55 err = ioutil.WriteFile(HomePath(".ssh", "id_rsa.pub"), []byte("auth key\n"), 0666)
56 c.Assert(err, IsNil)
57
58- return FakeHome(oldHome)
59+ return fake
60 }
61
62 // MakeFakeHome creates a new temporary directory through the test checker,
63@@ -88,6 +87,13 @@
64 return fake
65 }
66
67+func MakeEmptyFakeHome(c *C) FakeHome {
68+ oldHome := os.Getenv("HOME")
69+ os.Setenv("HOME", c.MkDir())
70+
71+ return FakeHome(oldHome)
72+}
73+
74 func HomePath(names ...string) string {
75 all := append([]string{os.Getenv("HOME")}, names...)
76 return filepath.Join(all...)

Subscribers

People subscribed via source and target branches