diff --git a/.travis.yml b/.travis.yml index 55d6595625..ff197a23e7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,7 +7,7 @@ jobs: - stage: test go_import_path: github.com/golang/dep install: - - go get -u honnef.co/go/tools/cmd/{gosimple,staticcheck} + - go get -u github.com/golang/lint/golint honnef.co/go/tools/cmd/megacheck - npm install -g codeclimate-test-reporter env: - DEPTESTBYPASS501=1 diff --git a/cmd/dep/ensure.go b/cmd/dep/ensure.go index 9683d2dd67..b5861ef9c3 100644 --- a/cmd/dep/ensure.go +++ b/cmd/dep/ensure.go @@ -135,7 +135,6 @@ type ensureCommand struct { noVendor bool vendorOnly bool dryRun bool - overrides stringSlice } func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error { @@ -699,19 +698,6 @@ func (cmd *ensureCommand) runAdd(ctx *dep.Ctx, args []string, p *dep.Project, sm return errors.Wrapf(f.Close(), "closing %s", dep.ManifestName) } -type stringSlice []string - -func (s *stringSlice) String() string { - if len(*s) == 0 { - return "" - } - return strings.Join(*s, ", ") -} - -func (s *stringSlice) Set(value string) error { - *s = append(*s, value) - return nil -} func getProjectConstraint(arg string, sm gps.SourceManager) (gps.ProjectConstraint, string, error) { emptyPC := gps.ProjectConstraint{ Constraint: gps.Any(), // default to any; avoids panics later diff --git a/context.go b/context.go index 6a33809df1..9e862bc494 100644 --- a/context.go +++ b/context.go @@ -83,6 +83,8 @@ func defaultGOPATH() string { return "" } +// SourceManager produces an instance of gps's built-in SourceManager +// initialized to log to the receiver's logger. func (c *Ctx) SourceManager() (*gps.SourceMgr, error) { return gps.NewSourceManager(gps.SourceManagerConfig{ Cachedir: filepath.Join(c.GOPATH, "pkg", "dep"), diff --git a/hack/lint.bash b/hack/lint.bash index 1e446eb3a8..811f2d4d7e 100755 --- a/hack/lint.bash +++ b/hack/lint.bash @@ -6,7 +6,7 @@ # This script will validate code with various linters set -e -PKGS=$(go list ./... | grep -v /vendor/) +PKGS=$(go list ./... | grep -vF /vendor/) go vet $PKGS -staticcheck $PKGS -gosimple $PKGS +golint $PKGS +megacheck -unused.exported -ignore github.com/golang/dep/internal/test/test.go:U1000 $PKGS diff --git a/internal/gps/bridge.go b/internal/gps/bridge.go index 36751e4cb7..4ffdcb493e 100644 --- a/internal/gps/bridge.go +++ b/internal/gps/bridge.go @@ -56,12 +56,6 @@ type bridge struct { // held by the solver that it ends up being easier and saner to do this. s *solver - // Simple, local cache of the root's PackageTree - crp *struct { - ptree pkgtree.PackageTree - err error - } - // Map of project root name to their available version list. This cache is // layered on top of the proper SourceManager's cache; the only difference // is that this keeps the versions sorted in the direction required by the diff --git a/internal/gps/cmd.go b/internal/gps/cmd.go index 799f1685c5..6ab6032f11 100644 --- a/internal/gps/cmd.go +++ b/internal/gps/cmd.go @@ -72,14 +72,14 @@ func (c *monitoredCmd) run(ctx context.Context) error { // in at the same time as process completion, but one of the former are // picked first; in such a case, cmd.Process could(?) be nil by the time we // call signal methods on it. - var isDone *int32 = new(int32) + var isDone int32 done := make(chan error, 1) go func() { // Wait() can only be called once, so this must act as the completion // indicator for both normal *and* signal-induced termination. done <- c.cmd.Wait() - atomic.CompareAndSwapInt32(isDone, 0, 1) + atomic.CompareAndSwapInt32(&isDone, 0, 1) }() var killerr error @@ -89,8 +89,8 @@ selloop: case err := <-done: return err case <-ticker.C: - if !atomic.CompareAndSwapInt32(isDone, 1, 1) && c.hasTimedOut() { - if err := killProcess(c.cmd, isDone); err != nil { + if !atomic.CompareAndSwapInt32(&isDone, 1, 1) && c.hasTimedOut() { + if err := killProcess(c.cmd, &isDone); err != nil { killerr = &killCmdError{err} } else { killerr = &noProgressError{c.timeout} @@ -98,8 +98,8 @@ selloop: break selloop } case <-ctx.Done(): - if !atomic.CompareAndSwapInt32(isDone, 1, 1) { - if err := killProcess(c.cmd, isDone); err != nil { + if !atomic.CompareAndSwapInt32(&isDone, 1, 1) { + if err := killProcess(c.cmd, &isDone); err != nil { killerr = &killCmdError{err} } else { killerr = ctx.Err() diff --git a/internal/gps/pkgtree/pkgtree_test.go b/internal/gps/pkgtree/pkgtree_test.go index ab134f02e7..94b86248b8 100644 --- a/internal/gps/pkgtree/pkgtree_test.go +++ b/internal/gps/pkgtree/pkgtree_test.go @@ -32,9 +32,6 @@ func TestWorkmapToReach(t *testing.T) { return make(map[string]bool) } - e := struct { - Internal, External []string - }{} table := map[string]struct { workmap map[string]wm rm ReachMap @@ -49,7 +46,7 @@ func TestWorkmapToReach(t *testing.T) { }, }, rm: ReachMap{ - "foo": e, + "foo": {}, }, }, "no external": { @@ -64,8 +61,8 @@ func TestWorkmapToReach(t *testing.T) { }, }, rm: ReachMap{ - "foo": e, - "foo/bar": e, + "foo": {}, + "foo/bar": {}, }, }, "no external with subpkg": { @@ -85,7 +82,7 @@ func TestWorkmapToReach(t *testing.T) { "foo": { Internal: []string{"foo/bar"}, }, - "foo/bar": e, + "foo/bar": {}, }, }, "simple base transitive": { diff --git a/internal/gps/selection.go b/internal/gps/selection.go index e29d83fe53..8654294ab8 100644 --- a/internal/gps/selection.go +++ b/internal/gps/selection.go @@ -73,10 +73,6 @@ func (s *selection) depperCount(id ProjectIdentifier) int { return len(s.deps[id.ProjectRoot]) } -func (s *selection) setDependenciesOn(id ProjectIdentifier, deps []dependency) { - s.deps[id.ProjectRoot] = deps -} - // Compute a list of the unique packages within the given ProjectIdentifier that // have dependers, and the number of dependers they have. func (s *selection) getRequiredPackagesIn(id ProjectIdentifier) map[string]int { @@ -93,6 +89,9 @@ func (s *selection) getRequiredPackagesIn(id ProjectIdentifier) map[string]int { return uniq } +// Suppress unused warning. +var _ = (*selection)(nil).getSelectedPackagesIn + // Compute a list of the unique packages within the given ProjectIdentifier that // are currently selected, and the number of times each package has been // independently selected. diff --git a/internal/gps/solve_basic_test.go b/internal/gps/solve_basic_test.go index fe5eb7a443..80d3d14d3e 100644 --- a/internal/gps/solve_basic_test.go +++ b/internal/gps/solve_basic_test.go @@ -1353,14 +1353,6 @@ func (sm *depspecSourceManager) GetManifestAndLock(id ProjectIdentifier, v Versi return nil, nil, fmt.Errorf("Project %s at version %s could not be found", id, v) } -func (sm *depspecSourceManager) ExternalReach(id ProjectIdentifier, v Version) (map[string][]string, error) { - pid := pident{n: ProjectRoot(id.normalizedSource()), v: v} - if m, exists := sm.rm[pid]; exists { - return m, nil - } - return nil, fmt.Errorf("No reach data for %s at version %s", id, v) -} - func (sm *depspecSourceManager) ListPackages(id ProjectIdentifier, v Version) (pkgtree.PackageTree, error) { pid := pident{n: ProjectRoot(id.normalizedSource()), v: v} if pv, ok := v.(PairedVersion); ok && pv.Revision() == "FAKEREV" { @@ -1568,10 +1560,6 @@ func (ds depspec) DependencyConstraints() ProjectConstraints { type fixLock []LockedProject -func (fixLock) SolverVersion() string { - return "-1" -} - // impl Lock interface func (fixLock) InputHash() []byte { return []byte("fooooorooooofooorooofoo") @@ -1584,11 +1572,6 @@ func (l fixLock) Projects() []LockedProject { type dummyLock struct{} -// impl Lock interface -func (dummyLock) SolverVersion() string { - return "-1" -} - // impl Lock interface func (dummyLock) InputHash() []byte { return []byte("fooooorooooofooorooofoo") diff --git a/internal/gps/solve_failures.go b/internal/gps/solve_failures.go index 0d281920f5..9865c17ba1 100644 --- a/internal/gps/solve_failures.go +++ b/internal/gps/solve_failures.go @@ -11,17 +11,6 @@ import ( "strings" ) -type errorLevel uint8 - -// TODO(sdboyer) consistent, sensible way of handling 'type' and 'severity' - or figure -// out that they're not orthogonal and collapse into just 'type' - -const ( - warning errorLevel = 1 << iota - mustResolve - cannotResolve -) - func a2vs(a atom) string { if a.v == rootRev || a.v == nil { return "(root)" diff --git a/internal/gps/source_manager.go b/internal/gps/source_manager.go index b8d7d5ac28..ee07237048 100644 --- a/internal/gps/source_manager.go +++ b/internal/gps/source_manager.go @@ -213,9 +213,9 @@ func NewSourceManager(c SourceManagerConfig) (*SourceMgr, error) { lasttime = nowtime } - if _, ok := err.(interface { + if t, ok := err.(interface { Temporary() bool - }); ok { + }); ok && t.Temporary() { time.Sleep(time.Second * 1) } else { return nil, CouldNotCreateLockError{ @@ -677,7 +677,6 @@ const ( ctSourcePing ctSourceInit ctSourceFetch - ctCheckoutVersion ctExportTree ) diff --git a/internal/gps/typed_radix.go b/internal/gps/typed_radix.go index 9c6a9216ac..615f297efd 100644 --- a/internal/gps/typed_radix.go +++ b/internal/gps/typed_radix.go @@ -30,6 +30,9 @@ func newDeducerTrie() *deducerTrie { } } +// Suppress unused warning. +var _ = (*deducerTrie)(nil).Delete + // Delete is used to delete a key, returning the previous value and if it was deleted func (t *deducerTrie) Delete(s string) (pathDeducer, bool) { t.Lock() diff --git a/internal/gps/vcs_repo.go b/internal/gps/vcs_repo.go index 5bc687b5da..3c17c2ea4e 100644 --- a/internal/gps/vcs_repo.go +++ b/internal/gps/vcs_repo.go @@ -53,6 +53,10 @@ func getVCSRepo(s vcs.Type, ustr, path string) (r ctxRepo, err error) { var repo *vcs.HgRepo repo, err = vcs.NewHgRepo(ustr, path) r = &hgRepo{repo} + case vcs.Svn: + var repo *vcs.SvnRepo + repo, err = vcs.NewSvnRepo(ustr, path) + r = &svnRepo{repo} } return @@ -218,7 +222,7 @@ func (r *svnRepo) get(ctx context.Context) error { return nil } -func (r *svnRepo) update(ctx context.Context) error { +func (r *svnRepo) fetch(ctx context.Context) error { out, err := runFromRepoDir(ctx, r, expensiveCmdTimeout, "svn", "update") if err != nil { return newVcsRemoteErrorOr("unable to update repository", err, string(out)) diff --git a/internal/gps/vcs_repo_test.go b/internal/gps/vcs_repo_test.go index c5d6b8b557..343d00ac87 100644 --- a/internal/gps/vcs_repo_test.go +++ b/internal/gps/vcs_repo_test.go @@ -171,7 +171,7 @@ func testSvnRepo(t *testing.T) { } // Perform an update which should take up back to the latest version. - err = repo.update(ctx) + err = repo.fetch(ctx) if err != nil { t.Fatal(err) } diff --git a/internal/gps/vcs_source.go b/internal/gps/vcs_source.go index e8883dc61b..36f86c4dce 100644 --- a/internal/gps/vcs_source.go +++ b/internal/gps/vcs_source.go @@ -570,11 +570,6 @@ func (s *hgSource) listVersions(ctx context.Context) ([]PairedVersion, error) { return vlist, nil } -type repo struct { - // Object for direct repo interaction - r ctxRepo -} - // This func copied from Masterminds/vcs so we can exec our own commands func mergeEnvLists(in, out []string) []string { NextVar: diff --git a/internal/gps/vcs_source_test.go b/internal/gps/vcs_source_test.go index a9bdfdceee..d797f970b8 100644 --- a/internal/gps/vcs_source_test.go +++ b/internal/gps/vcs_source_test.go @@ -25,8 +25,7 @@ func TestSlowVcs(t *testing.T) { t.Run("source-gateway", testSourceGateway) t.Run("bzr-repo", testBzrRepo) t.Run("bzr-source", testBzrSourceInteractions) - // TODO(kris-nova) re-enable syn-repo after gps is merged into dep - //t.Run("svn-repo", testSvnRepo) + t.Run("svn-repo", testSvnRepo) // TODO(sdboyer) svn-source t.Run("hg-repo", testHgRepo) t.Run("hg-source", testHgSourceInteractions) diff --git a/internal/gps/version.go b/internal/gps/version.go index 0462892826..63e925b692 100644 --- a/internal/gps/version.go +++ b/internal/gps/version.go @@ -119,6 +119,9 @@ func (r Revision) String() string { return string(r) } +// ImpliedCaretString follows the same rules as String(), but in accordance with +// the Constraint interface will always print a leading "=", as all Versions, +// when acting as a Constraint, act as exact matches. func (r Revision) ImpliedCaretString() string { return r.String() } diff --git a/internal/gps/version_queue_test.go b/internal/gps/version_queue_test.go index 0bcbea5053..b2ea87881f 100644 --- a/internal/gps/version_queue_test.go +++ b/internal/gps/version_queue_test.go @@ -5,11 +5,12 @@ package gps import ( - "fmt" "testing" + + "github.com/pkg/errors" ) -// just need a ListVersions method +// just need a listVersions method type fakeBridge struct { *bridge vl []Version @@ -27,10 +28,6 @@ func init() { SortForUpgrade(fakevl) } -func (fb *fakeBridge) ListVersions(id ProjectIdentifier) ([]PairedVersion, error) { - return nil, nil -} - func (fb *fakeBridge) listVersions(id ProjectIdentifier) ([]Version, error) { // it's a fixture, we only ever do the one, regardless of id return fb.vl, nil @@ -40,11 +37,7 @@ type fakeFailBridge struct { *bridge } -var errVQ = fmt.Errorf("vqerr") - -func (fb *fakeFailBridge) ListVersions(id ProjectIdentifier) ([]PairedVersion, error) { - return nil, nil -} +var errVQ = errors.New("vqerr") func (fb *fakeFailBridge) listVersions(id ProjectIdentifier) ([]Version, error) { return nil, errVQ @@ -152,7 +145,7 @@ func TestVersionQueueAdvance(t *testing.T) { } for k, v := range fakevl[1:] { - err = vq.advance(fmt.Errorf("advancment fail for %s", fakevl[k])) + err = vq.advance(errors.Errorf("advancment fail for %s", fakevl[k])) if err != nil { t.Errorf("error on advancing vq from %s to %s", fakevl[k], v) break @@ -166,7 +159,7 @@ func TestVersionQueueAdvance(t *testing.T) { if vq.isExhausted() { t.Error("should not be exhausted until advancing 'past' the end") } - if err = vq.advance(fmt.Errorf("final advance failure")); err != nil { + if err = vq.advance(errors.Errorf("final advance failure")); err != nil { t.Errorf("should not error on advance, even past end, but got %s", err) } @@ -191,7 +184,7 @@ func TestVersionQueueAdvance(t *testing.T) { t.Error("can't be exhausted, we aren't even 'allLoaded' yet") } - err = vq.advance(fmt.Errorf("dequeue lockv")) + err = vq.advance(errors.Errorf("dequeue lockv")) if err != nil { t.Error("unexpected error when advancing past lockv", err) } else { @@ -203,7 +196,7 @@ func TestVersionQueueAdvance(t *testing.T) { } } - err = vq.advance(fmt.Errorf("dequeue prefv")) + err = vq.advance(errors.Errorf("dequeue prefv")) if err != nil { t.Error("unexpected error when advancing past prefv", err) } else { diff --git a/internal/test/integration/testproj.go b/internal/test/integration/testproj.go index c32aa1a6bb..ebd07a76c6 100644 --- a/internal/test/integration/testproj.go +++ b/internal/test/integration/testproj.go @@ -31,7 +31,6 @@ type RunFunc func(prog string, newargs []string, outW, errW io.Writer, dir strin // and content type TestProject struct { t *testing.T - h *test.Helper preImports []string tempdir string env []string diff --git a/internal/test/test.go b/internal/test/test.go index 1b1eb5ec06..9a7fbc8d98 100644 --- a/internal/test/test.go +++ b/internal/test/test.go @@ -24,10 +24,13 @@ import ( ) var ( - ExeSuffix string // ".exe" on Windows - mu sync.Mutex - PrintLogs *bool = flag.Bool("logs", false, "log stdin/stdout of test commands") - UpdateGolden *bool = flag.Bool("update", false, "update golden files") + // ExeSuffix is the suffix of executable files; ".exe" on Windows. + ExeSuffix string + mu sync.Mutex + // PrintLogs controls logging of test commands. + PrintLogs = flag.Bool("logs", false, "log stdin/stdout of test commands") + // UpdateGolden controls updating test fixtures. + UpdateGolden = flag.Bool("update", false, "update golden files") ) const ( @@ -191,7 +194,7 @@ func (h *Helper) DoRun(args []string) error { return errors.Wrapf(status, "Error running %s\n%s", strings.Join(newargs, " "), h.stderr.String()) } -// run runs the test go command, and expects it to succeed. +// Run runs the test go command, and expects it to succeed. func (h *Helper) Run(args ...string) { if runtime.GOOS == "windows" { mu.Lock() @@ -446,7 +449,7 @@ func (h *Helper) WriteTestFile(src string, content string) error { return err } -// GetTestFile reads a file into memory +// GetFile reads a file into memory func (h *Helper) GetFile(path string) io.ReadCloser { content, err := os.Open(path) if err != nil { @@ -608,6 +611,8 @@ func (h *Helper) ReadLock() string { return string(f) } +// GetCommit treats repo as a path to a git repository and returns the current +// revision. func (h *Helper) GetCommit(repo string) string { repoPath := h.Path("pkg/dep/sources/https---" + strings.Replace(repo, "/", "-", -1)) cmd := exec.Command("git", "rev-parse", "HEAD")