diff --git a/cmd/dep/ensure.go b/cmd/dep/ensure.go index ab98b479ed..2bebd63ec9 100644 --- a/cmd/dep/ensure.go +++ b/cmd/dep/ensure.go @@ -15,6 +15,7 @@ import ( "path/filepath" "sort" "strings" + "sync" "github.com/golang/dep" "github.com/golang/dep/internal/gps" @@ -111,6 +112,8 @@ dep ensure -update -no-vendor ` +var errUpdateArgsValidation = errors.New("update arguments validation failed") + func (cmd *ensureCommand) Name() string { return "ensure" } func (cmd *ensureCommand) Args() string { return "[-update | -add] [-no-vendor | -vendor-only] [-dry-run] [...]" @@ -345,37 +348,8 @@ func (cmd *ensureCommand) runUpdate(ctx *dep.Ctx, args []string, p *dep.Project, params.ChangeAll = true } - // Allow any of specified project versions to change, regardless of the lock - // file. - for _, arg := range args { - // Ensure the provided path has a deducible project root - // TODO(sdboyer) do these concurrently - pc, path, err := getProjectConstraint(arg, sm) - if err != nil { - // TODO(sdboyer) return all errors, not just the first one we encounter - // TODO(sdboyer) ensure these errors are contextualized in a sensible way for -update - return err - } - if path != string(pc.Ident.ProjectRoot) { - // TODO(sdboyer): does this really merit an abortive error? - return errors.Errorf("%s is not a project root, try %s instead", path, pc.Ident.ProjectRoot) - } - - if !p.Lock.HasProjectWithRoot(pc.Ident.ProjectRoot) { - return errors.Errorf("%s is not present in %s, cannot -update it", pc.Ident.ProjectRoot, dep.LockName) - } - - if pc.Ident.Source != "" { - return errors.Errorf("cannot specify alternate sources on -update (%s)", pc.Ident.Source) - } - - if !gps.IsAny(pc.Constraint) { - // TODO(sdboyer) constraints should be allowed to allow solves that - // target particular versions while remaining within declared constraints - return errors.Errorf("version constraint %s passed for %s, but -update follows constraints declared in %s, not CLI arguments", pc.Constraint, pc.Ident.ProjectRoot, dep.ManifestName) - } - - params.ToChange = append(params.ToChange, gps.ProjectRoot(arg)) + if err := validateUpdateArgs(ctx, args, p, sm, ¶ms); err != nil { + return err } // Re-prepare a solver now that our params are complete. @@ -793,3 +767,77 @@ func (e pkgtreeErrs) Error() string { return fmt.Sprintf("found %d errors in the package tree:\n%s", len(e), strings.Join(errs, "\n")) } + +func validateUpdateArgs(ctx *dep.Ctx, args []string, p *dep.Project, sm gps.SourceManager, params *gps.SolveParameters) error { + // Channel for receiving all the valid arguments. + argsCh := make(chan string, len(args)) + + // Channel for receiving all the validation errors. + errCh := make(chan error, len(args)) + + var wg sync.WaitGroup + + // Allow any of specified project versions to change, regardless of the lock + // file. + for _, arg := range args { + wg.Add(1) + + go func(arg string) { + defer wg.Done() + + // Ensure the provided path has a deducible project root. + pc, path, err := getProjectConstraint(arg, sm) + if err != nil { + // TODO(sdboyer) ensure these errors are contextualized in a sensible way for -update + errCh <- err + return + } + if path != string(pc.Ident.ProjectRoot) { + // TODO(sdboyer): does this really merit an abortive error? + errCh <- errors.Errorf("%s is not a project root, try %s instead", path, pc.Ident.ProjectRoot) + return + } + + if !p.Lock.HasProjectWithRoot(pc.Ident.ProjectRoot) { + errCh <- errors.Errorf("%s is not present in %s, cannot -update it", pc.Ident.ProjectRoot, dep.LockName) + return + } + + if pc.Ident.Source != "" { + errCh <- errors.Errorf("cannot specify alternate sources on -update (%s)", pc.Ident.Source) + return + } + + if !gps.IsAny(pc.Constraint) { + // TODO(sdboyer) constraints should be allowed to allow solves that + // target particular versions while remaining within declared constraints. + errCh <- errors.Errorf("version constraint %s passed for %s, but -update follows constraints declared in %s, not CLI arguments", pc.Constraint, pc.Ident.ProjectRoot, dep.ManifestName) + return + } + + // Valid argument. + argsCh <- arg + }(arg) + } + + wg.Wait() + close(errCh) + close(argsCh) + + // Log all the errors. + if len(errCh) > 0 { + ctx.Err.Printf("Invalid arguments passed to ensure -update:\n\n") + for err := range errCh { + ctx.Err.Println(" ✗", err.Error()) + } + ctx.Err.Println() + return errUpdateArgsValidation + } + + // Add all the valid arguments to solve params. + for arg := range argsCh { + params.ToChange = append(params.ToChange, gps.ProjectRoot(arg)) + } + + return nil +} diff --git a/cmd/dep/ensure_test.go b/cmd/dep/ensure_test.go index a9a1ec922b..c4fb0c7a24 100644 --- a/cmd/dep/ensure_test.go +++ b/cmd/dep/ensure_test.go @@ -5,12 +5,18 @@ package main import ( + "bytes" "errors" "go/build" + "io/ioutil" + "log" + "strings" "testing" + "github.com/golang/dep" "github.com/golang/dep/internal/gps" "github.com/golang/dep/internal/gps/pkgtree" + "github.com/golang/dep/internal/test" ) func TestInvalidEnsureFlagCombinations(t *testing.T) { @@ -139,3 +145,103 @@ func TestCheckErrors(t *testing.T) { }) } } + +func TestValidateUpdateArgs(t *testing.T) { + cases := []struct { + name string + args []string + wantError error + wantWarn []string + lockedProjects []string + }{ + { + name: "empty args", + args: []string{}, + wantError: nil, + }, + { + name: "not project root", + args: []string{"github.com/golang/dep/cmd"}, + wantError: errUpdateArgsValidation, + wantWarn: []string{ + "github.com/golang/dep/cmd is not a project root, try github.com/golang/dep instead", + }, + }, + { + name: "not present in lock", + args: []string{"github.com/golang/dep"}, + wantError: errUpdateArgsValidation, + wantWarn: []string{ + "github.com/golang/dep is not present in Gopkg.lock, cannot -update it", + }, + }, + { + name: "cannot specify alternate sources", + args: []string{"github.com/golang/dep:github.com/example/dep"}, + wantError: errUpdateArgsValidation, + wantWarn: []string{ + "cannot specify alternate sources on -update (github.com/example/dep)", + }, + lockedProjects: []string{"github.com/golang/dep"}, + }, + { + name: "version constraint passed", + args: []string{"github.com/golang/dep@master"}, + wantError: errUpdateArgsValidation, + wantWarn: []string{ + "version constraint master passed for github.com/golang/dep, but -update follows constraints declared in Gopkg.toml, not CLI arguments", + }, + lockedProjects: []string{"github.com/golang/dep"}, + }, + } + + h := test.NewHelper(t) + defer h.Cleanup() + + h.TempDir("src") + pwd := h.Path(".") + + stderrOutput := &bytes.Buffer{} + errLogger := log.New(stderrOutput, "", 0) + ctx := &dep.Ctx{ + GOPATH: pwd, + Out: log.New(ioutil.Discard, "", 0), + Err: errLogger, + } + + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + p := new(dep.Project) + params := p.MakeParams() + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + // Empty the buffer for every case + stderrOutput.Reset() + + // Fill up the locked projects + lockedProjects := []gps.LockedProject{} + for _, lp := range c.lockedProjects { + pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(lp)} + lockedProjects = append(lockedProjects, gps.NewLockedProject(pi, gps.NewVersion("v1.0.0"), []string{})) + } + + // Add lock to project + p.Lock = &dep.Lock{P: lockedProjects} + + err := validateUpdateArgs(ctx, c.args, p, sm, ¶ms) + if err != c.wantError { + t.Fatalf("Unexpected error while validating update args:\n\t(GOT): %v\n\t(WNT): %v", err, c.wantError) + } + + warnings := stderrOutput.String() + for _, warn := range c.wantWarn { + if !strings.Contains(warnings, warn) { + t.Fatalf("Expected validateUpdateArgs errors to contain: %q", warn) + } + } + }) + } +}