From 87e4177928bce2d896dde8e6460afcbcb8d24f52 Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Sun, 29 Jun 2014 15:43:59 -0700 Subject: [PATCH 1/2] Treat GOPATH as a list of workspaces; use first workspace. Fixes #60. The behavior should be improved (fixes #60) if user has multiple GOPATH workspaces. The behavior should be unchanged if user has a single GOPATH workspace. First cut at correct GOPATH handling as a list, rather than assuming it contains no more than 1 entry. It's unfinished and needs more work to correctly handle all scenarios with multiple GOPATH workspaces, but it's an improvement over the original version. Use filepath.Join instead of concatenating strings for path manipulation. --- build/build.go | 38 ++++++++++++++++++++++++++++---------- tool.go | 17 ++++++++++++----- 2 files changed, 40 insertions(+), 15 deletions(-) diff --git a/build/build.go b/build/build.go index 399f3b5e0..ab1dc880d 100644 --- a/build/build.go +++ b/build/build.go @@ -1,11 +1,7 @@ package build import ( - "bitbucket.org/kardianos/osext" - "code.google.com/p/go.exp/fsnotify" "fmt" - "github.com/gopherjs/gopherjs/compiler" - "github.com/neelance/sourcemap" "go/ast" "go/build" "go/parser" @@ -17,6 +13,11 @@ import ( "runtime" "strings" "time" + + "bitbucket.org/kardianos/osext" + "code.google.com/p/go.exp/fsnotify" + "github.com/gopherjs/gopherjs/compiler" + "github.com/neelance/sourcemap" ) type ImportCError struct{} @@ -55,7 +56,11 @@ func Import(path string, mode build.ImportMode, archSuffix string) (*build.Packa } if _, err := os.Stat(pkg.PkgObj); os.IsNotExist(err) && strings.HasPrefix(pkg.PkgObj, build.Default.GOROOT) { // fall back to GOPATH - gopathPkgObj := build.Default.GOPATH + pkg.PkgObj[len(build.Default.GOROOT):] + // TODO: Instead of always using the first GOPATH workspace, perhaps one should be chosen more intelligently? + // In this case, I think all GOPATH workspaces should be consulted, so one would need to iterate over + // each one, until there's a match (or no matches at all). + firstGopathWorkspace := filepath.SplitList(build.Default.GOPATH)[0] + gopathPkgObj := filepath.Join(firstGopathWorkspace, pkg.PkgObj[len(build.Default.GOROOT):]) if _, err := os.Stat(gopathPkgObj); err == nil { pkg.PkgObj = gopathPkgObj } @@ -361,8 +366,9 @@ func (s *Session) BuildPackage(pkg *PackageData) error { if err := s.writeLibraryPackage(pkg, pkg.PkgObj); err != nil { if strings.HasPrefix(pkg.PkgObj, s.options.GOROOT) { - // fall back to GOPATH - if err := s.writeLibraryPackage(pkg, s.options.GOPATH+pkg.PkgObj[len(s.options.GOROOT):]); err != nil { + // fall back to first GOPATH workspace + firstGopathWorkspace := filepath.SplitList(s.options.GOPATH)[0] + if err := s.writeLibraryPackage(pkg, filepath.Join(firstGopathWorkspace, pkg.PkgObj[len(s.options.GOROOT):])); err != nil { return err } return nil @@ -421,9 +427,9 @@ func (s *Session) WriteCommandPackage(pkg *PackageData, pkgObj string) error { } pos := fileSet.Position(originalPos) file := pos.Filename - switch { - case strings.HasPrefix(file, s.options.GOPATH): - file = filepath.ToSlash(filepath.Join("/gopath", file[len(s.options.GOPATH):])) + switch hasGopathPrefix, prefixLen := hasGopathPrefix(file, s.options.GOPATH); { + case hasGopathPrefix: + file = filepath.ToSlash(filepath.Join("/gopath", file[prefixLen:])) case strings.HasPrefix(file, s.options.GOROOT): file = filepath.ToSlash(filepath.Join("/goroot", file[len(s.options.GOROOT):])) default: @@ -440,6 +446,18 @@ func (s *Session) WriteCommandPackage(pkg *PackageData, pkgObj string) error { return compiler.WriteProgramCode(deps, s.ImportContext, sourceMapFilter) } +// hasGopathPrefix returns true and the length of the matched GOPATH workspace, +// iff file has a prefix that matches one of the GOPATH workspaces. +func hasGopathPrefix(file, gopath string) (hasGopathPrefix bool, prefixLen int) { + gopathWorkspaces := filepath.SplitList(gopath) + for _, gopathWorkspace := range gopathWorkspaces { + if strings.HasPrefix(file, gopathWorkspace) { + return true, len(gopathWorkspace) + } + } + return false, 0 +} + func (s *Session) WaitForChange() { fmt.Println("\x1B[32mwatching for changes...\x1B[39m") select { diff --git a/tool.go b/tool.go index cd58ec856..6a1a091ed 100644 --- a/tool.go +++ b/tool.go @@ -2,11 +2,8 @@ package main import ( "bytes" - "code.google.com/p/go.tools/go/types" "flag" "fmt" - gbuild "github.com/gopherjs/gopherjs/build" - "github.com/gopherjs/gopherjs/compiler" "go/ast" "go/build" "go/parser" @@ -20,6 +17,10 @@ import ( "syscall" "text/template" "time" + + "code.google.com/p/go.tools/go/types" + gbuild "github.com/gopherjs/gopherjs/build" + "github.com/gopherjs/gopherjs/compiler" ) var currentDirectory string @@ -136,7 +137,10 @@ func main() { exitCode := handleError(func() error { pkgs := installFlags.Args() if len(pkgs) == 0 { - srcDir, err := filepath.EvalSymlinks(filepath.Join(build.Default.GOPATH, "src")) + // TODO: Instead of always using the first GOPATH workspace, perhaps one should be chosen more intelligently? + // In this case, the GOPATH workspace that contains the package source should be chosen. + firstGopathWorkspace := filepath.SplitList(build.Default.GOPATH)[0] + srcDir, err := filepath.EvalSymlinks(filepath.Join(firstGopathWorkspace, "src")) if err != nil { return err } @@ -220,7 +224,10 @@ func main() { } } if len(pkgs) == 0 { - srcDir, err := filepath.EvalSymlinks(filepath.Join(build.Default.GOPATH, "src")) + // TODO: Instead of always using the first GOPATH workspace, perhaps one should be chosen more intelligently? + // Not sure which GOPATH workspace should be chosen for this... + firstGopathWorkspace := filepath.SplitList(build.Default.GOPATH)[0] + srcDir, err := filepath.EvalSymlinks(filepath.Join(firstGopathWorkspace, "src")) if err != nil { return err } From e6992b00737620559f48e18a12b7f6d41ec1f21e Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Sat, 5 Jul 2014 17:11:34 -0700 Subject: [PATCH 2/2] Clean up the PR. Use gofmt instead of goimports to not change the order of the imports. Consolidate TODO comments into a short version. --- build/build.go | 14 +++++--------- tool.go | 15 +++++---------- 2 files changed, 10 insertions(+), 19 deletions(-) diff --git a/build/build.go b/build/build.go index ab1dc880d..84e486a7f 100644 --- a/build/build.go +++ b/build/build.go @@ -1,7 +1,11 @@ package build import ( + "bitbucket.org/kardianos/osext" + "code.google.com/p/go.exp/fsnotify" "fmt" + "github.com/gopherjs/gopherjs/compiler" + "github.com/neelance/sourcemap" "go/ast" "go/build" "go/parser" @@ -13,11 +17,6 @@ import ( "runtime" "strings" "time" - - "bitbucket.org/kardianos/osext" - "code.google.com/p/go.exp/fsnotify" - "github.com/gopherjs/gopherjs/compiler" - "github.com/neelance/sourcemap" ) type ImportCError struct{} @@ -56,10 +55,7 @@ func Import(path string, mode build.ImportMode, archSuffix string) (*build.Packa } if _, err := os.Stat(pkg.PkgObj); os.IsNotExist(err) && strings.HasPrefix(pkg.PkgObj, build.Default.GOROOT) { // fall back to GOPATH - // TODO: Instead of always using the first GOPATH workspace, perhaps one should be chosen more intelligently? - // In this case, I think all GOPATH workspaces should be consulted, so one would need to iterate over - // each one, until there's a match (or no matches at all). - firstGopathWorkspace := filepath.SplitList(build.Default.GOPATH)[0] + firstGopathWorkspace := filepath.SplitList(build.Default.GOPATH)[0] // TODO: Need to check inside all GOPATH workspaces. gopathPkgObj := filepath.Join(firstGopathWorkspace, pkg.PkgObj[len(build.Default.GOROOT):]) if _, err := os.Stat(gopathPkgObj); err == nil { pkg.PkgObj = gopathPkgObj diff --git a/tool.go b/tool.go index 6a1a091ed..e166564f3 100644 --- a/tool.go +++ b/tool.go @@ -2,8 +2,11 @@ package main import ( "bytes" + "code.google.com/p/go.tools/go/types" "flag" "fmt" + gbuild "github.com/gopherjs/gopherjs/build" + "github.com/gopherjs/gopherjs/compiler" "go/ast" "go/build" "go/parser" @@ -17,10 +20,6 @@ import ( "syscall" "text/template" "time" - - "code.google.com/p/go.tools/go/types" - gbuild "github.com/gopherjs/gopherjs/build" - "github.com/gopherjs/gopherjs/compiler" ) var currentDirectory string @@ -137,9 +136,7 @@ func main() { exitCode := handleError(func() error { pkgs := installFlags.Args() if len(pkgs) == 0 { - // TODO: Instead of always using the first GOPATH workspace, perhaps one should be chosen more intelligently? - // In this case, the GOPATH workspace that contains the package source should be chosen. - firstGopathWorkspace := filepath.SplitList(build.Default.GOPATH)[0] + firstGopathWorkspace := filepath.SplitList(build.Default.GOPATH)[0] // TODO: The GOPATH workspace that contains the package source should be chosen. srcDir, err := filepath.EvalSymlinks(filepath.Join(firstGopathWorkspace, "src")) if err != nil { return err @@ -224,9 +221,7 @@ func main() { } } if len(pkgs) == 0 { - // TODO: Instead of always using the first GOPATH workspace, perhaps one should be chosen more intelligently? - // Not sure which GOPATH workspace should be chosen for this... - firstGopathWorkspace := filepath.SplitList(build.Default.GOPATH)[0] + firstGopathWorkspace := filepath.SplitList(build.Default.GOPATH)[0] // TODO: Not sure if always picking first GOPATH workspace here is the right thing. srcDir, err := filepath.EvalSymlinks(filepath.Join(firstGopathWorkspace, "src")) if err != nil { return err