From 9c285d6be470b0435c8d56c098f8cc596dd5958e Mon Sep 17 00:00:00 2001 From: Sunny Date: Fri, 9 Jun 2017 23:37:01 +0530 Subject: [PATCH 1/4] Implement godep_importer --- cmd/dep/godep_importer.go | 169 +++++++++++ cmd/dep/godep_importer_test.go | 262 ++++++++++++++++++ cmd/dep/testdata/godep/Godeps.json | 16 ++ .../testdata/godep/expected_import_output.txt | 6 + 4 files changed, 453 insertions(+) create mode 100644 cmd/dep/godep_importer.go create mode 100644 cmd/dep/godep_importer_test.go create mode 100644 cmd/dep/testdata/godep/Godeps.json create mode 100644 cmd/dep/testdata/godep/expected_import_output.txt diff --git a/cmd/dep/godep_importer.go b/cmd/dep/godep_importer.go new file mode 100644 index 0000000000..4f5b58027f --- /dev/null +++ b/cmd/dep/godep_importer.go @@ -0,0 +1,169 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "encoding/json" + "io/ioutil" + "log" + "os" + "path/filepath" + + "github.com/golang/dep" + fb "github.com/golang/dep/internal/feedback" + "github.com/golang/dep/internal/gps" + "github.com/pkg/errors" +) + +const godepJsonName = "Godeps.json" + +type godepImporter struct { + json godepJson + + logger *log.Logger + verbose bool + sm gps.SourceManager +} + +func newGodepImporter(logger *log.Logger, verbose bool, sm gps.SourceManager) *godepImporter { + return &godepImporter{ + logger: logger, + verbose: verbose, + sm: sm, + } +} + +type godepJson struct { + Name string `json:"ImportPath"` + Imports []godepPackage `json:"Deps"` +} + +type godepPackage struct { + ImportPath string `json:"ImportPath"` + Rev string `json:"Rev"` + Comment string `json:"Comment"` +} + +func (g *godepImporter) Name() string { + return "godep" +} + +func (g *godepImporter) HasDepMetadata(dir string) bool { + y := filepath.Join(dir, "Godeps", godepJsonName) + if _, err := os.Stat(y); err != nil { + return false + } + + return true +} + +func (g *godepImporter) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { + err := g.load(dir) + if err != nil { + return nil, nil, err + } + + return g.convert(pr) +} + +func (g *godepImporter) load(projectDir string) error { + g.logger.Println("Detected godep configuration files...") + j := filepath.Join(projectDir, "Godeps", godepJsonName) + if g.verbose { + g.logger.Printf(" Loading %s", j) + } + jb, err := ioutil.ReadFile(j) + if err != nil { + return errors.Wrapf(err, "Unable to read %s", j) + } + err = json.Unmarshal(jb, &g.json) + if err != nil { + return errors.Wrapf(err, "Unable to parse %s", j) + } + + return nil +} + +func (g *godepImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { + g.logger.Println("Converting from Godeps.json ...") + + manifest := &dep.Manifest{ + Constraints: make(gps.ProjectConstraints), + } + lock := &dep.Lock{} + + for _, pkg := range g.json.Imports { + // ImportPath must not be empty + if pkg.ImportPath == "" { + err := errors.New("Invalid godep configuration, ImportPath is required") + return nil, nil, err + } + + // Rev must not be empty + if pkg.Rev == "" { + err := errors.New("Invalid godep configuration, Rev is required") + return nil, nil, err + } + + if pkg.Comment == "" { + // When there's no comment, try to get corresponding version for the Rev + // and fill Comment. + pi := gps.ProjectIdentifier{ + ProjectRoot: gps.ProjectRoot(pkg.ImportPath), + } + revision := gps.Revision(pkg.Rev) + version, err := lookupVersionForRevision(revision, pi, g.sm) + if err != nil { + warn := errors.Wrapf(err, "Unable to lookup the version represented by %s in %s. Falling back to locking the revision only.", pkg.Rev, pi.ProjectRoot) + g.logger.Printf(warn.Error()) + version = revision + } + + pp := getProjectPropertiesFromVersion(version) + if pp.Constraint != nil { + pkg.Comment = pp.Constraint.String() + } + } + + if pkg.Comment != "" { + // If there's a comment, use it to create project constraint + pc, err := g.buildProjectConstraint(pkg) + if err != nil { + return nil, nil, err + } + manifest.Constraints[pc.Ident.ProjectRoot] = gps.ProjectProperties{Source: pc.Ident.Source, Constraint: pc.Constraint} + } + + lp := g.buildLockedProject(pkg) + lock.P = append(lock.P, lp) + } + + return manifest, lock, nil +} + +// buildProjectConstraint uses the provided package ImportPath and Comment to +// create a project constraint +func (g *godepImporter) buildProjectConstraint(pkg godepPackage) (pc gps.ProjectConstraint, err error) { + pc.Ident = gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.ImportPath)} + pc.Constraint, err = deduceConstraint(pkg.Comment, pc.Ident, g.sm) + return +} + +// buildLockedProject uses the package Rev and Comment to create lock project +func (g *godepImporter) buildLockedProject(pkg godepPackage) gps.LockedProject { + pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.ImportPath)} + + var version gps.Version + + if pkg.Comment != "" { + ver := gps.NewVersion(pkg.Comment) + version = ver.Is(gps.Revision(pkg.Rev)) + } else { + version = gps.Revision(pkg.Rev) + } + + feedback(version, pi.ProjectRoot, fb.DepTypeImported, g.logger) + return gps.NewLockedProject(pi, version, nil) +} diff --git a/cmd/dep/godep_importer_test.go b/cmd/dep/godep_importer_test.go new file mode 100644 index 0000000000..b96b08045d --- /dev/null +++ b/cmd/dep/godep_importer_test.go @@ -0,0 +1,262 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "bytes" + "log" + "path/filepath" + "testing" + + "github.com/golang/dep/internal/gps" + "github.com/golang/dep/internal/test" + "github.com/pkg/errors" +) + +const testGodepProjectRoot = "github.com/golang/notexist" + +func TestGodepConfig_Import(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + + cacheDir := "gps-repocache" + h.TempDir(cacheDir) + h.TempDir("src") + h.TempDir(filepath.Join("src", testGodepProjectRoot)) + h.TempCopy(filepath.Join(testGodepProjectRoot, "Godeps", godepJsonName), "godep/Godeps.json") + + projectRoot := h.Path(testGodepProjectRoot) + sm, err := gps.NewSourceManager(h.Path(cacheDir)) + h.Must(err) + defer sm.Release() + + // Capture stderr so we can verify output + verboseOutput := &bytes.Buffer{} + logger := log.New(verboseOutput, "", 0) + + g := newGodepImporter(logger, false, sm) // Disable verbose so that we don't print values that change each test run + if !g.HasDepMetadata(projectRoot) { + t.Fatal("Expected the importer to detect godep configuration file") + } + + m, l, err := g.Import(projectRoot, testGodepProjectRoot) + h.Must(err) + + if m == nil { + t.Fatal("Expected the manifest to be generated") + } + + if l == nil { + t.Fatal("Expected the lock to be generated") + } + + goldenFile := "godep/expected_import_output.txt" + got := verboseOutput.String() + want := h.GetTestFileString(goldenFile) + if want != got { + if *test.UpdateGolden { + if err := h.WriteTestFile(goldenFile, got); err != nil { + t.Fatalf("%+v", errors.Wrapf(err, "Unable to write updated golden file %s", goldenFile)) + } + } else { + t.Fatalf("expected %s, got %s", want, got) + } + } +} + +func TestGodepConfig_JsonLoad(t *testing.T) { + // This is same as cmd/dep/testdata/Godeps.json + wantJSON := godepJson{ + Name: "github.com/golang/notexist", + Imports: []godepPackage{ + { + ImportPath: "github.com/sdboyer/deptest", + Rev: "3f4c3bea144e112a69bbe5d8d01c1b09a544253f", + }, + { + ImportPath: "github.com/sdboyer/deptestdos", + Rev: "5c607206be5decd28e6263ffffdcee067266015e", + Comment: "v2.0.0", + }, + }, + } + + h := test.NewHelper(t) + defer h.Cleanup() + + ctx := newTestContext(h) + + h.TempCopy(filepath.Join(testGodepProjectRoot, "Godeps", godepJsonName), "godep/Godeps.json") + + projectRoot := h.Path(testGodepProjectRoot) + + g := newGodepImporter(ctx.Err, true, nil) + err := g.load(projectRoot) + if err != nil { + t.Fatalf("Error while loading... %v", err) + } + + if g.json.Name != wantJSON.Name { + t.Fatalf("Expected project name to be %v, but got %v", wantJSON.Name, g.json.Name) + } + + if !equalImports(g.json.Imports, wantJSON.Imports) { + t.Fatalf("Expected imports to be equal. \n\t(GOT): %v\n\t(WNT): %v", g.json.Imports, wantJSON.Imports) + } +} + +func TestGodepConfig_ConvertProject(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + g := newGodepImporter(discardLogger, true, sm) + g.json = godepJson{ + Name: "github.com/foo/bar", + Imports: []godepPackage{ + { + ImportPath: "github.com/sdboyer/deptest", + // This revision has 2 versions attached to it, v1.0.0 & v0.8.0. + Rev: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + Comment: "v0.8.0", + }, + }, + } + + manifest, lock, err := g.convert("") + if err != nil { + t.Fatal(err) + } + + d, ok := manifest.Constraints["github.com/sdboyer/deptest"] + if !ok { + t.Fatal("Expected the manifest to have a dependency for 'github.com/sdboyer/deptest' but got none") + } + + v := d.Constraint.String() + if v != "^0.8.0" { + t.Fatalf("Expected manifest constraint to be ^0.8.0, got %s", v) + } + + p := lock.P[0] + if p.Ident().ProjectRoot != "github.com/sdboyer/deptest" { + t.Fatalf("Expected the lock to have a project for 'github.com/sdboyer/deptest' but got '%s'", p.Ident().ProjectRoot) + } + + lv := p.Version() + lpv, ok := lv.(gps.PairedVersion) + if !ok { + t.Fatalf("Expected locked version to be PairedVersion but got %T", lv) + } + + rev := lpv.Underlying() + if rev != "ff2948a2ac8f538c4ecd55962e919d1e13e74baf" { + t.Fatalf("Expected locked revision to be 'ff2948a2ac8f538c4ecd55962e919d1e13e74baf', got %s", rev) + } + + ver := lpv.String() + if ver != "v0.8.0" { + t.Fatalf("Expected locked version to be 'v0.8.0', got %s", ver) + } +} + +func TestGodepConfig_ConvertProject_EmptyComment(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + h.TempDir("src") + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + g := newGodepImporter(discardLogger, true, sm) + g.json = godepJson{ + Name: "github.com/foo/bar", + Imports: []godepPackage{ + { + ImportPath: "github.com/sdboyer/deptest", + // This revision has 2 versions attached to it, v1.0.0 & v0.8.0. + Rev: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + }, + }, + } + + manifest, lock, err := g.convert("") + if err != nil { + t.Fatal(err) + } + + d, ok := manifest.Constraints["github.com/sdboyer/deptest"] + if !ok { + t.Fatal("Expected the manifest to have a dependency for 'github.com/sdboyer/deptest' but got none") + } + + v := d.Constraint.String() + if v != "^1.0.0" { + t.Fatalf("Expected manifest constraint to be ^1.0.0, got %s", v) + } + + p := lock.P[0] + if p.Ident().ProjectRoot != "github.com/sdboyer/deptest" { + t.Fatalf("Expected the lock to have a project for 'github.com/sdboyer/deptest' but got '%s'", p.Ident().ProjectRoot) + } + + lv := p.Version() + lpv, ok := lv.(gps.PairedVersion) + if !ok { + t.Fatalf("Expected locked version to be PairedVersion but got %T", lv) + } + + rev := lpv.Underlying() + if rev != "ff2948a2ac8f538c4ecd55962e919d1e13e74baf" { + t.Fatalf("Expected locked revision to be 'ff2948a2ac8f538c4ecd55962e919d1e13e74baf', got %s", rev) + } + + ver := lpv.String() + if ver != "^1.0.0" { + t.Fatalf("Expected locked version to be '^1.0.0', got %s", ver) + } +} + +func TestGodepConfig_Convert_BadInput_EmptyPackageName(t *testing.T) { + g := newGodepImporter(discardLogger, true, nil) + g.json = godepJson{ + Imports: []godepPackage{{ImportPath: ""}}, + } + + _, _, err := g.convert("") + if err == nil { + t.Fatal("Expected conversion to fail because the ImportPath is empty") + } +} + +// Compares two slices of godepPackage and checks if they are equal. +func equalImports(a, b []godepPackage) bool { + + if a == nil && b == nil { + return true + } + + if a == nil || b == nil { + return false + } + + if len(a) != len(b) { + return false + } + + for i := range a { + if a[i] != b[i] { + return false + } + } + + return true +} diff --git a/cmd/dep/testdata/godep/Godeps.json b/cmd/dep/testdata/godep/Godeps.json new file mode 100644 index 0000000000..15126ac12d --- /dev/null +++ b/cmd/dep/testdata/godep/Godeps.json @@ -0,0 +1,16 @@ +{ + "ImportPath": "github.com/golang/notexist", + "GoVersion": "go1.8", + "GodepVersion": "vXYZ", + "Deps": [ + { + "ImportPath": "github.com/sdboyer/deptest", + "Rev": "3f4c3bea144e112a69bbe5d8d01c1b09a544253f" + }, + { + "ImportPath": "github.com/sdboyer/deptestdos", + "Comment": "v2.0.0", + "Rev": "5c607206be5decd28e6263ffffdcee067266015e" + } + ] +} diff --git a/cmd/dep/testdata/godep/expected_import_output.txt b/cmd/dep/testdata/godep/expected_import_output.txt new file mode 100644 index 0000000000..b023ee471a --- /dev/null +++ b/cmd/dep/testdata/godep/expected_import_output.txt @@ -0,0 +1,6 @@ +Detected godep configuration files... +Converting from Godeps.json ... + Using ^0.8.1 as initial constraint for imported dep github.com/sdboyer/deptest + Trying ^0.8.1 (3f4c3be) as initial lock for imported dep github.com/sdboyer/deptest + Using ^2.0.0 as initial constraint for imported dep github.com/sdboyer/deptestdos + Trying v2.0.0 (5c60720) as initial lock for imported dep github.com/sdboyer/deptestdos From b8e0254486b5c081d75de54a99e5f9c07668e3be Mon Sep 17 00:00:00 2001 From: Sunny Date: Sun, 11 Jun 2017 01:14:10 +0530 Subject: [PATCH 2/4] Add integration test --- cmd/dep/root_analyzer.go | 1 + .../init/godep/case1/final/Gopkg.lock | 21 +++++++++++++++++++ .../init/godep/case1/final/Gopkg.toml | 4 ++++ .../godep/case1/initial/Godeps/Godeps.json | 17 +++++++++++++++ .../init/godep/case1/initial/main.go | 16 ++++++++++++++ .../init/godep/case1/testcase.json | 13 ++++++++++++ 6 files changed, 72 insertions(+) create mode 100644 cmd/dep/testdata/harness_tests/init/godep/case1/final/Gopkg.lock create mode 100644 cmd/dep/testdata/harness_tests/init/godep/case1/final/Gopkg.toml create mode 100644 cmd/dep/testdata/harness_tests/init/godep/case1/initial/Godeps/Godeps.json create mode 100644 cmd/dep/testdata/harness_tests/init/godep/case1/initial/main.go create mode 100644 cmd/dep/testdata/harness_tests/init/godep/case1/testcase.json diff --git a/cmd/dep/root_analyzer.go b/cmd/dep/root_analyzer.go index a92e9063da..785869b996 100644 --- a/cmd/dep/root_analyzer.go +++ b/cmd/dep/root_analyzer.go @@ -73,6 +73,7 @@ func (a *rootAnalyzer) importManifestAndLock(dir string, pr gps.ProjectRoot, sup importers := []importer{ newGlideImporter(logger, a.ctx.Verbose, a.sm), + newGodepImporter(logger, a.ctx.Verbose, a.sm), } for _, i := range importers { diff --git a/cmd/dep/testdata/harness_tests/init/godep/case1/final/Gopkg.lock b/cmd/dep/testdata/harness_tests/init/godep/case1/final/Gopkg.lock new file mode 100644 index 0000000000..0e62baa0cc --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/godep/case1/final/Gopkg.lock @@ -0,0 +1,21 @@ +# This file is autogenerated, do not edit; changes may be undone by the next 'dep ensure'. + + +[[projects]] + name = "github.com/sdboyer/deptest" + packages = ["."] + revision = "3f4c3bea144e112a69bbe5d8d01c1b09a544253f" + version = "master" + +[[projects]] + name = "github.com/sdboyer/deptestdos" + packages = ["."] + revision = "5c607206be5decd28e6263ffffdcee067266015e" + version = "v2.0.0" + +[solve-meta] + analyzer-name = "dep" + analyzer-version = 1 + inputs-digest = "1ed417a0bec57ffe988fae1cba8f3d49994fb893394d61844e0b3c96d69573fe" + solver-name = "gps-cdcl" + solver-version = 1 diff --git a/cmd/dep/testdata/harness_tests/init/godep/case1/final/Gopkg.toml b/cmd/dep/testdata/harness_tests/init/godep/case1/final/Gopkg.toml new file mode 100644 index 0000000000..aaf78303fa --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/godep/case1/final/Gopkg.toml @@ -0,0 +1,4 @@ + +[[constraint]] + name = "github.com/sdboyer/deptestdos" + version = "2.0.0" diff --git a/cmd/dep/testdata/harness_tests/init/godep/case1/initial/Godeps/Godeps.json b/cmd/dep/testdata/harness_tests/init/godep/case1/initial/Godeps/Godeps.json new file mode 100644 index 0000000000..ee87370e5d --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/godep/case1/initial/Godeps/Godeps.json @@ -0,0 +1,17 @@ +{ + "ImportPath": "github.com/golang/notexist", + "GoVersion": "go1.8", + "GodepVersion": "vXYZ", + "Deps": [ + { + "ImportPath": "github.com/sdboyer/deptest", + "Comment": "master", + "Rev": "3f4c3bea144e112a69bbe5d8d01c1b09a544253f" + }, + { + "ImportPath": "github.com/sdboyer/deptestdos", + "Comment": "v2.0.0", + "Rev": "5c607206be5decd28e6263ffffdcee067266015e" + } + ] +} diff --git a/cmd/dep/testdata/harness_tests/init/godep/case1/initial/main.go b/cmd/dep/testdata/harness_tests/init/godep/case1/initial/main.go new file mode 100644 index 0000000000..2b2c7c396e --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/godep/case1/initial/main.go @@ -0,0 +1,16 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "fmt" + + "github.com/sdboyer/deptestdos" +) + +func main() { + var x deptestdos.Bar + fmt.Println(x) +} diff --git a/cmd/dep/testdata/harness_tests/init/godep/case1/testcase.json b/cmd/dep/testdata/harness_tests/init/godep/case1/testcase.json new file mode 100644 index 0000000000..017dc4cd55 --- /dev/null +++ b/cmd/dep/testdata/harness_tests/init/godep/case1/testcase.json @@ -0,0 +1,13 @@ +{ + "commands": [ + ["init", "-no-examples"] + ], + "error-expected": "", + "gopath-initial": { + "github.com/sdboyer/deptest": "3f4c3bea144e112a69bbe5d8d01c1b09a544253f" + }, + "vendor-final": [ + "github.com/sdboyer/deptest", + "github.com/sdboyer/deptestdos" + ] +} From f27c34e914c4caeab70a5027b7267a160509dce4 Mon Sep 17 00:00:00 2001 From: Sunny Date: Sun, 11 Jun 2017 01:58:45 +0530 Subject: [PATCH 3/4] golint fixes --- cmd/dep/godep_importer.go | 10 +++++----- cmd/dep/godep_importer_test.go | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/cmd/dep/godep_importer.go b/cmd/dep/godep_importer.go index 4f5b58027f..50843b5054 100644 --- a/cmd/dep/godep_importer.go +++ b/cmd/dep/godep_importer.go @@ -17,10 +17,10 @@ import ( "github.com/pkg/errors" ) -const godepJsonName = "Godeps.json" +const godepJSONName = "Godeps.json" type godepImporter struct { - json godepJson + json godepJSON logger *log.Logger verbose bool @@ -35,7 +35,7 @@ func newGodepImporter(logger *log.Logger, verbose bool, sm gps.SourceManager) *g } } -type godepJson struct { +type godepJSON struct { Name string `json:"ImportPath"` Imports []godepPackage `json:"Deps"` } @@ -51,7 +51,7 @@ func (g *godepImporter) Name() string { } func (g *godepImporter) HasDepMetadata(dir string) bool { - y := filepath.Join(dir, "Godeps", godepJsonName) + y := filepath.Join(dir, "Godeps", godepJSONName) if _, err := os.Stat(y); err != nil { return false } @@ -70,7 +70,7 @@ func (g *godepImporter) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, * func (g *godepImporter) load(projectDir string) error { g.logger.Println("Detected godep configuration files...") - j := filepath.Join(projectDir, "Godeps", godepJsonName) + j := filepath.Join(projectDir, "Godeps", godepJSONName) if g.verbose { g.logger.Printf(" Loading %s", j) } diff --git a/cmd/dep/godep_importer_test.go b/cmd/dep/godep_importer_test.go index b96b08045d..a70c5828e5 100644 --- a/cmd/dep/godep_importer_test.go +++ b/cmd/dep/godep_importer_test.go @@ -25,7 +25,7 @@ func TestGodepConfig_Import(t *testing.T) { h.TempDir(cacheDir) h.TempDir("src") h.TempDir(filepath.Join("src", testGodepProjectRoot)) - h.TempCopy(filepath.Join(testGodepProjectRoot, "Godeps", godepJsonName), "godep/Godeps.json") + h.TempCopy(filepath.Join(testGodepProjectRoot, "Godeps", godepJSONName), "godep/Godeps.json") projectRoot := h.Path(testGodepProjectRoot) sm, err := gps.NewSourceManager(h.Path(cacheDir)) @@ -68,7 +68,7 @@ func TestGodepConfig_Import(t *testing.T) { func TestGodepConfig_JsonLoad(t *testing.T) { // This is same as cmd/dep/testdata/Godeps.json - wantJSON := godepJson{ + wantJSON := godepJSON{ Name: "github.com/golang/notexist", Imports: []godepPackage{ { @@ -88,7 +88,7 @@ func TestGodepConfig_JsonLoad(t *testing.T) { ctx := newTestContext(h) - h.TempCopy(filepath.Join(testGodepProjectRoot, "Godeps", godepJsonName), "godep/Godeps.json") + h.TempCopy(filepath.Join(testGodepProjectRoot, "Godeps", godepJSONName), "godep/Godeps.json") projectRoot := h.Path(testGodepProjectRoot) @@ -117,7 +117,7 @@ func TestGodepConfig_ConvertProject(t *testing.T) { defer sm.Release() g := newGodepImporter(discardLogger, true, sm) - g.json = godepJson{ + g.json = godepJSON{ Name: "github.com/foo/bar", Imports: []godepPackage{ { @@ -177,7 +177,7 @@ func TestGodepConfig_ConvertProject_EmptyComment(t *testing.T) { defer sm.Release() g := newGodepImporter(discardLogger, true, sm) - g.json = godepJson{ + g.json = godepJSON{ Name: "github.com/foo/bar", Imports: []godepPackage{ { @@ -227,7 +227,7 @@ func TestGodepConfig_ConvertProject_EmptyComment(t *testing.T) { func TestGodepConfig_Convert_BadInput_EmptyPackageName(t *testing.T) { g := newGodepImporter(discardLogger, true, nil) - g.json = godepJson{ + g.json = godepJSON{ Imports: []godepPackage{{ImportPath: ""}}, } From d2109d3e3e73e274bbe515770d24201abf447799 Mon Sep 17 00:00:00 2001 From: Sunny Date: Mon, 12 Jun 2017 22:20:26 +0530 Subject: [PATCH 4/4] Skip duplicate and sub-package imports - Adds skipping duplicate and sub-package imports using projectExistsInLock(). - Adds tests for missing Rev in godep import package list. - Removes `Name` field from godepImporter. --- cmd/dep/godep_importer.go | 27 ++++++++- cmd/dep/godep_importer_test.go | 108 ++++++++++++++++++++++++++++++--- 2 files changed, 126 insertions(+), 9 deletions(-) diff --git a/cmd/dep/godep_importer.go b/cmd/dep/godep_importer.go index 50843b5054..3d00acc4b8 100644 --- a/cmd/dep/godep_importer.go +++ b/cmd/dep/godep_importer.go @@ -36,7 +36,6 @@ func newGodepImporter(logger *log.Logger, verbose bool, sm gps.SourceManager) *g } type godepJSON struct { - Name string `json:"ImportPath"` Imports []godepPackage `json:"Deps"` } @@ -101,6 +100,18 @@ func (g *godepImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, e return nil, nil, err } + // Obtain ProjectRoot. Required for avoiding sub-package imports. + ip, err := g.sm.DeduceProjectRoot(pkg.ImportPath) + if err != nil { + return nil, nil, err + } + pkg.ImportPath = string(ip) + + // Check if it already existing in locked projects + if projectExistsInLock(lock, pkg.ImportPath) { + continue + } + // Rev must not be empty if pkg.Rev == "" { err := errors.New("Invalid godep configuration, Rev is required") @@ -133,7 +144,7 @@ func (g *godepImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, e if err != nil { return nil, nil, err } - manifest.Constraints[pc.Ident.ProjectRoot] = gps.ProjectProperties{Source: pc.Ident.Source, Constraint: pc.Constraint} + manifest.Constraints[pc.Ident.ProjectRoot] = gps.ProjectProperties{Constraint: pc.Constraint} } lp := g.buildLockedProject(pkg) @@ -167,3 +178,15 @@ func (g *godepImporter) buildLockedProject(pkg godepPackage) gps.LockedProject { feedback(version, pi.ProjectRoot, fb.DepTypeImported, g.logger) return gps.NewLockedProject(pi, version, nil) } + +// projectExistsInLock checks if the given import path already existing in +// locked projects. +func projectExistsInLock(l *dep.Lock, ip string) bool { + for _, lp := range l.P { + if ip == string(lp.Ident().ProjectRoot) { + return true + } + } + + return false +} diff --git a/cmd/dep/godep_importer_test.go b/cmd/dep/godep_importer_test.go index a70c5828e5..a81a88c8b0 100644 --- a/cmd/dep/godep_importer_test.go +++ b/cmd/dep/godep_importer_test.go @@ -10,6 +10,7 @@ import ( "path/filepath" "testing" + "github.com/golang/dep" "github.com/golang/dep/internal/gps" "github.com/golang/dep/internal/test" "github.com/pkg/errors" @@ -69,7 +70,6 @@ func TestGodepConfig_Import(t *testing.T) { func TestGodepConfig_JsonLoad(t *testing.T) { // This is same as cmd/dep/testdata/Godeps.json wantJSON := godepJSON{ - Name: "github.com/golang/notexist", Imports: []godepPackage{ { ImportPath: "github.com/sdboyer/deptest", @@ -98,10 +98,6 @@ func TestGodepConfig_JsonLoad(t *testing.T) { t.Fatalf("Error while loading... %v", err) } - if g.json.Name != wantJSON.Name { - t.Fatalf("Expected project name to be %v, but got %v", wantJSON.Name, g.json.Name) - } - if !equalImports(g.json.Imports, wantJSON.Imports) { t.Fatalf("Expected imports to be equal. \n\t(GOT): %v\n\t(WNT): %v", g.json.Imports, wantJSON.Imports) } @@ -118,7 +114,6 @@ func TestGodepConfig_ConvertProject(t *testing.T) { g := newGodepImporter(discardLogger, true, sm) g.json = godepJSON{ - Name: "github.com/foo/bar", Imports: []godepPackage{ { ImportPath: "github.com/sdboyer/deptest", @@ -178,7 +173,6 @@ func TestGodepConfig_ConvertProject_EmptyComment(t *testing.T) { g := newGodepImporter(discardLogger, true, sm) g.json = godepJSON{ - Name: "github.com/foo/bar", Imports: []godepPackage{ { ImportPath: "github.com/sdboyer/deptest", @@ -237,6 +231,106 @@ func TestGodepConfig_Convert_BadInput_EmptyPackageName(t *testing.T) { } } +func TestGodepConfig_Convert_BadInput_EmptyRev(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + h.TempDir("src") + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + g := newGodepImporter(discardLogger, true, sm) + g.json = godepJSON{ + Imports: []godepPackage{ + { + ImportPath: "github.com/sdboyer/deptest", + }, + }, + } + + _, _, err = g.convert("") + if err == nil { + t.Fatal("Expected conversion to fail because the Rev is empty") + } +} + +func TestGodepConfig_Convert_SubPackages(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + h.TempDir("src") + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + g := newGodepImporter(discardLogger, true, sm) + g.json = godepJSON{ + Imports: []godepPackage{ + { + ImportPath: "github.com/sdboyer/deptest", + Rev: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + }, + { + ImportPath: "github.com/sdboyer/deptest/foo", + Rev: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + }, + }, + } + + manifest, lock, err := g.convert("") + if err != nil { + t.Fatal(err) + } + + if _, present := manifest.Constraints["github.com/sdboyer/deptest"]; !present { + t.Fatal("Expected the manifest to have a dependency for 'github.com/sdboyer/deptest'") + } + + if _, present := manifest.Constraints["github.com/sdboyer/deptest/foo"]; present { + t.Fatal("Expected the manifest to not have a dependency for 'github.com/sdboyer/deptest/foo'") + } + + if len(lock.P) != 1 { + t.Fatalf("Expected lock to have 1 project, got %v", len(lock.P)) + } + + if string(lock.P[0].Ident().ProjectRoot) != "github.com/sdboyer/deptest" { + t.Fatal("Expected lock to have 'github.com/sdboyer/deptest'") + } +} + +func TestGodepConfig_ProjectExistsInLock(t *testing.T) { + lock := &dep.Lock{} + pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")} + ver := gps.NewVersion("v1.0.0") + lock.P = append(lock.P, gps.NewLockedProject(pi, ver, nil)) + + cases := []struct { + importPath string + want bool + }{ + { + importPath: "github.com/sdboyer/deptest", + want: true, + }, + { + importPath: "github.com/golang/notexist", + want: false, + }, + } + + for _, c := range cases { + result := projectExistsInLock(lock, c.importPath) + + if result != c.want { + t.Fatalf("projectExistsInLock result is not as expected: \n\t(GOT) %v \n\t(WNT) %v", result, c.want) + } + } +} + // Compares two slices of godepPackage and checks if they are equal. func equalImports(a, b []godepPackage) bool {