diff --git a/Changelog.md b/Changelog.md index d872c7b..03f08f1 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,16 @@ +# v42 2015/12/22 + +* Fix a bunch of GO15VENDOREXPERIMENT issues + * Find package directories better. Previously we used build.FindOnly which didn't work the way I expected it to (any dir would work w/o error). + * Set the VendorExperiment bool based on go version as 1.6 defaults to on. + * A bunch of extra debugging for use while sanity checking myself. + * vendor flag for test structs. + * Some tests for vendor/ stuff: + * Basic Test + * Transitive + * Transitive, across GOPATHs + collapse vendor/ directories. +* Should Fix #358 + # v41 2015/12/17 * Don't rewrite packages outside of the project. This would happen if you specified diff --git a/errors.go b/errors.go index 6fbf94e..445682c 100644 --- a/errors.go +++ b/errors.go @@ -8,3 +8,11 @@ var ( errorCopyingSourceCode = errors.New("error copying source code") errorNoPackagesUpdatable = errors.New("no packages can be updated") ) + +type errPackageNotFound struct { + path string +} + +func (e errPackageNotFound) Error() string { + return "Package (" + e.path + ") not found" +} diff --git a/godepfile.go b/godepfile.go index d330398..aa16e30 100644 --- a/godepfile.go +++ b/godepfile.go @@ -54,6 +54,8 @@ func loadDefaultGodepsFile() (Godeps, error) { // pkgs is the list of packages to read dependencies for func (g *Godeps) fill(pkgs []*Package, destImportPath string) error { + debugln("fill", destImportPath) + ppln(pkgs) var err1 error var path, testImports []string for _, p := range pkgs { @@ -87,10 +89,12 @@ func (g *Godeps) fill(pkgs []*Package, destImportPath string) error { path = append(path, p.ImportPath) path = append(path, p.Deps...) } + debugln("path", path) for i, p := range path { path[i] = unqualify(p) } path = uniq(path) + debugln("uniq, unqualify'd path", path) ps, err = LoadPackages(path...) if err != nil { return err diff --git a/list.go b/list.go index bb12ffe..eda9512 100644 --- a/list.go +++ b/list.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "go/build" "go/parser" @@ -104,27 +105,11 @@ func fullPackageInDir(dir string) (*build.Package, error) { // listPackage specified by path func listPackage(path string) (*Package, error) { - var dir string + debugln("listPackage", path) var lp *build.Package - var err error - if build.IsLocalImport(path) { - dir = path - if !filepath.IsAbs(dir) { - if abs, err := filepath.Abs(dir); err == nil { - // interpret relative to current directory - dir = abs - } - } - } else { - dir, err = os.Getwd() - if err != nil { - return nil, err - } - lp, err = build.Import(path, dir, build.FindOnly) - if err != nil { - return nil, err - } - dir = lp.Dir + dir, err := findDirForPath(path, nil) + if err != nil { + return nil, err } lp, err = fullPackageInDir(dir) p := &Package{ @@ -152,32 +137,11 @@ func listPackage(path string) (*Package, error) { ip, i := ds.Next() debugf("Processing import %s for %s\n", i, ip.Dir) - // We need to check to see if the import exists in vendor/ folders up the hierachy of the importing package, - var dp *build.Package - if VendorExperiment && !ip.Goroot { - for base := ip.Dir; base != ip.Root; base = filepath.Dir(base) { - vdir := filepath.Join(base, "vendor", i) - debugln("checking for vendor dir:", vdir) - dp, err = build.ImportDir(vdir, build.FindOnly) - if err != nil { - if os.IsNotExist(err) { - continue - } - debugln(err.Error()) - ppln(err) - panic("Unknown error attempt to find vendor/") - } - goto Found - } - } - // Wasn't found above, so resolve it using the build.Context - dp, err = build.Import(i, ip.Dir, build.FindOnly) + pdir, err := findDirForPath(i, ip) if err != nil { - ppln(err) - return nil, errorMissingDep{i: i, dir: ip.Dir} + return nil, err } - Found: - dp, err = fullPackageInDir(dp.Dir) + dp, err := fullPackageInDir(pdir) if err != nil { // This really should happen in this context though ppln(err) return nil, errorMissingDep{i: i, dir: ip.Dir} @@ -199,18 +163,75 @@ func listPackage(path string) (*Package, error) { } p.Imports = uniq(p.Imports) p.Deps = uniq(p.Deps) - debugln("Looking For Package:", path, "in", dir) + debugln("Done Looking For Package:", path, "in", dir) ppln(p) return p, nil } -// fillPackage full of info. Assumes a build.Package discovered in build.FindOnly mode +// finds the directory for the given import path in the context of the provided build.Package (if provided) +func findDirForPath(path string, ip *build.Package) (string, error) { + debugln("findDirForPath", path, ip) + var search []string + + if build.IsLocalImport(path) { + dir := path + if !filepath.IsAbs(dir) { + if abs, err := filepath.Abs(dir); err == nil { + // interpret relative to current directory + dir = abs + } + } + return dir, nil + } + + // We need to check to see if the import exists in vendor/ folders up the hierachy of the importing package + if VendorExperiment && ip != nil { + debugln("resolving vendor posibilities:", ip.Dir, ip.Root) + for base := ip.Dir; base != ip.Root; base = filepath.Dir(base) { + s := filepath.Join(base, "vendor", path) + debugln("Adding search dir:", s) + search = append(search, s) + } + } + + for _, base := range build.Default.SrcDirs() { + search = append(search, filepath.Join(base, path)) + } + + for _, dir := range search { + debugln("searching", dir) + fi, err := os.Stat(dir) + if err == nil && fi.IsDir() { + return dir, nil + } + } + + return "", errPackageNotFound{path} +} + +// fillPackage full of info. Assumes p.Dir is set at a minimum func fillPackage(p *build.Package) error { if p.Goroot { return nil } + if p.SrcRoot == "" { + for _, base := range build.Default.SrcDirs() { + if strings.HasPrefix(p.Dir, base) { + p.SrcRoot = base + } + } + } + + if p.SrcRoot == "" { + return errors.New("Unable to find SrcRoot for package " + p.ImportPath) + } + + if p.Root == "" { + p.Root = filepath.Dir(p.SrcRoot) + } + var buildMatch = "+build " var buildFieldSplit = func(r rune) bool { return unicode.IsSpace(r) || r == ',' diff --git a/main.go b/main.go index 6764187..0ed9c1c 100644 --- a/main.go +++ b/main.go @@ -12,9 +12,12 @@ import ( ) var ( - cpuprofile string - verbose bool // Verbose flag for commands that support it - debug bool // Debug flag for commands that support it + cpuprofile string + verbose bool // Verbose flag for commands that support it + debug bool // Debug flag for commands that support it + majorGoVersion string + VendorExperiment bool + sep string ) // Command is an implementation of a godep command @@ -77,6 +80,21 @@ func main() { return } + var err error + majorGoVersion, err = goVersion() + if err != nil { + log.Fatal(err) + } + + // VendorExperiment is the Go 1.5 vendor directory experiment flag, see + // https://github.com/golang/go/commit/183cc0cd41f06f83cb7a2490a499e3f9101befff + VendorExperiment = (majorGoVersion == "go1.5" && os.Getenv("GO15VENDOREXPERIMENT") == "1") || + (majorGoVersion == "go1.6" && os.Getenv("GO15VENDOREXPERIMENT") != "0") + + // sep is the signature set of path elements that + // precede the original path of an imported package. + sep = defaultSep(VendorExperiment) + for _, cmd := range commands { if cmd.Name == args[0] { cmd.Flag.BoolVar(&verbose, "v", false, "enable verbose output") @@ -84,6 +102,11 @@ func main() { cmd.Flag.StringVar(&cpuprofile, "cpuprofile", "", "Write cpu profile to this file") cmd.Flag.Usage = func() { cmd.UsageExit() } cmd.Flag.Parse(args[1:]) + + debugln("majorGoVersion", majorGoVersion) + debugln("VendorExperiment", VendorExperiment) + debugln("sep", sep) + if cpuprofile != "" { f, err := os.Create(cpuprofile) if err != nil { diff --git a/pkg.go b/pkg.go index c6f26cd..8fbdaa2 100644 --- a/pkg.go +++ b/pkg.go @@ -38,6 +38,7 @@ type Package struct { // Files with a build tag of `ignore` are skipped. Files with other build tags // are however processed. func LoadPackages(names ...string) (a []*Package, err error) { + debugln("LoadPackages", names) if len(names) == 0 { return nil, nil } diff --git a/rewrite.go b/rewrite.go index 7f26423..c1b6de2 100644 --- a/rewrite.go +++ b/rewrite.go @@ -21,6 +21,7 @@ import ( // according to the rules for func qualify. func rewrite(pkgs []*Package, qual string, paths []string) error { for _, path := range pkgFiles(pkgs) { + debugln("rewrite", path) err := rewriteTree(path, qual, paths) if err != nil { return err @@ -51,17 +52,14 @@ func rewriteTree(path, qual string, paths []string) error { continue } s := w.Stat() - switch s.IsDir() { - case true: - if s.Name() == "testdata" { - w.SkipDir() - } - case false: - if strings.HasSuffix(w.Path(), ".go") { - err := rewriteGoFile(w.Path(), qual, paths) - if err != nil { - return err - } + if s.IsDir() && s.Name() == "testdata" { + w.SkipDir() + continue + } + if strings.HasSuffix(w.Path(), ".go") { + err := rewriteGoFile(w.Path(), qual, paths) + if err != nil { + return err } } } @@ -71,6 +69,7 @@ func rewriteTree(path, qual string, paths []string) error { // rewriteGoFile rewrites import statments in the named file // according to the rules for func qualify. func rewriteGoFile(name, qual string, paths []string) error { + debugln("rewriteGoFile", name, ",", qual, ",", paths) printerConfig := &printer.Config{Mode: printer.TabIndent | printer.UseSpaces, Tabwidth: 8} fset := token.NewFileSet() f, err := parser.ParseFile(fset, name, nil, parser.ParseComments) @@ -118,14 +117,6 @@ func rewriteGoFile(name, qual string, paths []string) error { return os.Rename(tpath, name) } -// VendorExperiment is the Go 1.5 vendor directory experiment flag, see -// https://github.com/golang/go/commit/183cc0cd41f06f83cb7a2490a499e3f9101befff -var VendorExperiment = os.Getenv("GO15VENDOREXPERIMENT") == "1" - -// sep is the signature set of path elements that -// precede the original path of an imported package. -var sep = defaultSep(VendorExperiment) - func defaultSep(experiment bool) string { if experiment { return "/vendor/" diff --git a/rewrite_test.go b/rewrite_test.go index dfc5514..60fef5e 100644 --- a/rewrite_test.go +++ b/rewrite_test.go @@ -7,6 +7,7 @@ import ( ) func TestUnqualify(t *testing.T) { + setGlobals(false) var cases = []struct { path string want string @@ -243,7 +244,7 @@ func TestRewrite(t *testing.T) { const gopath = "godeptest" defer os.RemoveAll(gopath) for pos, test := range cases { - clearPkgCache() + setGlobals(false) err := os.RemoveAll(gopath) if err != nil { t.Fatal(err) diff --git a/save.go b/save.go index 865ce36..3138219 100644 --- a/save.go +++ b/save.go @@ -142,15 +142,19 @@ func save(pkgs []string) error { if err != nil { return err } - debugln("LoadPackages", pkgs) + debugln("Project Packages", pkgs) ppln(a) projA := projectPackages(dp.Dir, a) + debugln("Filtered projectPackages") + ppln(projA) + err = gnew.fill(a, dp.ImportPath) if err != nil { return err } debugln("New Godeps Filled") + ppln(gnew) if gnew.Deps == nil { gnew.Deps = make([]Dependency, 0) // produce json [], not null @@ -160,6 +164,7 @@ func save(pkgs []string) error { if err != nil { return err } + if gold.isOldFile { // If we are migrating from an old format file, // we require that the listed version of every @@ -186,7 +191,11 @@ func save(pkgs []string) error { // godep go list ./... srcdir := filepath.FromSlash(strings.Trim(sep, "/")) rem := subDeps(gold.Deps, gnew.Deps) + debugln("Deps to remove") + ppln(rem) add := subDeps(gnew.Deps, gold.Deps) + debugln("Deps to add") + ppln(add) err = removeSrc(srcdir, rem) if err != nil { return err @@ -205,6 +214,8 @@ func save(pkgs []string) error { rewritePaths = append(rewritePaths, dep.ImportPath) } } + debugln("rewritePaths") + ppln(rewritePaths) return rewrite(projA, dp.ImportPath, rewritePaths) } diff --git a/save_test.go b/save_test.go index dbfd0fe..ea64346 100644 --- a/save_test.go +++ b/save_test.go @@ -84,12 +84,21 @@ func godeps(importpath string, keyval ...string) *Godeps { return g } +func setGlobals(vendor bool) { + clearPkgCache() + VendorExperiment = vendor + sep = defaultSep(VendorExperiment) + //debug = testing.Verbose() + //verbose = testing.Verbose() +} + func TestSave(t *testing.T) { var cases = []struct { cwd string args []string flagR bool flagT bool + vendor bool start []*node altstart []*node want []*node @@ -97,7 +106,8 @@ func TestSave(t *testing.T) { werr bool }{ { // 0 - simple case, one dependency - cwd: "C", + cwd: "C", + flagR: false, start: []*node{ { "C", @@ -1317,6 +1327,126 @@ func TestSave(t *testing.T) { }, }, }, + { // 34 - vendor (#1) on, simple case, one dependency + vendor: true, + cwd: "C", + start: []*node{ + { + "C", + "", + []*node{ + {"main.go", pkg("main", "D"), nil}, + {"+git", "", nil}, + }, + }, + { + "D", + "", + []*node{ + {"main.go", pkg("D"), nil}, + {"+git", "D1", nil}, + }, + }, + }, + want: []*node{ + {"C/main.go", pkg("main", "D"), nil}, + {"C/vendor/D/main.go", pkg("D"), nil}, + }, + wdep: Godeps{ + ImportPath: "C", + Deps: []Dependency{ + {ImportPath: "D", Comment: "D1"}, + }, + }, + }, + { // 35 - vendor (#4) transitive dependency + vendor: true, + cwd: "C", + start: []*node{ + { + "C", + "", + []*node{ + {"main.go", pkg("main", "D"), nil}, + {"+git", "", nil}, + }, + }, + { + "D", + "", + []*node{ + {"main.go", pkg("D", "T"), nil}, + {"+git", "D1", nil}, + }, + }, + { + "T", + "", + []*node{ + {"main.go", pkg("T"), nil}, + {"+git", "T1", nil}, + }, + }, + }, + want: []*node{ + {"C/main.go", pkg("main", "D"), nil}, + {"C/vendor/D/main.go", pkg("D", "T"), nil}, + {"C/vendor/T/main.go", pkg("T"), nil}, + }, + wdep: Godeps{ + ImportPath: "C", + Deps: []Dependency{ + {ImportPath: "D", Comment: "D1"}, + {ImportPath: "T", Comment: "T1"}, + }, + }, + }, + { // 36 vendor (#21) find transitive dependencies across roots + vendor: true, + cwd: "C", + altstart: []*node{ + { + "T", + "", + []*node{ + {"main.go", pkg("T"), nil}, + {"+git", "T1", nil}, + }, + }, + }, + start: []*node{ + { + "C", + "", + []*node{ + {"main.go", pkg("main", "D"), nil}, + {"+git", "", nil}, + }, + }, + { + "D", + "", + []*node{ + {"main.go", pkg("D", "T"), nil}, + {"vendor/T/main.go", pkg("T"), nil}, + {"Godeps/Godeps.json", godeps("D", "T", "T1"), nil}, + {"+git", "D1", nil}, + }, + }, + }, + want: []*node{ + {"C/main.go", pkg("main", "D"), nil}, + {"C/vendor/D/main.go", pkg("D", "T"), nil}, + {"C/vendor/T/main.go", pkg("T"), nil}, + }, + wdep: Godeps{ + ImportPath: "C", + Deps: []Dependency{ + {ImportPath: "D", Comment: "D1"}, + {ImportPath: "T", Comment: "T1"}, + }, + }, + }, } wd, err := os.Getwd() @@ -1326,7 +1456,8 @@ func TestSave(t *testing.T) { const scratch = "godeptest" defer os.RemoveAll(scratch) for pos, test := range cases { - clearPkgCache() + setGlobals(test.vendor) + err = os.RemoveAll(scratch) if err != nil { t.Fatal(err) @@ -1337,7 +1468,6 @@ func TestSave(t *testing.T) { } src := filepath.Join(scratch, "r1", "src") makeTree(t, &node{src, "", test.start}, altsrc) - dir := filepath.Join(wd, src, test.cwd) err = os.Chdir(dir) if err != nil { diff --git a/update_test.go b/update_test.go index 7810f13..db263bf 100644 --- a/update_test.go +++ b/update_test.go @@ -12,12 +12,13 @@ import ( func TestUpdate(t *testing.T) { var cases = []struct { - cwd string - args []string - start []*node - want []*node - wdep Godeps - werr bool + cwd string + args []string + vendor bool + start []*node + want []*node + wdep Godeps + werr bool }{ { // simple case, update one dependency cwd: "C", @@ -408,7 +409,7 @@ func TestUpdate(t *testing.T) { const gopath = "godeptest" defer os.RemoveAll(gopath) for pos, test := range cases { - clearPkgCache() + setGlobals(test.vendor) err = os.RemoveAll(gopath) if err != nil { t.Fatal(err) diff --git a/version.go b/version.go index c0fde74..53535b7 100644 --- a/version.go +++ b/version.go @@ -5,7 +5,7 @@ import ( "runtime" ) -const version = 41 +const version = 42 var cmdVersion = &Command{ Name: "version",