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

Commit

Permalink
fix(manifest): validate project roots in manifest
Browse files Browse the repository at this point in the history
Validating ProjectRoot(s) require source manager, which is created after
loading the project. Hence, ProjectRoot validation can't be done in
existing validateManifest.

This change adds `ValidateProjectRoots()` which validates only the Constraint
and Override names to be valid ProjectRoot. Warnings are issued at stderr when
invalid ProjectRoot is found in manifest.

`ensure` and `status` call `ValidateProjectRoots()` expecitly.
  • Loading branch information
darkowlzz committed Sep 3, 2017
1 parent 3b01418 commit 34e1de4
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 0 deletions.
4 changes: 4 additions & 0 deletions cmd/dep/ensure.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error {
sm.UseDefaultSignalHandling()
defer sm.Release()

if err := dep.ValidateProjectRoots(ctx, p.Manifest, sm); err != nil {
return err
}

params := p.MakeParams()
if ctx.Verbose {
params.TraceLogger = ctx.Err
Expand Down
4 changes: 4 additions & 0 deletions cmd/dep/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ func (cmd *statusCommand) Run(ctx *dep.Ctx, args []string) error {
sm.UseDefaultSignalHandling()
defer sm.Release()

if err := dep.ValidateProjectRoots(ctx, p.Manifest, sm); err != nil {
return err
}

var buf bytes.Buffer
var out outputter
switch {
Expand Down
23 changes: 23 additions & 0 deletions manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,29 @@ func validateManifest(s string) ([]error, error) {
return warns, nil
}

// ValidateProjectRoots validates the project roots present in manifest.
func ValidateProjectRoots(c *Ctx, m *Manifest, sm gps.SourceManager) error {
projectRoots := make([]gps.ProjectRoot, 0, len(m.Constraints)+len(m.Ovr))
for pr := range m.Constraints {
projectRoots = append(projectRoots, pr)
}
for pr := range m.Ovr {
projectRoots = append(projectRoots, pr)
}

for _, pr := range projectRoots {
origPR, err := sm.DeduceProjectRoot(string(pr))
if err != nil {
return errors.Wrapf(err, "could not deduce project root for %s", pr)
}
if origPR != pr {
c.Err.Printf("dep: WARNING: name %q in Gopkg.toml should be project root", pr)
}
}

return nil
}

// readManifest returns a Manifest read from r and a slice of validation warnings.
func readManifest(r io.Reader) (*Manifest, []error, error) {
buf := &bytes.Buffer{}
Expand Down
110 changes: 110 additions & 0 deletions manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
package dep

import (
"bytes"
"errors"
"io/ioutil"
"log"
"reflect"
"strings"
"testing"
Expand Down Expand Up @@ -372,3 +375,110 @@ func TestValidateManifest(t *testing.T) {
}
}
}

func TestValidateProjectRoots(t *testing.T) {
cases := []struct {
name string
manifest Manifest
wantError bool
wantWarn []string
}{
{
name: "empty Manifest",
manifest: Manifest{},
wantError: false,
wantWarn: []string{},
},
{
name: "valid project root",
manifest: Manifest{
Constraints: map[gps.ProjectRoot]gps.ProjectProperties{
gps.ProjectRoot("github.com/goland/dep"): {
Constraint: gps.Any(),
},
},
},
wantError: false,
wantWarn: []string{},
},
{
name: "invalid project roots in Constraints and Overrides",
manifest: Manifest{
Constraints: map[gps.ProjectRoot]gps.ProjectProperties{
gps.ProjectRoot("github.com/golang/dep/foo"): {
Constraint: gps.Any(),
},
gps.ProjectRoot("github.com/golang/go/xyz"): {
Constraint: gps.Any(),
},
gps.ProjectRoot("github.com/golang/fmt"): {
Constraint: gps.Any(),
},
},
Ovr: map[gps.ProjectRoot]gps.ProjectProperties{
gps.ProjectRoot("github.com/golang/mock/bar"): {
Constraint: gps.Any(),
},
gps.ProjectRoot("github.com/golang/mock"): {
Constraint: gps.Any(),
},
},
},
wantError: false,
wantWarn: []string{
"name \"github.com/golang/dep/foo\" in Gopkg.toml should be project root",
"name \"github.com/golang/mock/bar\" in Gopkg.toml should be project root",
"name \"github.com/golang/go/xyz\" in Gopkg.toml should be project root",
},
},
{
name: "invalid source path",
manifest: Manifest{
Constraints: map[gps.ProjectRoot]gps.ProjectProperties{
gps.ProjectRoot("github.com/golang"): {
Constraint: gps.Any(),
},
},
},
wantError: true,
wantWarn: []string{},
},
}

h := test.NewHelper(t)
defer h.Cleanup()

h.TempDir("src")
pwd := h.Path(".")

// Capture the stderr to verify the warnings
stderrOutput := &bytes.Buffer{}
errLogger := log.New(stderrOutput, "", 0)
ctx := &Ctx{
GOPATH: pwd,
Out: log.New(ioutil.Discard, "", 0),
Err: errLogger,
}

sm, err := ctx.SourceManager()
h.Must(err)
defer sm.Release()

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
// Empty the buffer for every case
stderrOutput.Reset()
err := ValidateProjectRoots(ctx, &c.manifest, sm)
if err != nil && !c.wantError {
t.Fatalf("Unexpected error while validating project roots: %q", err)
}

warnings := stderrOutput.String()
for _, warn := range c.wantWarn {
if !strings.Contains(warnings, warn) {
t.Fatalf("Expected ValidateProjectRoot warnings to contain: %q", warn)
}
}
})
}
}

0 comments on commit 34e1de4

Please sign in to comment.