Merge pull request #1256 from ipfs/fix/init

clean up and fix init permissions handling

cmd/ipfs/init.go

@@ -5,6 +5,8 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"os"
+	"path"
 
 	context "github.com/ipfs/go-ipfs/Godeps/_workspace/src/golang.org/x/net/context"
 	assets "github.com/ipfs/go-ipfs/assets"
@@ -36,7 +38,10 @@ var initCmd = &cmds.Command{
 		// TODO cmds.StringOption("event-logs", "l", "Location for machine-readable event logs"),
 	},
 	PreRun: func(req cmds.Request) error {
-		daemonLocked := fsrepo.LockedByOtherProcess(req.Context().ConfigRoot)
+		daemonLocked, err := fsrepo.LockedByOtherProcess(req.Context().ConfigRoot)
+		if err != nil {
+			return err
+		}
 
 		log.Info("checking if daemon is running...")
 		if daemonLocked {
@@ -47,6 +52,10 @@ var initCmd = &cmds.Command{
 		return nil
 	},
 	Run: func(req cmds.Request, res cmds.Response) {
+		if req.Context().Online {
+			res.SetError(errors.New("init must be run offline only!"), cmds.ErrNormal)
+			return
+		}
 
 		force, _, err := req.Option("f").Bool() // if !found, it's okay force == false
 		if err != nil {
@@ -64,15 +73,10 @@ var initCmd = &cmds.Command{
 			nBitsForKeypair = nBitsForKeypairDefault
 		}
 
-		rpipe, wpipe := io.Pipe()
-		go func() {
-			defer wpipe.Close()
-			if err := doInit(wpipe, req.Context().ConfigRoot, force, nBitsForKeypair); err != nil {
-				res.SetError(err, cmds.ErrNormal)
-				return
-			}
-		}()
-		res.SetOutput(rpipe)
+		if err := doInit(os.Stdout, req.Context().ConfigRoot, force, nBitsForKeypair); err != nil {
+			res.SetError(err, cmds.ErrNormal)
+			return
+		}
 	},
 }
 
@@ -82,8 +86,7 @@ Reinitializing would overwrite your keys.
 `)
 
 func initWithDefaults(out io.Writer, repoRoot string) error {
-	err := doInit(out, repoRoot, false, nBitsForKeypairDefault)
-	return err
+	return doInit(out, repoRoot, false, nBitsForKeypairDefault)
 }
 
 func doInit(out io.Writer, repoRoot string, force bool, nBitsForKeypair int) error {
@@ -91,6 +94,10 @@ func doInit(out io.Writer, repoRoot string, force bool, nBitsForKeypair int) err
 		return err
 	}
 
+	if err := checkWriteable(repoRoot); err != nil {
+		return err
+	}
+
 	if fsrepo.IsInitialized(repoRoot) && !force {
 		return errRepoExists
 	}
@@ -117,6 +124,34 @@ func doInit(out io.Writer, repoRoot string, force bool, nBitsForKeypair int) err
 	return initializeIpnsKeyspace(repoRoot)
 }
 
+func checkWriteable(dir string) error {
+	_, err := os.Stat(dir)
+	if err == nil {
+		// dir exists, make sure we can write to it
+		testfile := path.Join(dir, "test")
+		fi, err := os.Create(testfile)
+		if err != nil {
+			if os.IsPermission(err) {
+				return fmt.Errorf("%s is not writeable by the current user", dir)
+			}
+			return fmt.Errorf("unexpected error while checking writeablility of repo root: %s", err)
+		}
+		fi.Close()
+		return os.Remove(testfile)
+	}
+
+	if os.IsNotExist(err) {
+		// dir doesnt exist, check that we can create it
+		return os.Mkdir(dir, 0775)
+	}
+
+	if os.IsPermission(err) {
+		return fmt.Errorf("cannot write to %s, incorrect permissions", err)
+	}
+
+	return err
+}
+
 func addDefaultAssets(out io.Writer, repoRoot string) error {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()

cmd/ipfs/main.go

@@ -387,7 +387,10 @@ func commandShouldRunOnDaemon(details cmdDetails, req cmds.Request, root *cmds.C
 
 	// at this point need to know whether daemon is running. we defer
 	// to this point so that some commands dont open files unnecessarily.
-	daemonLocked := fsrepo.LockedByOtherProcess(req.Context().ConfigRoot)
+	daemonLocked, err := fsrepo.LockedByOtherProcess(req.Context().ConfigRoot)
+	if err != nil {
+		return false, err
+	}
 
 	if daemonLocked {
 

repo/fsrepo/fsrepo.go

@@ -283,7 +283,7 @@ func Remove(repoPath string) error {
 
 // LockedByOtherProcess returns true if the FSRepo is locked by another
 // process. If true, then the repo cannot be opened by this process.
-func LockedByOtherProcess(repoPath string) bool {
+func LockedByOtherProcess(repoPath string) (bool, error) {
 	repoPath = path.Clean(repoPath)
 
 	// TODO replace this with the "api" file

repo/fsrepo/lock/lock.go

@@ -2,6 +2,7 @@ package lock
 
 import (
 	"io"
+	"os"
 	"path"
 
 	lock "github.com/ipfs/go-ipfs/Godeps/_workspace/src/github.com/camlistore/lock"
@@ -17,14 +18,17 @@ func Lock(confdir string) (io.Closer, error) {
 	return c, err
 }
 
-func Locked(confdir string) bool {
+func Locked(confdir string) (bool, error) {
 	if !util.FileExists(path.Join(confdir, LockFile)) {
-		return false
+		return false, nil
 	}
 	if lk, err := Lock(confdir); err != nil {
-		return true
+		if os.IsPermission(err) {
+			return false, err
+		}
+		return true, nil
 	} else {
 		lk.Close()
-		return false
+		return false, nil
 	}
 }

test/sharness/t0020-init.sh

@@ -8,6 +8,22 @@ test_description="Test init command"
 
 . lib/test-lib.sh
 
+# test that ipfs fails to init if IPFS_PATH isnt writeable
+test_expect_success "create dir and change perms succeeds" '
+	export IPFS_PATH="$(pwd)/.badipfs" &&
+	mkdir "$IPFS_PATH" &&
+	chmod 000 "$IPFS_PATH"
+'
+
+test_expect_success "ipfs init fails" '
+	test_must_fail ipfs init 2> init_fail_out
+'
+
+test_expect_success "ipfs init output looks good" '
+	echo "Error: open $IPFS_PATH/repo.lock: permission denied" > init_fail_exp &&
+	test_cmp init_fail_out init_fail_exp
+'
+
 test_expect_success "ipfs init succeeds" '
 	export IPFS_PATH="$(pwd)/.ipfs" &&
 	BITS="2048" &&
@@ -17,7 +33,8 @@ test_expect_success "ipfs init succeeds" '
 test_expect_success ".ipfs/ has been created" '
 	test -d ".ipfs" &&
 	test -f ".ipfs/config" &&
-	test -d ".ipfs/datastore" ||
+	test -d ".ipfs/datastore" &&
+	test -d ".ipfs/blocks" ||
 	test_fsh ls -al .ipfs
 '
 
@@ -44,6 +61,10 @@ test_expect_success "ipfs init output looks good" '
 	test_cmp expected actual_init
 '
 
+test_expect_success "clean up ipfs dir" '
+	rm -rf "$IPFS_PATH"
+'
+
 test_init_ipfs
 
 test_launch_ipfs_daemon