From d2109d3e3e73e274bbe515770d24201abf447799 Mon Sep 17 00:00:00 2001 From: Sunny Date: Mon, 12 Jun 2017 22:20:26 +0530 Subject: [PATCH] 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 {