From 6bfc155eea23a0db7227d52e577f3b483b8dc8f3 Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Wed, 27 Apr 2016 11:44:27 -0700 Subject: [PATCH 1/6] Add tests for update situations We'll also number these cases so it's easier to figure out which ones have problems. All new tests should use vendor/ --- update_test.go | 149 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 140 insertions(+), 9 deletions(-) diff --git a/update_test.go b/update_test.go index db263bf..5852984 100644 --- a/update_test.go +++ b/update_test.go @@ -20,7 +20,7 @@ func TestUpdate(t *testing.T) { wdep Godeps werr bool }{ - { // simple case, update one dependency + { // 0 - simple case, update one dependency cwd: "C", args: []string{"D"}, start: []*node{ @@ -55,7 +55,7 @@ func TestUpdate(t *testing.T) { }, }, }, - { // simple case, update one dependency, trailing slash + { // 1 - simple case, update one dependency, trailing slash cwd: "C", args: []string{"D/"}, start: []*node{ @@ -90,7 +90,7 @@ func TestUpdate(t *testing.T) { }, }, }, - { // update one dependency, keep other one, no rewrite + { // 2 - update one dependency, keep other one, no rewrite cwd: "C", args: []string{"D"}, start: []*node{ @@ -138,7 +138,7 @@ func TestUpdate(t *testing.T) { }, }, }, - { // update one dependency, keep other one, with rewrite + { // 3 - update one dependency, keep other one, with rewrite cwd: "C", args: []string{"D"}, start: []*node{ @@ -187,7 +187,7 @@ func TestUpdate(t *testing.T) { }, }, }, - { // update all dependencies + { // 4 - update all dependencies cwd: "C", args: []string{"..."}, start: []*node{ @@ -235,7 +235,7 @@ func TestUpdate(t *testing.T) { }, }, }, - { // one match of two patterns + { // 5 - one match of two patterns cwd: "C", args: []string{"D", "X"}, start: []*node{ @@ -270,7 +270,7 @@ func TestUpdate(t *testing.T) { }, }, }, - { // no matches + { // 6 - no matches cwd: "C", args: []string{"X"}, start: []*node{ @@ -306,7 +306,7 @@ func TestUpdate(t *testing.T) { }, werr: true, }, - { // update just one package of two in a repo skips it + { // 7 - update just one package of two in a repo skips it cwd: "C", args: []string{"D/A", "E"}, start: []*node{ @@ -359,7 +359,7 @@ func TestUpdate(t *testing.T) { }, }, }, - { // update just one package of two in a repo, none left + { // 8 - update just one package of two in a repo, none left cwd: "C", args: []string{"D/A"}, start: []*node{ @@ -400,6 +400,134 @@ func TestUpdate(t *testing.T) { }, werr: true, }, + { // 9 - package/..., just version bump + vendor: true, + cwd: "C", + args: []string{"D/..."}, + start: []*node{ + { + "D", + "", + []*node{ + {"A/main.go", pkg("A") + decl("D1"), nil}, + {"B/main.go", pkg("B") + decl("D1"), nil}, + {"+git", "D1", nil}, + {"A/main.go", pkg("A") + decl("D2"), nil}, + {"B/main.go", pkg("B") + decl("D2"), nil}, + {"+git", "D2", nil}, + }, + }, + { + "C", + "", + []*node{ + {"main.go", pkg("main", "D/A", "D/B"), nil}, + {"Godeps/Godeps.json", godeps("C", "D/A", "D1", "D/B", "D1"), nil}, + {"vendor/D/A/main.go", pkg("A") + decl("D1"), nil}, + {"vendor/D/B/main.go", pkg("B") + decl("D1"), nil}, + {"+git", "", nil}, + }, + }, + }, + want: []*node{ + {"C/vendor/D/A/main.go", pkg("A") + decl("D2"), nil}, + {"C/vendor/D/B/main.go", pkg("B") + decl("D2"), nil}, + }, + wdep: Godeps{ + ImportPath: "C", + Deps: []Dependency{ + {ImportPath: "D/A", Comment: "D2"}, + {ImportPath: "D/B", Comment: "D2"}, + }, + }, + }, + { // 10 - package/..., new unrelated package that's not imported + vendor: true, + cwd: "C", + args: []string{"D/..."}, + start: []*node{ + { + "D", + "", + []*node{ + {"A/main.go", pkg("A") + decl("D1"), nil}, + {"B/main.go", pkg("B") + decl("D1"), nil}, + {"+git", "D1", nil}, + {"A/main.go", pkg("A") + decl("D2"), nil}, + {"B/main.go", pkg("B") + decl("D2"), nil}, + {"E/main.go", pkg("E") + decl("D2"), nil}, + {"+git", "D2", nil}, + }, + }, + { + "C", + "", + []*node{ + {"main.go", pkg("main", "D/A", "D/B"), nil}, + {"Godeps/Godeps.json", godeps("C", "D/A", "D1", "D/B", "D1"), nil}, + {"vendor/D/A/main.go", pkg("A") + decl("D1"), nil}, + {"vendor/D/B/main.go", pkg("B") + decl("D1"), nil}, + {"+git", "", nil}, + }, + }, + }, + want: []*node{ + {"C/vendor/D/A/main.go", pkg("A") + decl("D2"), nil}, + {"C/vendor/D/B/main.go", pkg("B") + decl("D2"), nil}, + {"C/vendor/D/E/main.go", "(absent)", nil}, + }, + wdep: Godeps{ + ImportPath: "C", + Deps: []Dependency{ + {ImportPath: "D/A", Comment: "D2"}, + {ImportPath: "D/B", Comment: "D2"}, + }, + }, + }, + { // 10 - package/..., new transitive package + vendor: true, + cwd: "C", + args: []string{"D/..."}, + start: []*node{ + { + "D", + "", + []*node{ + {"A/main.go", pkg("A") + decl("D1"), nil}, + {"B/main.go", pkg("B") + decl("D1"), nil}, + {"+git", "D1", nil}, + {"A/main.go", pkg("A") + decl("D2"), nil}, + {"B/main.go", pkg("B", "D/E") + decl("D2"), nil}, + {"E/main.go", pkg("E") + decl("D2"), nil}, + {"+git", "D2", nil}, + }, + }, + { + "C", + "", + []*node{ + {"main.go", pkg("main", "D/A", "D/B"), nil}, + {"Godeps/Godeps.json", godeps("C", "D/A", "D1", "D/B", "D1"), nil}, + {"vendor/D/A/main.go", pkg("A") + decl("D1"), nil}, + {"vendor/D/B/main.go", pkg("B") + decl("D1"), nil}, + {"+git", "", nil}, + }, + }, + }, + want: []*node{ + {"C/vendor/D/A/main.go", pkg("A") + decl("D2"), nil}, + {"C/vendor/D/B/main.go", pkg("B", "D/E") + decl("D2"), nil}, + {"C/vendor/D/E/main.go", pkg("E") + decl("D2"), nil}, + }, + wdep: Godeps{ + ImportPath: "C", + Deps: []Dependency{ + {ImportPath: "D/A", Comment: "D2"}, + {ImportPath: "D/B", Comment: "D2"}, + {ImportPath: "D/E", Comment: "D2"}, + }, + }, + }, } wd, err := os.Getwd() @@ -426,6 +554,9 @@ func TestUpdate(t *testing.T) { log.SetOutput(ioutil.Discard) err = update(test.args) log.SetOutput(os.Stderr) + if err != nil { + t.Log(pos, "Err:", err) + } if g := err != nil; g != test.werr { t.Errorf("update err = %v (%v) want %v", g, err, test.werr) } From 8749230729cb47ff0ac6181f832feb9da8224bd1 Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Wed, 27 Apr 2016 16:58:31 -0700 Subject: [PATCH 2/6] godep update golang.org/x/tools/go/vcs --- Godeps/Godeps.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index b88324a..9bd89be 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -22,7 +22,7 @@ }, { "ImportPath": "golang.org/x/tools/go/vcs", - "Rev": "3f1f7eeff104d9ef829677f12b9780dfdd26a96a" + "Rev": "1f1b3322f67af76803c942fd237291538ec68262" } ] } From 0c3b66ef1422545d4624592717e853f73f35abc6 Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Fri, 29 Apr 2016 17:20:28 -0700 Subject: [PATCH 3/6] Scan through new transitive deps and add them There are cases where an updated package will need additional packages, so handle that. --- godepfile.go | 20 ++++++++ list.go | 34 ++++++++++++- pkg.go | 4 +- restore.go | 2 +- save.go | 3 ++ save_test.go | 1 + update.go | 129 ++++++++++++++++++++++++++++++------------------- update_test.go | 2 +- 8 files changed, 140 insertions(+), 55 deletions(-) diff --git a/godepfile.go b/godepfile.go index 468f49a..fd084aa 100644 --- a/godepfile.go +++ b/godepfile.go @@ -185,3 +185,23 @@ func (g *Godeps) writeTo(w io.Writer) (int64, error) { n, err := w.Write(append(b, '\n')) return int64(n), err } + +func (g *Godeps) addOrUpdateDeps(deps []Dependency) { + var missing []Dependency + for _, d := range deps { + var found bool + for i := range g.Deps { + if g.Deps[i].ImportPath == d.ImportPath { + g.Deps[i] = d + found = true + break + } + } + if !found { + missing = append(missing, d) + } + } + for _, d := range missing { + g.Deps = append(g.Deps, d) + } +} diff --git a/list.go b/list.go index 2295af0..09d8b80 100644 --- a/list.go +++ b/list.go @@ -164,6 +164,7 @@ func listPackage(path string) (*Package, error) { p.Imports = append(p.Imports, dp.ImportPath) } p.Deps = append(p.Deps, dp.ImportPath) + p.Dependencies = addDependency(p.Dependencies, dp) } p.Imports = uniq(p.Imports) p.Deps = uniq(p.Deps) @@ -172,6 +173,15 @@ func listPackage(path string) (*Package, error) { return p, nil } +func addDependency(deps []build.Package, d *build.Package) []build.Package { + for i := range deps { + if deps[i].Dir == d.Dir { + return deps + } + } + return append(deps, *d) +} + // 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) @@ -206,7 +216,7 @@ func findDirForPath(path string, ip *build.Package) (string, error) { for _, dir := range search { debugln("searching", dir) - fi, err := os.Stat(dir) + fi, err := stat(dir) if err == nil && fi.IsDir() { return dir, nil } @@ -215,6 +225,28 @@ func findDirForPath(path string, ip *build.Package) (string, error) { return "", errPackageNotFound{path} } +type statEntry struct { + fi os.FileInfo + err error +} + +var ( + statCache = make(map[string]statEntry) +) + +func clearStatCache() { + statCache = make(map[string]statEntry) +} + +func stat(p string) (os.FileInfo, error) { + if e, ok := statCache[p]; ok { + return e.fi, e.err + } + fi, err := os.Stat(p) + statCache[p] = statEntry{fi, err} + return fi, err +} + // fillPackage full of info. Assumes p.Dir is set at a minimum func fillPackage(p *build.Package) error { if p.Goroot { diff --git a/pkg.go b/pkg.go index 8fbdaa2..4dd5557 100644 --- a/pkg.go +++ b/pkg.go @@ -1,6 +1,7 @@ package main import ( + "go/build" "regexp" "strings" ) @@ -28,7 +29,8 @@ type Package struct { } // --- New stuff for now - Imports []string + Imports []string + Dependencies []build.Package } // LoadPackages loads the named packages using go list -json. diff --git a/restore.go b/restore.go index eb18331..847d6d9 100644 --- a/restore.go +++ b/restore.go @@ -83,7 +83,7 @@ func runRestore(cmd *Command, args []string) { var downloaded = make(map[string]bool) -// download downloads the given dependency. +// download the given dependency. // 2 Passes: 1) go get -d , 2) git pull (if necessary) func download(dep *Dependency) error { diff --git a/save.go b/save.go index 574dd36..23ed6a4 100644 --- a/save.go +++ b/save.go @@ -365,6 +365,9 @@ func copySrc(dir string, deps []Dependency) error { debugln("copySrc for", dep.ImportPath) srcdir := filepath.Join(dep.ws, "src") rel, err := filepath.Rel(srcdir, dep.dir) + debugln("srcdir", srcdir) + debugln("rel", rel) + debugln("err", err) if err != nil { // this should never happen return err } diff --git a/save_test.go b/save_test.go index 1979ec8..3fb8d5d 100644 --- a/save_test.go +++ b/save_test.go @@ -89,6 +89,7 @@ func godeps(importpath string, keyval ...string) *Godeps { func setGlobals(vendor bool) { clearPkgCache() + clearStatCache() VendorExperiment = vendor sep = defaultSep(VendorExperiment) //debug = testing.Verbose() diff --git a/update.go b/update.go index cfd0aa1..7002b17 100644 --- a/update.go +++ b/update.go @@ -101,6 +101,7 @@ func update(args []string) error { if len(deps) == 0 { return errorNoPackagesUpdatable } + g.addOrUpdateDeps(deps) if _, err = g.save(); err != nil { return err } @@ -172,81 +173,107 @@ func markMatches(pat string, deps []Dependency) (matched bool) { return matched } +func fillDeps(deps []Dependency) ([]Dependency, error) { + for i := range deps { + if deps[i].pkg != nil { + continue + } + ps, err := LoadPackages(deps[i].ImportPath) + if err != nil { + return nil, err + } + if len(ps) > 1 { + panic("More than one package found for " + deps[i].ImportPath) + } + p := ps[0] + deps[i].pkg = p + deps[i].dir = p.Dir + deps[i].ws = p.Root + + vcs, reporoot, err := VCSFromDir(p.Dir, filepath.Join(p.Root, "src")) + if err != nil { + return nil, errorLoadingDeps + } + deps[i].root = filepath.ToSlash(reporoot) + deps[i].vcs = vcs + } + + return deps, nil +} + // LoadVCSAndUpdate loads and updates a set of dependencies. func LoadVCSAndUpdate(deps []Dependency) ([]Dependency, error) { var err1 error - var paths []string - for _, dep := range deps { - paths = append(paths, dep.ImportPath) - } - ps, err := LoadPackages(paths...) + + deps, err := fillDeps(deps) if err != nil { return nil, err } - noupdate := make(map[string]bool) // repo roots - var candidates []*Dependency - var tocopy []Dependency + + repoMask := make(map[string]bool) for i := range deps { - dep := &deps[i] - for _, pkg := range ps { - if dep.ImportPath == pkg.ImportPath { - dep.pkg = pkg - break - } - } - if dep.pkg == nil { - log.Println(dep.ImportPath + ": error listing package") - err1 = errorLoadingDeps - continue + if !deps[i].matched { + repoMask[deps[i].root] = true } - if dep.pkg.Error.Err != "" { - log.Println(dep.pkg.Error.Err) - err1 = errorLoadingDeps - continue - } - vcs, reporoot, err := VCSFromDir(dep.pkg.Dir, filepath.Join(dep.pkg.Root, "src")) - if err != nil { - log.Println(err) - err1 = errorLoadingDeps + } + + // Determine if we need any new packages because of new transitive imports + for _, dep := range deps { + if !dep.matched { continue } - dep.dir = dep.pkg.Dir - dep.ws = dep.pkg.Root - dep.root = filepath.ToSlash(reporoot) - dep.vcs = vcs - if dep.matched { - candidates = append(candidates, dep) - } else { - noupdate[dep.root] = true + for _, dp := range dep.pkg.Dependencies { + if dp.Goroot { + continue + } + var have bool + for _, d := range deps { + if d.ImportPath == dp.ImportPath { + have = true + break + } + } + if !have { + deps = append(deps, Dependency{ImportPath: dp.ImportPath, matched: true}) + } } } - if err1 != nil { - return nil, err1 + + deps, err = fillDeps(deps) + if err != nil { + return nil, err } - for _, dep := range candidates { - dep.dir = dep.pkg.Dir - dep.ws = dep.pkg.Root - if noupdate[dep.root] { + var toUpdate []Dependency + for _, d := range deps { + if !d.matched || repoMask[d.root] { continue } - id, err := dep.vcs.identify(dep.pkg.Dir) + toUpdate = append(toUpdate, d) + } + debugln("toUpdate") + ppln(toUpdate) + + var toCopy []Dependency + for _, d := range toUpdate { + id, err := d.vcs.identify(d.dir) if err != nil { log.Println(err) err1 = errorLoadingDeps continue } - if dep.vcs.isDirty(dep.pkg.Dir, id) { - log.Println("dirty working tree (please commit changes):", dep.pkg.Dir) - err1 = errorLoadingDeps - break + if d.vcs.isDirty(d.dir, id) { + log.Println("dirty working tree (please commit changes):", d.dir) } - dep.Rev = id - dep.Comment = dep.vcs.describe(dep.pkg.Dir, id) - tocopy = append(tocopy, *dep) + d.Rev = id + d.Comment = d.vcs.describe(d.dir, id) + toCopy = append(toCopy, d) } + debugln("toCopy") + ppln(toCopy) + if err1 != nil { return nil, err1 } - return tocopy, nil + return toCopy, nil } diff --git a/update_test.go b/update_test.go index 5852984..84a5065 100644 --- a/update_test.go +++ b/update_test.go @@ -484,7 +484,7 @@ func TestUpdate(t *testing.T) { }, }, }, - { // 10 - package/..., new transitive package + { // 11 - package/..., new transitive package vendor: true, cwd: "C", args: []string{"D/..."}, From 6b9bcf4a0a234c52835c92019cb883e870ec4c7b Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Tue, 3 May 2016 10:57:22 -0700 Subject: [PATCH 4/6] vscode stuff --- .vscode/settings.json | 3 +++ .vscode/tasks.json | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 .vscode/settings.json create mode 100644 .vscode/tasks.json diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..20af2f6 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +// Place your settings in this file to overwrite default and user settings. +{ +} \ No newline at end of file diff --git a/.vscode/tasks.json b/.vscode/tasks.json new file mode 100644 index 0000000..2b7845e --- /dev/null +++ b/.vscode/tasks.json @@ -0,0 +1,16 @@ +{ + // See http://go.microsoft.com/fwlink/?LinkId=733558 + // for the documentation about the tasks.json format + "version": "0.1.0", + "command": "go", + "isShellCommand": true, + "args": ["test"], + "showOutput": "always", + "tasks": [ + { + "taskName":"Local Test", + "args":[ "-v", "-run=Update"], + "suppressTaskName": true + } + ] +} \ No newline at end of file From 8ee9abeea613ae1b583efc63f71eeb8cce32476c Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Mon, 9 May 2016 13:16:01 -0700 Subject: [PATCH 5/6] Differentiate between repos Handling can be different if the new transitive dep is in the same or different repo, so test that too. --- update_test.go | 53 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/update_test.go b/update_test.go index 84a5065..e01e3d0 100644 --- a/update_test.go +++ b/update_test.go @@ -484,7 +484,7 @@ func TestUpdate(t *testing.T) { }, }, }, - { // 11 - package/..., new transitive package + { // 11 - package/..., new transitive package, same repo vendor: true, cwd: "C", args: []string{"D/..."}, @@ -528,6 +528,57 @@ func TestUpdate(t *testing.T) { }, }, }, + { // 11 - package/..., new transitive package, different repo + vendor: true, + cwd: "C", + args: []string{"D/..."}, + start: []*node{ + { + "D", + "", + []*node{ + {"A/main.go", pkg("A") + decl("D1"), nil}, + {"B/main.go", pkg("B") + decl("D1"), nil}, + {"+git", "D1", nil}, + {"A/main.go", pkg("A") + decl("D2"), nil}, + {"B/main.go", pkg("B", "E") + decl("D2"), nil}, + {"+git", "D2", nil}, + }, + }, + { + "E", + "", + []*node{ + {"main.go", pkg("E") + decl("E1"), nil}, + {"+git", "E1", nil}, + }, + }, + { + "C", + "", + []*node{ + {"main.go", pkg("main", "D/A", "D/B"), nil}, + {"Godeps/Godeps.json", godeps("C", "D/A", "D1", "D/B", "D1"), nil}, + {"vendor/D/A/main.go", pkg("A") + decl("D1"), nil}, + {"vendor/D/B/main.go", pkg("B") + decl("D1"), nil}, + {"+git", "", nil}, + }, + }, + }, + want: []*node{ + {"C/vendor/D/A/main.go", pkg("A") + decl("D2"), nil}, + {"C/vendor/D/B/main.go", pkg("B", "E") + decl("D2"), nil}, + {"C/vendor/E/main.go", pkg("E") + decl("E1"), nil}, + }, + wdep: Godeps{ + ImportPath: "C", + Deps: []Dependency{ + {ImportPath: "D/A", Comment: "D2"}, + {ImportPath: "D/B", Comment: "D2"}, + {ImportPath: "E", Comment: "E1"}, + }, + }, + }, } wd, err := os.Getwd() From c06d9386fa227cbec5571c5c51c9c80fa0d876e5 Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Mon, 9 May 2016 14:26:40 -0700 Subject: [PATCH 6/6] Bump version --- Changelog.md | 4 ++++ version.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Changelog.md b/Changelog.md index 311e6ac..8d6bd52 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,7 @@ +#v65 (2016/06/09) + +* Rewrite update so that it considers new transitive dependencies, both in the same repo and outside of it. + #v64 (2016/06/09) * godep update golang.org/x/tools/go/vcs diff --git a/version.go b/version.go index 9650c01..dcd6b74 100644 --- a/version.go +++ b/version.go @@ -8,7 +8,7 @@ import ( "strings" ) -const version = 64 +const version = 65 var cmdVersion = &Command{ Name: "version",