Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Commit

Permalink
Avoid duplicate and sub-package imports
Browse files Browse the repository at this point in the history
- Adds avoiding duplicate and sub-package imports using projectExistsInLock().
- Adds tests for missing Rev in godep import package list.
- Removes `Name` field from godepImporter.
  • Loading branch information
darkowlzz committed Jun 12, 2017
1 parent f27c34e commit 8f58ee6
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 9 deletions.
27 changes: 25 additions & 2 deletions cmd/dep/godep_importer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}

Expand Down Expand Up @@ -101,6 +100,18 @@ func (g *godepImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, e
return nil, nil, err
}

// Obtain ProjectRoot. Required for avoid 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")
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
105 changes: 98 additions & 7 deletions cmd/dep/godep_importer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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)
}
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -237,6 +231,103 @@ 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 _, 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 {

Expand Down

0 comments on commit 8f58ee6

Please sign in to comment.