Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bugfix] fix race conditions by using sync.Once #245

Merged
merged 1 commit into from
Dec 28, 2019
Merged

[bugfix] fix race conditions by using sync.Once #245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion cmd_create_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"path/filepath"
"reflect"
"strings"
"sync"
"testing"

"github.com/Songmu/gitconfig"
Expand All @@ -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"})
Expand Down
5 changes: 5 additions & 0 deletions cmd_list_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"runtime"
"sort"
"strings"
"sync"
"testing"

"github.com/urfave/cli/v2"
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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"})
Expand All @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions cmd_root_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"path/filepath"
"runtime"
"strings"
"sync"
"testing"

"github.com/Songmu/gitconfig"
Expand Down Expand Up @@ -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"})
Expand Down
2 changes: 2 additions & 0 deletions commands_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"net/url"
"os"
"path/filepath"
"sync"
"testing"

"github.com/Songmu/gitconfig"
Expand Down Expand Up @@ -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
Expand Down
114 changes: 61 additions & 53 deletions local_repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions local_repository_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"reflect"
"runtime"
"sort"
"sync"
"testing"

"github.com/Songmu/gitconfig"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down