Merge pull request #245 from motemen/fix-race

[bugfix] fix race conditions by using sync.Once

cmd_create_test.go

@@ -6,6 +6,7 @@ import (
 	"path/filepath"
 	"reflect"
 	"strings"
+	"sync"
 	"testing"
 
 	"github.com/Songmu/gitconfig"
@@ -23,11 +24,14 @@ func TestDoCreate(t *testing.T) {
 	}
 	defer func(orig string) { _home = orig }(_home)
 	_home = ""
+	homeOnce = &sync.Once{}
 	defer gitconfig.WithConfig(t, "")()
 	tmpd := newTempDir(t)
 	defer os.RemoveAll(tmpd)
 	defer func(orig []string) { _localRepositoryRoots = orig }(_localRepositoryRoots)
-	_localRepositoryRoots = []string{tmpd}
+	defer tmpEnv(envGhqRoot, tmpd)()
+	_localRepositoryRoots = nil
+	localRepoOnce = &sync.Once{}
 
 	out, _, _ := capture(func() {
 		newApp().Run([]string{"", "create", "motemen/ghqq"})

cmd_list_test.go

@@ -7,6 +7,7 @@ import (
 	"runtime"
 	"sort"
 	"strings"
+	"sync"
 	"testing"
 
 	"github.com/urfave/cli/v2"
@@ -183,6 +184,7 @@ func TestDoList_unique(t *testing.T) {
 	defer os.RemoveAll(tmp2)
 
 	_localRepositoryRoots = nil
+	localRepoOnce = &sync.Once{}
 	rootPaths := []string{tmp1, tmp2}
 	os.Setenv(envGhqRoot, strings.Join(rootPaths, string(os.PathListSeparator)))
 	for _, rootPath := range rootPaths {
@@ -200,6 +202,7 @@ func TestDoList_unknownRoot(t *testing.T) {
 	defer func(orig []string) { _localRepositoryRoots = orig }(_localRepositoryRoots)
 	defer tmpEnv(envGhqRoot, "/path/to/unknown-ghq")()
 	_localRepositoryRoots = nil
+	localRepoOnce = &sync.Once{}
 
 	err := newApp().Run([]string{"ghq", "list"})
 	if err != nil {
@@ -220,6 +223,7 @@ func TestDoList_notPermittedRoot(t *testing.T) {
 	defer tmpEnv(envGhqRoot, tmpdir)()
 
 	_localRepositoryRoots = nil
+	localRepoOnce = &sync.Once{}
 	os.Chmod(tmpdir, 0000)
 
 	err := newApp().Run([]string{"ghq", "list"})
@@ -243,6 +247,7 @@ func TestDoList_withSystemHiddenDir(t *testing.T) {
 	defer tmpEnv(envGhqRoot, tmpdir)()
 
 	_localRepositoryRoots = nil
+	localRepoOnce = &sync.Once{}
 
 	err := newApp().Run([]string{"ghq", "list"})
 	if err != nil {

cmd_root_test.go

@@ -5,6 +5,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
+	"sync"
 	"testing"
 
 	"github.com/Songmu/gitconfig"
@@ -95,8 +96,10 @@ func TestDoRoot(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			defer func(orig []string) { _localRepositoryRoots = orig }(_localRepositoryRoots)
 			_localRepositoryRoots = nil
+			localRepoOnce = &sync.Once{}
 			defer func(orig string) { _home = orig }(_home)
 			_home = ""
+			homeOnce = &sync.Once{}
 			defer tc.setup()()
 			out, _, _ := capture(func() {
 				newApp().Run([]string{"", "root"})

commands_test.go

@@ -4,6 +4,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"sync"
 	"testing"
 
 	"github.com/Songmu/gitconfig"
@@ -52,6 +53,7 @@ func withFakeGitBackend(t *testing.T, block func(*testing.T, string, *_cloneArgs
 	}
 	defer func(orig string) { _home = orig }(_home)
 	_home = ""
+	homeOnce = &sync.Once{}
 	defer gitconfig.WithConfig(t, "")()
 
 	GitBackend = tmpBackend

local_repository.go

@@ -310,18 +310,24 @@ func walkLocalRepositories(vcs string, callback func(*LocalRepository)) error {
 	return nil
 }
 
-var _home string
+var (
+	_home    string
+	_homeErr error
+	homeOnce = &sync.Once{}
+)
 
 func getHome() (string, error) {
-	if _home != "" {
-		return _home, nil
-	}
-	var err error
-	_home, err = os.UserHomeDir()
-	return _home, err
+	homeOnce.Do(func() {
+		_home, _homeErr = os.UserHomeDir()
+	})
+	return _home, _homeErr
 }
 
-var _localRepositoryRoots []string
+var (
+	_localRepositoryRoots []string
+	_localRepoErr         error
+	localRepoOnce         = &sync.Once{}
+)
 
 // localRepositoryRoots returns locally cloned repositories' root directories.
 // The root dirs are determined as following:
@@ -332,60 +338,62 @@ var _localRepositoryRoots []string
 //   - When GHQ_ROOT is empty, specific root dirs are added from the result of
 //     `git config --path --get-regexp '^ghq\..+\.root$`
 func localRepositoryRoots(all bool) ([]string, error) {
-	if len(_localRepositoryRoots) != 0 {
-		return _localRepositoryRoots, nil
-	}
-
-	envRoot := os.Getenv(envGhqRoot)
-	if envRoot != "" {
-		_localRepositoryRoots = filepath.SplitList(envRoot)
-	} else {
-		var err error
-		_localRepositoryRoots, err = gitconfig.PathAll("ghq.root")
-		if err != nil && !gitconfig.IsNotFound(err) {
-			return nil, err
-		}
-		// reverse slice
-		for i := len(_localRepositoryRoots)/2 - 1; i >= 0; i-- {
-			opp := len(_localRepositoryRoots) - 1 - i
-			_localRepositoryRoots[i], _localRepositoryRoots[opp] =
-				_localRepositoryRoots[opp], _localRepositoryRoots[i]
+	localRepoOnce.Do(func() {
+		envRoot := os.Getenv(envGhqRoot)
+		if envRoot != "" {
+			_localRepositoryRoots = filepath.SplitList(envRoot)
+		} else {
+			var err error
+			_localRepositoryRoots, err = gitconfig.PathAll("ghq.root")
+			if err != nil && !gitconfig.IsNotFound(err) {
+				_localRepoErr = err
+				return
+			}
+			// reverse slice
+			for i := len(_localRepositoryRoots)/2 - 1; i >= 0; i-- {
+				opp := len(_localRepositoryRoots) - 1 - i
+				_localRepositoryRoots[i], _localRepositoryRoots[opp] =
+					_localRepositoryRoots[opp], _localRepositoryRoots[i]
+			}
 		}
-	}
 
-	if len(_localRepositoryRoots) == 0 {
-		homeDir, err := getHome()
-		if err != nil {
-			return nil, err
+		if len(_localRepositoryRoots) == 0 {
+			homeDir, err := getHome()
+			if err != nil {
+				_localRepoErr = err
+				return
+			}
+			_localRepositoryRoots = []string{filepath.Join(homeDir, ".ghq")}
 		}
-		_localRepositoryRoots = []string{filepath.Join(homeDir, ".ghq")}
-	}
 
-	if all && envRoot == "" {
-		roots, err := urlMatchLocalRepositoryRoots()
-		if err != nil {
-			return nil, err
+		if all && envRoot == "" {
+			roots, err := urlMatchLocalRepositoryRoots()
+			if err != nil {
+				_localRepoErr = err
+				return
+			}
+			_localRepositoryRoots = append(_localRepositoryRoots, roots...)
 		}
-		_localRepositoryRoots = append(_localRepositoryRoots, roots...)
-	}
 
-	for i, v := range _localRepositoryRoots {
-		path := filepath.Clean(v)
-		if _, err := os.Stat(path); err == nil {
-			if path, err = filepath.EvalSymlinks(path); err != nil {
-				return nil, err
+		for i, v := range _localRepositoryRoots {
+			path := filepath.Clean(v)
+			if _, err := os.Stat(path); err == nil {
+				if path, err = filepath.EvalSymlinks(path); err != nil {
+					_localRepoErr = err
+					return
+				}
 			}
-		}
-		if !filepath.IsAbs(path) {
-			var err error
-			if path, err = filepath.Abs(path); err != nil {
-				return nil, err
+			if !filepath.IsAbs(path) {
+				var err error
+				if path, err = filepath.Abs(path); err != nil {
+					_localRepoErr = err
+					return
+				}
 			}
+			_localRepositoryRoots[i] = path
 		}
-		_localRepositoryRoots[i] = path
-	}
-
-	return _localRepositoryRoots, nil
+	})
+	return _localRepositoryRoots, _localRepoErr
 }
 
 func urlMatchLocalRepositoryRoots() ([]string, error) {

local_repository_test.go

@@ -6,6 +6,7 @@ import (
 	"reflect"
 	"runtime"
 	"sort"
+	"sync"
 	"testing"
 
 	"github.com/Songmu/gitconfig"
@@ -109,6 +110,7 @@ func TestNewLocalRepository(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			defer func(orig string) { _home = orig }(_home)
 			_home = ""
+			homeOnce = &sync.Once{}
 			defer gitconfig.WithConfig(t, "")()
 			r, err := LocalRepositoryFromURL(mustParseURL(tc.url))
 			if err != nil {
@@ -147,6 +149,7 @@ func TestLocalRepositoryRoots(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.root, func(t *testing.T) {
 			_localRepositoryRoots = nil
+			localRepoOnce = &sync.Once{}
 			os.Setenv(envGhqRoot, tc.root)
 			got, err := localRepositoryRoots(true)
 			if err != nil {
@@ -300,6 +303,7 @@ func TestLocalRepository_VCS(t *testing.T) {
 	defer func(orig string) { os.Setenv(envGhqRoot, orig) }(os.Getenv(envGhqRoot))
 
 	_localRepositoryRoots = nil
+	localRepoOnce = &sync.Once{}
 	tmpdir := newTempDir(t)
 	os.Setenv(envGhqRoot, tmpdir)
 
@@ -348,6 +352,7 @@ func TestURLMatchLocalRepositoryRoots(t *testing.T) {
 	defer tmpEnv("HOME", "/home/tmp")()
 	defer func(orig string) { _home = orig }(_home)
 	_home = ""
+	homeOnce = &sync.Once{}
 	defer gitconfig.WithConfig(t, `
 [ghq]
   root = /hoge