Update locking, ensure GooGetRoot permissions are setup

go.mod

@@ -3,27 +3,27 @@ module github.com/google/googet/v2
 go 1.12
 
 require (
-	cloud.google.com/go v0.39.0
-	github.com/StackExchange/wmi v0.0.0-20181212234831-e0a55b97c705
+	cloud.google.com/go v0.49.0 // indirect
+	cloud.google.com/go/storage v1.4.0
+	github.com/StackExchange/wmi v0.0.0-20190523213315-cbe66965904d
 	github.com/blang/semver v3.5.1+incompatible
 	github.com/dustin/go-humanize v1.0.0
 	github.com/go-ole/go-ole v1.2.4 // indirect
 	github.com/go-yaml/yaml v2.1.0+incompatible
-	github.com/golang/protobuf v1.3.1 // indirect
-	github.com/google/go-cmp v0.3.0 // indirect
+	github.com/golang/groupcache v0.0.0-20191027212112-611e8accdfc9 // indirect
 	github.com/google/logger v1.0.1
 	github.com/google/subcommands v1.0.1
-	github.com/hashicorp/golang-lru v0.5.1 // indirect
-	github.com/kr/pretty v0.1.0 // indirect
-	github.com/mattn/go-runewidth v0.0.4 // indirect
-	github.com/olekukonko/tablewriter v0.0.1
-	golang.org/x/net v0.0.0-20190509222800-a4d6f7feada5 // indirect
-	golang.org/x/oauth2 v0.0.0-20190402181905-9f3314589c9a // indirect
-	golang.org/x/sys v0.0.0-20190509141414-a5b02f93d862
-	golang.org/x/text v0.3.2 // indirect
-	google.golang.org/api v0.5.0
-	google.golang.org/appengine v1.5.0 // indirect
-	google.golang.org/grpc v1.20.1 // indirect
-	gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127 // indirect
-	gopkg.in/yaml.v2 v2.2.2 // indirect
+	github.com/jstemmer/go-junit-report v0.9.1 // indirect
+	github.com/olekukonko/tablewriter v0.0.4
+	go.opencensus.io v0.22.2 // indirect
+	golang.org/x/exp v0.0.0-20191129062945-2f5052295587 // indirect
+	golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f // indirect
+	golang.org/x/net v0.0.0-20191206103017-1ddd1de85cb0 // indirect
+	golang.org/x/oauth2 v0.0.0-20191202225959-858c2ad4c8b6 // indirect
+	golang.org/x/sys v0.0.0-20191204072324-ce4227a45e2e
+	golang.org/x/tools v0.0.0-20191205225056-3393d29bb9fe // indirect
+	google.golang.org/api v0.14.0
+	google.golang.org/appengine v1.6.5 // indirect
+	google.golang.org/genproto v0.0.0-20191205163323-51378566eb59 // indirect
+	google.golang.org/grpc v1.25.1 // indirect
 )

googet.go

