juju-core

Merge lp:~thumper/juju-core/fix-prepare-chown into lp:~go-bot/juju-core/trunk

  1. fix-prepare-chown
  2. Merge into trunk
Proposed by Tim Penhey
Status: Merged
Approved by: Tim Penhey
Approved revision: no longer in the source branch.
Merged at revision: 2260
Proposed branch: lp:~thumper/juju-core/fix-prepare-chown
Merge into: lp:~go-bot/juju-core/trunk
Diff against target: 54 lines (+0/-23)
1 file modified
environs/configstore/disk.go (+0/-23)
To merge this branch: bzr merge lp:~thumper/juju-core/fix-prepare-chown
Reviewer Review Type Date Requested Status
Juju Engineering Pending
Review via email: mp+203254@code.launchpad.net

Commit message

Remove chown from environ.configstore

The chown code was there primarily for the local provider.
This is no longer needed (and had a slight bug where it wasn't
actually checking to see if the current user was root before
trying to chown the files and dirs).

The code has simply been removed. I have tested this with
another user I have on my laptop and the local provider bootstraps
fine.

https://codereview.appspot.com/51150044/

Description of the change

Remove chown from environ.configstore

The chown code was there primarily for the local provider.
This is no longer needed (and had a slight bug where it wasn't
actually checking to see if the current user was root before
trying to chown the files and dirs).

The code has simply been removed. I have tested this with
another user I have on my laptop and the local provider bootstraps
fine.

https://codereview.appspot.com/51150044/

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

Reviewers: mp+203254_code.launchpad.net,

Message:
Please take a look.

Description:
Remove chown from environ.configstore

The chown code was there primarily for the local provider.
This is no longer needed (and had a slight bug where it wasn't
actually checking to see if the current user was root before
trying to chown the files and dirs).

The code has simply been removed. I have tested this with
another user I have on my laptop and the local provider bootstraps
fine.

https://code.launchpad.net/~thumper/juju-core/fix-prepare-chown/+merge/203254

(do not edit description out of merge proposal)

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

Affected files (+2, -23 lines):
   A [revision details]
   M environs/configstore/disk.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: tarmac-20140125125332-eckpnh62bygibitl
+New revision: <email address hidden>

Index: environs/configstore/disk.go
=== modified file 'environs/configstore/disk.go'
--- environs/configstore/disk.go 2014-01-22 22:48:54 +0000
+++ environs/configstore/disk.go 2014-01-26 19:19:03 +0000
@@ -58,20 +58,6 @@
   return filepath.Join(d.dir, "environments", envName+".jenv")
  }

-func ensurePathOwnedByUser(path string) error {
- uid, gid, err := utils.SudoCallerIds()
- if err != nil {
- return err
- }
- if uid != 0 {
- logger.Debugf("Making %v owned by %d:%d", path, uid, gid)
- if err := os.Chown(path, uid, gid); err != nil {
- return err
- }
- }
- return nil
-}
-
  func (d *diskStore) mkEnvironmentsDir() error {
   path := filepath.Join(d.dir, "environments")
   logger.Debugf("Making %v", path)
@@ -79,9 +65,6 @@
   if os.IsExist(err) {
    return nil
   }
- if err == nil {
- err = ensurePathOwnedByUser(path)
- }
   return err
  }

@@ -101,9 +84,6 @@
    return nil, err
   }
   file.Close()
- if err := ensurePathOwnedByUser(path); err != nil {
- return nil, err
- }
   return &environInfo{
    created: true,
    path: path,
@@ -203,9 +183,6 @@
    os.Remove(tmpFile.Name())
    return fmt.Errorf("cannot rename new environment info file: %v", err)
   }
- if err := ensurePathOwnedByUser(info.path); err != nil {
- return err
- }
   info.initialized = true
   return nil
  }

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

LGTM

https://codereview.appspot.com/51150044/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'environs/configstore/disk.go'
--- environs/configstore/disk.go 2014-01-22 22:48:54 +0000
+++ environs/configstore/disk.go 2014-01-26 19:37:25 +0000
@@ -58,20 +58,6 @@
58 return filepath.Join(d.dir, "environments", envName+".jenv")58 return filepath.Join(d.dir, "environments", envName+".jenv")
59}59}
6060
61func ensurePathOwnedByUser(path string) error {
62 uid, gid, err := utils.SudoCallerIds()
63 if err != nil {
64 return err
65 }
66 if uid != 0 {
67 logger.Debugf("Making %v owned by %d:%d", path, uid, gid)
68 if err := os.Chown(path, uid, gid); err != nil {
69 return err
70 }
71 }
72 return nil
73}
74
75func (d *diskStore) mkEnvironmentsDir() error {61func (d *diskStore) mkEnvironmentsDir() error {
76 path := filepath.Join(d.dir, "environments")62 path := filepath.Join(d.dir, "environments")
77 logger.Debugf("Making %v", path)63 logger.Debugf("Making %v", path)
@@ -79,9 +65,6 @@
79 if os.IsExist(err) {65 if os.IsExist(err) {
80 return nil66 return nil
81 }67 }
82 if err == nil {
83 err = ensurePathOwnedByUser(path)
84 }
85 return err68 return err
86}69}
8770
@@ -101,9 +84,6 @@
101 return nil, err84 return nil, err
102 }85 }
103 file.Close()86 file.Close()
104 if err := ensurePathOwnedByUser(path); err != nil {
105 return nil, err
106 }
107 return &environInfo{87 return &environInfo{
108 created: true,88 created: true,
109 path: path,89 path: path,
@@ -203,9 +183,6 @@
203 os.Remove(tmpFile.Name())183 os.Remove(tmpFile.Name())
204 return fmt.Errorf("cannot rename new environment info file: %v", err)184 return fmt.Errorf("cannot rename new environment info file: %v", err)
205 }185 }
206 if err := ensurePathOwnedByUser(info.path); err != nil {
207 return err
208 }
209 info.initialized = true186 info.initialized = true
210 return nil187 return nil
211}188}

Subscribers

People subscribed via source and target branches

to status/vote changes: