From a7591c0ab4ace3c3e5c6e188a9499bbdbedbb72e Mon Sep 17 00:00:00 2001 From: Sunny Date: Sun, 3 Sep 2017 20:49:25 +0530 Subject: [PATCH] fix(manifest): validate project roots in manifest 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. --- cmd/dep/ensure.go | 4 ++ cmd/dep/status.go | 4 ++ manifest.go | 23 ++++++++++ manifest_test.go | 110 ++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 141 insertions(+) diff --git a/cmd/dep/ensure.go b/cmd/dep/ensure.go index 583ae33e33..9683d2dd67 100644 --- a/cmd/dep/ensure.go +++ b/cmd/dep/ensure.go @@ -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 diff --git a/cmd/dep/status.go b/cmd/dep/status.go index 973033662a..a849b15cb3 100644 --- a/cmd/dep/status.go +++ b/cmd/dep/status.go @@ -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 { diff --git a/manifest.go b/manifest.go index 11472a64a5..74a64da273 100644 --- a/manifest.go +++ b/manifest.go @@ -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{} diff --git a/manifest_test.go b/manifest_test.go index c79f5e02b1..f206db8b82 100644 --- a/manifest_test.go +++ b/manifest_test.go @@ -5,7 +5,10 @@ package dep import ( + "bytes" "errors" + "io/ioutil" + "log" "reflect" "strings" "testing" @@ -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) + } + } + }) + } +}