From 6652d08143a1f126aa1ad03c60bb4f0f20b24da5 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Jan 30 2019 02:27:20 +0000 Subject: [PATCH 1/4] Remove unused functions and make one private. --- diff --git a/pkg/util/util.go b/pkg/util/util.go index d22111c..e321325 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -5,7 +5,6 @@ import ( "go/parser" "go/token" "os" - "os/exec" "path" "path/filepath" "regexp" @@ -14,7 +13,6 @@ import ( "pagure.io/golist/pkg/util/internal/load" "pagure.io/golist/pkg/util/internal/work" - "github.com/golang/glog" // initialize the load.ModInit function "pagure.io/golist/pkg/util/internal/modload" @@ -118,7 +116,7 @@ func (p *PackageInfoCollector) isStandard(pkg string) (bool, error) { if is, exists := p.isStdPackages[pkg]; exists { return is, nil } - pkgInfo, err := ListPackage(pkg) + pkgInfo, err := listPackage(pkg) if err != nil { return false, err } @@ -178,7 +176,7 @@ func (p *PackageInfoCollector) CollectPackageInfos(packagePath string) error { return nil } - pkgInfo, err := ListPackage(relPath) + pkgInfo, err := listPackage(relPath) if err != nil { if strings.Contains(err.Error(), "no Go files in") { return nil @@ -359,7 +357,7 @@ func (p *PackageInfoCollector) CollectProjectDeps(standard bool, skipSelf bool, continue } - pkgInfo, err := ListPackage(relPath) + pkgInfo, err := listPackage(relPath) // assuming the stdlib is always processed properly if !standard && err == nil && pkgInfo.Standard { continue @@ -413,7 +411,7 @@ func (p *PackageInfoCollector) BuildPackageTree(includeMain bool, tests bool) ([ return entryPoints, nil } -func ListPackage(path string) (*load.PackagePublic, error) { +func listPackage(path string) (*load.PackagePublic, error) { // TODO(jchaloup): more things need to be init most likely work.BuildInit() load.ModInit = modload.Init @@ -432,91 +430,6 @@ func ListPackage(path string) (*load.PackagePublic, error) { return &pkg.PackagePublic, nil } -func ListGoFiles(packagePath string, cgo bool) ([]string, error) { - - collectFiles := func(output string) []string { - line := strings.Split(string(output), "\n")[0] - line = line[1 : len(line)-1] - if line == "" { - return nil - } - return strings.Split(line, " ") - } - // check GOPATH/packagePath - filter := "{{.GoFiles}}" - if cgo { - filter = "{{.CgoFiles}}" - } - cmd := exec.Command("go", "list", "-f", filter, packagePath) - output, e := cmd.CombinedOutput() - if e == nil { - return collectFiles(string(output)), nil - } - - if strings.Contains(string(output), "no buildable Go source files in") { - return nil, nil - } - - // if strings.Contains(string(output), "no Go files in") { - // return nil, nil - // } - - return nil, fmt.Errorf("%v: %v, %v", strings.Join(cmd.Args, " "), e, string(output)) -} - -func GetPackageFiles(packageRoot, packagePath string) (files []string, packageLocation string, err error) { - - files, ppath, e := func() ([]string, string, error) { - var searched []string - // First searched the vendor directories - pathParts := strings.Split(packageRoot, string(os.PathSeparator)) - for i := len(pathParts); i >= 0; i-- { - vendorpath := path.Join(path.Join(pathParts[:i]...), "vendor", packagePath) - glog.V(1).Infof("Checking %v directory", vendorpath) - if l, e := ListGoFiles(vendorpath, false); e == nil { - glog.V(1).Infof("Found %v directory", vendorpath) - return l, vendorpath, e - } - searched = append(searched, vendorpath) - } - - glog.V(1).Infof("Checking %v directory", packagePath) - if l, e := ListGoFiles(packagePath, false); e == nil { - glog.V(1).Infof("Found %v directory", packagePath) - return l, packagePath, e - } - searched = append(searched, packagePath) - - return nil, "", fmt.Errorf("Unable to find %q in any of:\n\t\t%v\n", packagePath, strings.Join(searched, "\n\t\t")) - }() - - if e != nil { - return nil, "", e - } - - // cgo files enabled? - cgoFiles, e := ListGoFiles(ppath, true) - if e != nil { - return nil, "", e - } - - if len(cgoFiles) > 0 { - files = append(files, cgoFiles...) - } - - { - cmd := exec.Command("go", "list", "-f", "{{.Dir}}", ppath) - output, err := cmd.CombinedOutput() - if err != nil { - return nil, "", fmt.Errorf("go list -f {{.Dir}} %v failed: %v", ppath, err) - } - lines := strings.Split(string(output), "\n") - packageLocation = string(lines[0]) - } - - return files, packageLocation, nil -} - type ProjectData struct { Packages []string `json:"packages"` Dependencies map[string][]string `json:"dependencies"` From 420453690332c3311c96b4ba962cfcba06688f10 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Jan 31 2019 10:08:10 +0000 Subject: [PATCH 2/4] Replace Go internals with go/build. It does mean copying one function (`IsStandardImportPath`), but it's fairly short and exactly what, e.g., dep does. --- diff --git a/README.md b/README.md index c7a1781..d2ec50d 100644 --- a/README.md +++ b/README.md @@ -8,11 +8,7 @@ It is the analysis engine on which [go-rpm-macros](https://pagure.io/go-rpm-macr Building -------- -Building requires linking Go compiler sources inside the project: - -```sh -$ ln -s $(go env GOROOT)/src/cmd/go/internal/ pkg/util/internal -``` +Building requires github.com/urfave/cli, github.com/golang/glog. Licensing --------- diff --git a/pkg/util/util.go b/pkg/util/util.go index e321325..c89eeb9 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -2,6 +2,7 @@ package util import ( "fmt" + "go/build" "go/parser" "go/token" "os" @@ -10,12 +11,6 @@ import ( "regexp" "sort" "strings" - - "pagure.io/golist/pkg/util/internal/load" - "pagure.io/golist/pkg/util/internal/work" - - // initialize the load.ModInit function - "pagure.io/golist/pkg/util/internal/modload" ) func exists(path string) (bool, error) { @@ -54,6 +49,27 @@ func findPackageLocation(packagePath string) (string, string, error) { return abspath, pathPrefix, nil } +// IsStandardImportPath reports whether $GOROOT/src/path should be considered +// part of the standard distribution. For historical reasons we allow people to add +// their own code to $GOROOT instead of using $GOPATH, but we assume that +// code will start with a domain name (dot in the first element). +// +// Note that this function is meant to evaluate whether a directory found in GOROOT +// should be treated as part of the standard library. It should not be used to decide +// that a directory found in GOPATH should be rejected: directories in GOPATH +// need not have dots in the first element, and they just take their chances +// with future collisions in the standard library. +// +// This function was copied from src/cmd/go/internal/search/search.go in Go's code. +func isStandardImportPath(path string) bool { + i := strings.Index(path, "/") + if i < 0 { + i = len(path) + } + elem := path[:i] + return !strings.Contains(elem, ".") +} + // Ignore specifies a set of resources to ignore type Ignore struct { Dirs []string @@ -86,7 +102,7 @@ func (ignore *Ignore) ignore(path string) bool { } type PackageInfoCollector struct { - packageInfos map[string]*load.PackagePublic + packageInfos map[string]*build.Package mainFiles map[string][]string packagePath string ignore *Ignore @@ -104,7 +120,7 @@ type OtherResources struct { func NewPackageInfoCollector(ignore *Ignore, extensions []string) *PackageInfoCollector { return &PackageInfoCollector{ - packageInfos: make(map[string]*load.PackagePublic), + packageInfos: make(map[string]*build.Package), mainFiles: make(map[string][]string), isStdPackages: make(map[string]bool), ignore: ignore, @@ -112,18 +128,36 @@ func NewPackageInfoCollector(ignore *Ignore, extensions []string) *PackageInfoCo } } +func (p *PackageInfoCollector) isStandardPackage(pkg *build.Package) bool { + return pkg.Goroot && pkg.ImportPath != "" && isStandardImportPath(pkg.ImportPath) +} + func (p *PackageInfoCollector) isStandard(pkg string) (bool, error) { if is, exists := p.isStdPackages[pkg]; exists { return is, nil } - pkgInfo, err := listPackage(pkg) + pkgInfo, err := build.Import(pkg, "", build.IgnoreVendor) if err != nil { return false, err } - p.isStdPackages[pkg] = pkgInfo.Standard + p.isStdPackages[pkg] = p.isStandardPackage(pkgInfo) - return pkgInfo.Standard, nil + return p.isStdPackages[pkg], nil +} + +func (p *PackageInfoCollector) resolveImports(baseDir string, imports []string) []string { + var paths []string + for _, importPath := range imports { + importedPackage, err := build.Import(importPath, baseDir, 0) + if err == nil { + paths = append(paths, importedPackage.ImportPath) + } else { + // Just assume we haven't 'go get'd it yet. + paths = append(paths, importPath) + } + } + return paths } func (p *PackageInfoCollector) CollectPackageInfos(packagePath string) error { @@ -176,20 +210,19 @@ func (p *PackageInfoCollector) CollectPackageInfos(packagePath string) error { return nil } - pkgInfo, err := listPackage(relPath) + pkgInfo, err := build.ImportDir(path, build.IgnoreVendor) if err != nil { - if strings.Contains(err.Error(), "no Go files in") { - return nil - } - // TODO(jchaloup): remove later - if strings.Contains(err.Error(), "build constraints exclude all Go files in") { + if _, ok := err.(*build.NoGoError); ok { return nil } panic(err) - return nil } if len(pkgInfo.GoFiles) > 0 || len(pkgInfo.CgoFiles) > 0 { + // Show vendor-expanded paths in listing + pkgInfo.TestImports = p.resolveImports(abspath, pkgInfo.TestImports) + pkgInfo.XTestImports = p.resolveImports(abspath, pkgInfo.XTestImports) + p.packageInfos[relPath] = pkgInfo } @@ -357,9 +390,9 @@ func (p *PackageInfoCollector) CollectProjectDeps(standard bool, skipSelf bool, continue } - pkgInfo, err := listPackage(relPath) + pkgInfo, err := build.Import(relPath, "", build.IgnoreVendor) // assuming the stdlib is always processed properly - if !standard && err == nil && pkgInfo.Standard { + if !standard && err == nil && p.isStandardPackage(pkgInfo) { continue } @@ -411,25 +444,6 @@ func (p *PackageInfoCollector) BuildPackageTree(includeMain bool, tests bool) ([ return entryPoints, nil } -func listPackage(path string) (*load.PackagePublic, error) { - // TODO(jchaloup): more things need to be init most likely - work.BuildInit() - load.ModInit = modload.Init - d := load.PackagesAndErrors([]string{path}) - if d == nil { - return nil, fmt.Errorf("No package listing found for %v", path) - } - - pkg := d[0] - if pkg.Error != nil { - return nil, pkg.Error - } - // Show vendor-expanded paths in listing - pkg.TestImports = pkg.Resolve(pkg.TestImports) - pkg.XTestImports = pkg.Resolve(pkg.XTestImports) - return &pkg.PackagePublic, nil -} - type ProjectData struct { Packages []string `json:"packages"` Dependencies map[string][]string `json:"dependencies"` From 6d5c116c60a666dc268997d4401c9e58f81a17ef Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Jan 31 2019 10:08:11 +0000 Subject: [PATCH 3/4] Replace findPackageLocation by go.build.Import. This searches *all* GOPATH and GOROOT, and returns error message more consistent with regular go tools. --- diff --git a/pkg/util/util.go b/pkg/util/util.go index c89eeb9..63eb608 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -1,7 +1,6 @@ package util import ( - "fmt" "go/build" "go/parser" "go/token" @@ -13,42 +12,6 @@ import ( "strings" ) -func exists(path string) (bool, error) { - _, err := os.Stat(path) - if err == nil { - return true, nil - } - if os.IsNotExist(err) { - return false, nil - } - return true, err -} - -func findPackageLocation(packagePath string) (string, string, error) { - gopath := os.Getenv("GOPATH") - if gopath == "" { - return "", "", fmt.Errorf("GOPATH not set") - } - - var abspath, pathPrefix string - - // first, find the absolute path - for _, gpath := range strings.Split(gopath, ":") { - abspath = path.Join(gpath, "src", packagePath) - - if e, err := exists(abspath); err == nil && e { - pathPrefix = path.Join(gpath, "src") - break - } - } - - if pathPrefix == "" { - return "", "", fmt.Errorf("Path %v not found on %v", packagePath, gopath) - } - - return abspath, pathPrefix, nil -} - // IsStandardImportPath reports whether $GOROOT/src/path should be considered // part of the standard distribution. For historical reasons we allow people to add // their own code to $GOROOT instead of using $GOPATH, but we assume that @@ -161,14 +124,19 @@ func (p *PackageInfoCollector) resolveImports(baseDir string, imports []string) } func (p *PackageInfoCollector) CollectPackageInfos(packagePath string) error { - abspath, pathPrefix, err := findPackageLocation(packagePath) + pkg, err := build.Import(packagePath, "", build.IgnoreVendor) if err != nil { - return err + // Don't worry about lack of Go files; there may be Go files in + // subdirectories. We just want to find the absolute path in + // GOPATH/GOROOT, but return error if not found at all. + if _, ok := err.(*build.NoGoError); !ok { + return err + } } p.otherResources = &OtherResources{} - if err := filepath.Walk(abspath+"/", func(path string, info os.FileInfo, err error) error { + if err := filepath.Walk(pkg.Dir+"/", func(path string, info os.FileInfo, err error) error { if !info.IsDir() { // Cited from https://golang.org/cmd/go/#hdr-Package_lists: // Directory and file names that begin with "." or "_" are ignored @@ -194,7 +162,7 @@ func (p *PackageInfoCollector) CollectPackageInfos(packagePath string) error { return nil } - relPath := path[len(pathPrefix)+1:] + relPath := path[len(pkg.SrcRoot)+1:] if strings.HasSuffix(relPath, "/") { relPath = relPath[:len(relPath)-1] } @@ -220,8 +188,8 @@ func (p *PackageInfoCollector) CollectPackageInfos(packagePath string) error { if len(pkgInfo.GoFiles) > 0 || len(pkgInfo.CgoFiles) > 0 { // Show vendor-expanded paths in listing - pkgInfo.TestImports = p.resolveImports(abspath, pkgInfo.TestImports) - pkgInfo.XTestImports = p.resolveImports(abspath, pkgInfo.XTestImports) + pkgInfo.TestImports = p.resolveImports(pkg.Dir, pkgInfo.TestImports) + pkgInfo.XTestImports = p.resolveImports(pkg.Dir, pkgInfo.XTestImports) p.packageInfos[relPath] = pkgInfo } From 263dcfff17382fb43c241ae46a555f8072576526 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Jan 31 2019 10:08:11 +0000 Subject: [PATCH 4/4] Simplify code duplication in CollectProjectDeps. We can just leave it to the compiler to decide whether unrolling those loops is beneficial. --- diff --git a/pkg/util/util.go b/pkg/util/util.go index 63eb608..136a017 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -317,32 +317,22 @@ func (p *PackageInfoCollector) CollectInstalledResources() ([]string, error) { func (p *PackageInfoCollector) CollectProjectDeps(standard bool, skipSelf bool, tests bool) ([]string, error) { imports := make(map[string]struct{}) - if tests { - for _, info := range p.packageInfos { - for _, item := range info.TestImports { - if item == "C" { - continue - } - if pos := strings.LastIndex(item, "/vendor/"); pos != -1 { - item = item[pos+8:] - } - if _, ok := imports[item]; !ok { - imports[item] = struct{}{} - } - } + for _, info := range p.packageInfos { + var pkgImports []string + if tests { + pkgImports = info.TestImports + } else { + pkgImports = info.Imports } - } else { - for _, info := range p.packageInfos { - for _, item := range info.Imports { - if item == "C" { - continue - } - if pos := strings.LastIndex(item, "/vendor/"); pos != -1 { - item = item[pos+8:] - } - if _, ok := imports[item]; !ok { - imports[item] = struct{}{} - } + for _, item := range pkgImports { + if item == "C" { + continue + } + if pos := strings.LastIndex(item, "/vendor/"); pos != -1 { + item = item[pos+8:] + } + if _, ok := imports[item]; !ok { + imports[item] = struct{}{} } } }