From 95963e031d86b0e7dafe40fde044d7f610404855 Mon Sep 17 00:00:00 2001 From: Filippo Valsorda Date: Tue, 7 Jun 2016 21:47:49 +0100 Subject: [PATCH] imports: support symlinks in GOPATH MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The go tool will import a package with a import path that traverses symlinks with no problems, but goimports would remove that import because it would fail to recognize the package as existent. Fixes golang/go#14845 Note: if the file you are currently processing also has a name inside the symlink, you might have to use the "long" path for vendoring to work, as it wouldn't be recognized as "deeper" than the vendor folder otherwise. For example in this tree: . ├── myfile.go └── x ├── vendor │   └── mypkg │   └── foo.go └── y -> .. If myfile.go imports mypkg, you will have to process it as ./x/y/myfile.go, not ./myfile.go. Change-Id: Ic8f41baed3f28d4e9b813160d91aef09ece1fc9f Reviewed-on: https://go-review.googlesource.com/23803 Reviewed-by: Andrew Gerrand Run-TryBot: Andrew Gerrand TryBot-Result: Gobot Gobot --- imports/fix.go | 52 +++++++++++++++++++++++++++++++-- imports/fix_test.go | 70 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 2 deletions(-) diff --git a/imports/fix.go b/imports/fix.go index d13836ce..be838299 100644 --- a/imports/fix.go +++ b/imports/fix.go @@ -194,6 +194,54 @@ func (g gate) leave() { <-g } // Too much disk I/O -> too many threads -> swapping and bad scheduling. var fsgate = make(gate, 8) +var visitedSymlinks struct { + sync.Mutex + m map[string]struct{} +} + +// shouldTraverse checks if fi, found in dir, is a directory or a symlink to a directory. +// It makes sure symlinks were never visited before to avoid symlink loops. +func shouldTraverse(dir string, fi os.FileInfo) bool { + if fi.IsDir() { + return true + } + + if fi.Mode()&os.ModeSymlink == 0 { + return false + } + path := filepath.Join(dir, fi.Name()) + target, err := filepath.EvalSymlinks(path) + if err != nil { + fmt.Fprint(os.Stderr, err) + return false + } + ts, err := os.Stat(target) + if err != nil { + fmt.Fprint(os.Stderr, err) + return false + } + if !ts.IsDir() { + return false + } + + realParent, err := filepath.EvalSymlinks(dir) + if err != nil { + fmt.Fprint(os.Stderr, err) + return false + } + realPath := filepath.Join(realParent, fi.Name()) + visitedSymlinks.Lock() + defer visitedSymlinks.Unlock() + if visitedSymlinks.m == nil { + visitedSymlinks.m = make(map[string]struct{}) + } + if _, ok := visitedSymlinks.m[realPath]; ok { + return false + } + visitedSymlinks.m[realPath] = struct{}{} + return true +} + func loadPkgIndex() { pkgIndex.Lock() pkgIndex.m = make(map[string][]pkg) @@ -216,7 +264,7 @@ func loadPkgIndex() { continue } for _, child := range children { - if child.IsDir() { + if shouldTraverse(path, child) { wg.Add(1) go func(path, name string) { defer wg.Done() @@ -257,7 +305,7 @@ func loadPkg(wg *sync.WaitGroup, root, pkgrelpath string) { if strings.HasSuffix(name, ".go") { hasGo = true } - if child.IsDir() { + if shouldTraverse(dir, child) { wg.Add(1) go func(root, name string) { defer wg.Done() diff --git a/imports/fix_test.go b/imports/fix_test.go index 9c89ef67..78a50a8c 100644 --- a/imports/fix_test.go +++ b/imports/fix_test.go @@ -823,6 +823,76 @@ func TestFixImports(t *testing.T) { } } +// Test support for packages in GOPATH that are actually symlinks. +// Also test that a symlink loop does not block the process. +func TestImportSymlinks(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Skipping test on Windows as there are no symlinks.") + } + + newGoPath, err := ioutil.TempDir("", "symlinktest") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(newGoPath) + + targetPath := newGoPath + "/target" + if err := os.MkdirAll(targetPath, 0755); err != nil { + t.Fatal(err) + } + if err := ioutil.WriteFile(targetPath+"/f.go", []byte("package mypkg\nvar Foo = 123\n"), 0666); err != nil { + t.Fatal(err) + } + + symlinkPath := newGoPath + "/src/x/mypkg" + if err := os.MkdirAll(filepath.Dir(symlinkPath), 0755); err != nil { + t.Fatal(err) + } + if err := os.Symlink(targetPath, symlinkPath); err != nil { + t.Fatal(err) + } + + // Add a symlink loop. + if err := os.Symlink(newGoPath+"/src/x", newGoPath+"/src/x/apkg"); err != nil { + t.Fatal(err) + } + + pkgIndexOnce = &sync.Once{} + oldGOPATH := build.Default.GOPATH + build.Default.GOPATH = newGoPath + defer func() { + build.Default.GOPATH = oldGOPATH + visitedSymlinks.m = nil + }() + + input := `package p + +var ( + _ = fmt.Print + _ = mypkg.Foo +) +` + output := `package p + +import ( + "fmt" + "x/mypkg" +) + +var ( + _ = fmt.Print + _ = mypkg.Foo +) +` + buf, err := Process(newGoPath+"/src/myotherpkg/toformat.go", []byte(input), &Options{}) + if err != nil { + t.Fatal(err) + } + if got := string(buf); got != output { + t.Fatalf("results differ\nGOT:\n%s\nWANT:\n%s\n", got, output) + } +} + // Test for correctly identifying the name of a vendored package when it // differs from its directory name. In this test, the import line // "mypkg.com/mypkg.v1" would be removed if goimports wasn't able to detect