add new osutil.IsWritable() and use in AddSnapBinaries()

osutil/stat.go

@@ -22,6 +22,7 @@ package osutil
 import (
 	"os"
 	"os/exec"
+	"syscall"
 )
 
 // FileExists return true if given path can be stat()ed by us. Note that
@@ -76,3 +77,13 @@ func LookPathDefault(name string, defaultPath string) string {
 	}
 	return p
 }
+
+// IsWritable checks if the given file/directory can be written by
+// the current user
+func IsWritable(path string) bool {
+	// from "fcntl.h"
+	const W_OK = 2
+
+	err := syscall.Access(path, W_OK)
+	return err == nil
+}

osutil/stat_test.go

@@ -24,6 +24,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"strings"
 
 	. "gopkg.in/check.v1"
 )
@@ -100,3 +101,45 @@ func (s *StatTestSuite) TestLookPathDefaultReturnsDefaultWhenNotFound(c *C) {
 	lookPath = func(name string) (string, error) { return "", fmt.Errorf("Not found") }
 	c.Assert(LookPathDefault("bar", "/bin/bla"), Equals, "/bin/bla")
 }
+
+func makeTestPath(c *C, path string, mode os.FileMode) string {
+	path = filepath.Join(c.MkDir(), path)
+
+	switch {
+	// request for directory
+	case strings.HasSuffix(path, "/"):
+		err := os.MkdirAll(path, os.FileMode(mode))
+		c.Assert(err, IsNil)
+	default:
+		// request for a file
+		err := ioutil.WriteFile(path, nil, os.FileMode(mode))
+		c.Assert(err, IsNil)
+	}
+
+	return path
+}
+
+func (s *StatTestSuite) TestIsWritableDir(c *C) {
+	for _, t := range []struct {
+		path       string
+		mode       os.FileMode
+		isWritable bool
+	}{
+		{"dir/", 0755, true},
+		{"dir/", 0555, false},
+		{"dir/", 0750, true},
+		{"dir/", 0550, false},
+		{"dir/", 0700, true},
+		{"dir/", 0500, false},
+
+		{"file", 0644, true},
+		{"file", 0444, false},
+		{"file", 0640, true},
+		{"file", 0440, false},
+		{"file", 0600, true},
+		{"file", 0400, false},
+	} {
+		writable := IsWritable(makeTestPath(c, t.path, t.mode))
+		c.Check(writable, Equals, t.isWritable, Commentf("incorrect result for %q (%s), got %v, expected %v", t.path, t.mode, writable, t.isWritable))
+	}
+}

wrappers/binaries.go

@@ -25,7 +25,6 @@ import (
 
 	"github.com/snapcore/snapd/dirs"
 	"github.com/snapcore/snapd/osutil"
-	"github.com/snapcore/snapd/release"
 	"github.com/snapcore/snapd/snap"
 )
 
@@ -45,7 +44,7 @@ func AddSnapBinaries(s *snap.Info) (err error) {
 		return err
 	}
 
-	noCompletion := !osutil.FileExists(dirs.CompletersDir) || !osutil.FileExists(dirs.CompleteSh)
+	noCompletion := !osutil.IsWritable(dirs.CompletersDir) || !osutil.FileExists(dirs.CompletersDir) || !osutil.FileExists(dirs.CompleteSh)
 	for _, app := range s.Apps {
 		if app.IsService() {
 			continue
@@ -60,12 +59,7 @@ func AddSnapBinaries(s *snap.Info) (err error) {
 		}
 		created = append(created, wrapperPath)
 
-		// do not add completion symlinks if:
-		// - we are on "ubuntu-core" devices, in this case the
-		//   /usr/share/bash-completion dir is not writable
-		// - there is no completer dir
-		// - there is no completer for this snap
-		if !release.OnClassic || noCompletion || app.Completer == "" {
+		if noCompletion || app.Completer == "" {
 			continue
 		}
 		// symlink the completion snippet