@@ -41,7 +41,6 @@ const (
 	stateFile = "googet.state"
 	confFile  = "googet.conf"
 	logFile   = "googet.log"
-	lockFile  = "googet.lock"
 	cacheDir  = "cache"
 	repoDir   = "repos"
 	envVar    = "GooGetRoot"
@@ -357,26 +356,6 @@ func rotateLog(logPath string, ls int64) error {
 	return nil
 }
 
-func lock(lf string) (*os.File, error) {
-	// This locking process only works on Windows, on linux os.Remove will remove an open file.
-	// This is not currently an issue as running googet on linux is only done for testing.
-	// In the future using a semaphore for locking would be nice.
-	// 90% of all GooGet runs happen in < 60s, we wait 70s.
-	for i := 1; i < 15; i++ {
-		// Try to remove any old lock file that may exist, ignore errors as we don't care if
-		// we can't remove it or it does not exist.
-		os.Remove(lf)
-		if lk, err := os.OpenFile(lf, os.O_RDONLY|os.O_CREATE|os.O_EXCL, 0); err == nil {
-			return lk, nil
-		}
-		if i == 1 {
-			fmt.Fprintln(os.Stdout, "GooGet lock already held, waiting...")
-		}
-		time.Sleep(5 * time.Second)
-	}
-	return nil, errors.New("timed out waiting for lock")
-}
-
 func readConf(cf string) {
 	gc, err := unmarshalConfFile(cf)
 	if err != nil {
@@ -410,7 +389,47 @@ func readConf(cf string) {
 	allowUnsafeURL = gc.AllowUnsafeURL
 }
 
-func run() int {
+var deferredFuncs []func()
+
+func runDeferredFuncs() {
+	for _, f := range deferredFuncs {
+		f()
+	}
+}
+
+func obtainLock() error {
+	err := os.MkdirAll(filepath.Dir(lockFile), 0755)
+	if err != nil && !os.IsExist(err) {
+		return err
+	}
+
+	f, err := os.OpenFile(lockFile, os.O_RDWR|os.O_CREATE, 0600)
+	if err != nil && !os.IsExist(err) {
+		return err
+	}
+
+	c := make(chan error)
+	go func() {
+		c <- lock(f)
+	}()
+
+	ticker := time.NewTicker(5 * time.Second)
+	// 90% of all GooGet runs happen in < 60s, we wait 70s.
+	for i := 1; i < 15; i++ {
+		select {
+		case err := <-c:
+			if err != nil {
+				return err
+			}
+			return nil
+		case <-ticker.C:
+			fmt.Fprintln(os.Stdout, "GooGet lock already held, waiting...")
+		}
+	}
+	return errors.New("timed out waiting for lock")
+}
+
+func main() {
 	ggFlags := flag.NewFlagSet(filepath.Base(os.Args[0]), flag.ContinueOnError)
 	ggFlags.StringVar(&rootDir, "root", os.Getenv(envVar), "googet root directory")
 	ggFlags.BoolVar(&noConfirm, "noconfirm", false, "skip confirmation")
@@ -449,7 +468,7 @@ func run() int {
 
 	nonLockingCommands := []string{"help", "commands", "flags"}
 	if ggFlags.NArg() == 0 || goolib.ContainsString(ggFlags.Args()[0], nonLockingCommands) {
-		return int(cmdr.Execute(context.Background()))
+		os.Exit(int(cmdr.Execute(context.Background())))
 	}
 
 	if rootDir == "" {
@@ -459,38 +478,34 @@ func run() int {
 		logger.Fatalln("Error setting up root directory:", err)
 	}
 
-	readConf(filepath.Join(rootDir, confFile))
-
-	lkf := filepath.Join(rootDir, lockFile)
-	lk, err := lock(lkf)
-	if err != nil {
-		logger.Fatal(err)
+	if err := obtainLock(); err != nil {
+		logger.Fatalf("Cannot obtain GooGet lock, error: %v", err)
 	}
-	defer os.Remove(lkf)
-	defer lk.Close()
+	readConf(filepath.Join(rootDir, confFile))
 
 	logPath := filepath.Join(rootDir, logFile)
 	if err := rotateLog(logPath, logSize); err != nil {
 		logger.Error(err)
 	}
 	lf, err := os.OpenFile(logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0660)
 	if err != nil {
+		runDeferredFuncs()
 		logger.Fatalln("Failed to open log file:", err)
 	}
-	defer lf.Close()
+	deferredFuncs = append(deferredFuncs, func() { lf.Close() })
 
 	logger.Init("GooGet", verbose, systemLog, lf)
 
 	if err := os.MkdirAll(filepath.Join(rootDir, cacheDir), 0774); err != nil {
+		runDeferredFuncs()
 		logger.Fatalf("Error setting up cache directory: %v", err)
 	}
 	if err := os.MkdirAll(filepath.Join(rootDir, repoDir), 0774); err != nil {
+		runDeferredFuncs()
 		logger.Fatalf("Error setting up repo directory: %v", err)
 	}
 
-	return int(cmdr.Execute(context.Background()))
-}
-
-func main() {
-	os.Exit(run())
+	es := cmdr.Execute(context.Background())
+	runDeferredFuncs()
+	os.Exit(int(es))
 }

googet.goospec

@@ -1,4 +1,4 @@
-{{$version := "2.16.7@1" -}}
+{{$version := "2.17.0@1" -}}
 {
   "name": "googet",
   "version": "{{$version}}",
@@ -13,6 +13,7 @@
     "path": "install.ps1"
   },
   "releaseNotes": [
+    "2.17.0 - Update lock functionality.",
     "2.16.7 - Add a status code check for HTTP responses.",
     "2.16.6 - Additional error logging in cleanOld.",
     "2.16.4 - Add flag to 'googet installed' to list directly-managed files.",

install.ps1

@@ -9,3 +9,6 @@ $path = (Get-ItemProperty $machine_env).Path
 if ($path -notlike "*${googet_root}*") {
   Set-ItemProperty $machine_env -Name 'Path' -Value ($path + ";${googet_root}")
 }
+
+# Set permisons on GooGet root
+Start-Process icacls.exe -ArgumentList @($googet_root, '/grant:r', 'Administrators:(OI)(CI)F', '/grant:r', 'SYSTEM:(OI)(CI)F', '/grant:r', 'Users:(OI)(CI)RX', '/inheritance:r')

install/install.go

@@ -162,9 +162,7 @@ func FromRepo(ctx context.Context, pi goolib.PackageInfo, repo, cache string, rm
 	fmt.Printf("Installation of %s.%s.%s and all dependencies completed\n", pi.Name, pi.Arch, pi.Ver)
 	// Clean up old version, if applicable.
 	pi = goolib.PackageInfo{Name: pi.Name, Arch: pi.Arch, Ver: ""}
-	if err := cleanOld(state, pi, insFiles, dbOnly); err != nil {
-		return err
-	}
+	cleanOld(state, pi, insFiles, dbOnly)
 
 	state.Add(client.PackageState{
 		SourceRepo:     repo,
@@ -249,9 +247,7 @@ func FromDisk(arg, cache string, state *client.GooGetState, dbOnly, ri bool) err
 
 	// Clean up old version, if applicable.
 	pi := goolib.PackageInfo{Name: zs.Name, Arch: zs.Arch, Ver: ""}
-	if err := cleanOld(state, pi, insFiles, dbOnly); err != nil {
-		return err
-	}
+	cleanOld(state, pi, insFiles, dbOnly)
 
 	state.Add(client.PackageState{
 		LocalPath:      dst,
@@ -444,11 +440,10 @@ func resolveDst(dst string) string {
 	return dst
 }
 
-func cleanOld(state *client.GooGetState, pi goolib.PackageInfo, insFiles map[string]string, dbOnly bool) error {
+func cleanOld(state *client.GooGetState, pi goolib.PackageInfo, insFiles map[string]string, dbOnly bool) {
 	st, err := state.GetPackageState(pi)
 	if err != nil {
-		logger.Errorf("GetPackageState: %v", err)
-		return nil
+		return
 	}
 	if !dbOnly {
 		cleanOldFiles(st, insFiles)
@@ -459,7 +454,8 @@ func cleanOld(state *client.GooGetState, pi goolib.PackageInfo, insFiles map[str
 	if st.UnpackDir != "" && oswrap.RemoveAll(st.UnpackDir) != nil {
 		logger.Error(err)
 	}
-	return state.Remove(pi)
+	state.Remove(pi)
+	return
 }
 
 func cleanOldFiles(oldState client.PackageState, insFiles map[string]string) {

lock_linux.go

@@ -0,0 +1,31 @@
+//  Copyright 2019 Google Inc. All Rights Reserved.
+//
+//  Licensed under the Apache License, Version 2.0 (the "License");
+//  you may not use this file except in compliance with the License.
+//  You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+//  Unless required by applicable law or agreed to in writing, software
+//  distributed under the License is distributed on an "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//  See the License for the specific language governing permissions and
+//  limitations under the License.
+
+package main
+
+import (
+	"os"
+	"syscall"
+)
+
+const lockFile = "/run/lock/googet.lock"
+
+func lock(f *os.File) error {
+	if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil {
+		return err
+	}
+
+	deferredFuncs = append(deferredFuncs, func() { syscall.Flock(int(f.Fd()), syscall.LOCK_UN); f.Close(); os.Remove(lockFile) })
+	return nil
+}

lock_windows.go

@@ -0,0 +1,78 @@
+//  Copyright 2019 Google Inc. All Rights Reserved.
+//
+//  Licensed under the Apache License, Version 2.0 (the "License");
+//  you may not use this file except in compliance with the License.
+//  You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+//  Unless required by applicable law or agreed to in writing, software
+//  distributed under the License is distributed on an "AS IS" BASIS,
+//  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//  See the License for the specific language governing permissions and
+//  limitations under the License.
+
+package main
+
+import (
+	"errors"
+	"os"
+	"syscall"
+	"unsafe"
+
+	"golang.org/x/sys/windows"
+)
+
+var (
+	kernel32         = windows.NewLazySystemDLL("kernel32.dll")
+	procLockFileEx   = kernel32.NewProc("LockFileEx")
+	procUnlockFileEx = kernel32.NewProc("UnlockFileEx")
+)
+
+const (
+	lockFile = "C:/ProgramData/GooGet/googet.lock"
+
+	// https://docs.microsoft.com/en-us/windows/desktop/api/fileapi/nf-fileapi-lockfileex
+	LOCKFILE_EXCLUSIVE_LOCK   = 2
+	LOCKFILE_FAIL_IMMEDIATELY = 1
+)
+
+func lockFileEx(hFile uintptr, dwFlags, nNumberOfBytesToLockLow, nNumberOfBytesToLockHigh uint32, lpOverlapped *syscall.Overlapped) (err error) {
+	ret, _, _ := procLockFileEx.Call(
+		hFile,
+		uintptr(dwFlags),
+		0,
+		uintptr(nNumberOfBytesToLockLow),
+		uintptr(nNumberOfBytesToLockHigh),
+		uintptr(unsafe.Pointer(lpOverlapped)),
+	)
+	// If the function succeeds, the return value is nonzero.
+	if ret == 0 {
+		return errors.New("LockFileEx unable to obtain lock")
+	}
+	return nil
+}
+
+func unlockFileEx(hFile uintptr, nNumberOfBytesToLockLow, nNumberOfBytesToLockHigh uint32, lpOverlapped *syscall.Overlapped) (err error) {
+	ret, _, _ := procUnlockFileEx.Call(
+		hFile,
+		0,
+		uintptr(nNumberOfBytesToLockLow),
+		uintptr(nNumberOfBytesToLockHigh),
+		uintptr(unsafe.Pointer(lpOverlapped)),
+	)
+	// If the function succeeds, the return value is nonzero.
+	if ret == 0 {
+		return errors.New("UnlockFileEx unable to unlock")
+	}
+	return nil
+}
+
+func lock(f *os.File) error {
+	if err := lockFileEx(f.Fd(), LOCKFILE_EXCLUSIVE_LOCK, 1, 0, &syscall.Overlapped{}); err != nil {
+		return err
+	}
+
+	deferredFuncs = append(deferredFuncs, func() { unlockFileEx(f.Fd(), 1, 0, &syscall.Overlapped{}); f.Close(); os.Remove(lockFile) })
+	return nil
+}