diff --git a/cmd/dep/base_importer.go b/cmd/dep/base_importer.go new file mode 100644 index 0000000000..8e81e8bf42 --- /dev/null +++ b/cmd/dep/base_importer.go @@ -0,0 +1,277 @@ +// Copyright 2017 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "log" + + "github.com/golang/dep" + fb "github.com/golang/dep/internal/feedback" + "github.com/golang/dep/internal/gps" + "github.com/pkg/errors" +) + +// baseImporter provides a common implementation for importing from other +// dependency managers. +type baseImporter struct { + logger *log.Logger + verbose bool + sm gps.SourceManager + manifest *dep.Manifest + lock *dep.Lock +} + +// newBaseImporter creates a new baseImporter for embedding in an importer. +func newBaseImporter(logger *log.Logger, verbose bool, sm gps.SourceManager) *baseImporter { + return &baseImporter{ + logger: logger, + verbose: verbose, + manifest: dep.NewManifest(), + lock: &dep.Lock{}, + sm: sm, + } +} + +// isTag determines if the specified value is a tag (plain or semver). +func (i *baseImporter) isTag(pi gps.ProjectIdentifier, value string) (bool, gps.Version, error) { + versions, err := i.sm.ListVersions(pi) + if err != nil { + return false, nil, errors.Wrapf(err, "unable to list versions for %s(%s)", pi.ProjectRoot, pi.Source) + } + + for _, version := range versions { + if version.Type() != gps.IsVersion && version.Type() != gps.IsSemver { + continue + } + + if value == version.String() { + return true, version, nil + } + } + + return false, nil, nil +} + +// lookupVersionForLockedProject figures out the appropriate version for a locked +// project based on the locked revision and the constraint from the manifest. +// First try matching the revision to a version, then try the constraint from the +// manifest, then finally the revision. +func (i *baseImporter) lookupVersionForLockedProject(pi gps.ProjectIdentifier, c gps.Constraint, rev gps.Revision) (gps.Version, error) { + // Find the version that goes with this revision, if any + versions, err := i.sm.ListVersions(pi) + if err != nil { + return rev, errors.Wrapf(err, "Unable to lookup the version represented by %s in %s(%s). Falling back to locking the revision only.", rev, pi.ProjectRoot, pi.Source) + } + + var branchConstraint gps.PairedVersion + gps.SortPairedForUpgrade(versions) // Sort versions in asc order + matches := []gps.Version{} + for _, v := range versions { + if v.Revision() == rev { + matches = append(matches, v) + } + if c != nil && v.Type() == gps.IsBranch && v.String() == c.String() { + branchConstraint = v + } + } + + // Try to narrow down the matches with the constraint. Otherwise return the first match. + if len(matches) > 0 { + if c != nil { + for _, v := range matches { + if i.testConstraint(c, v) { + return v, nil + } + } + } + return matches[0], nil + } + + // Use branch constraint from the manifest + if branchConstraint != nil { + return branchConstraint.Unpair().Pair(rev), nil + } + + // Give up and lock only to a revision + return rev, nil +} + +// importedPackage is a common intermediate representation of a package imported +// from an external tool's configuration. +type importedPackage struct { + // Required. The package path, not necessarily the project root. + Name string + + // Required. Text representing a revision or tag. + LockHint string + + // Optional. Alternative source, or fork, for the project. + Source string + + // Optional. Text representing a branch or version. + ConstraintHint string +} + +// importedProject is a consolidated representation of a set of imported packages +// for the same project root. +type importedProject struct { + Root gps.ProjectRoot + importedPackage +} + +// loadPackages consolidates all package references into a set of project roots. +func (i *baseImporter) loadPackages(packages []importedPackage) ([]importedProject, error) { + // preserve the original order of the packages so that messages that + // are printed as they are processed are in a consistent order. + orderedProjects := make([]importedProject, 0, len(packages)) + + projects := make(map[gps.ProjectRoot]*importedProject, len(packages)) + for _, pkg := range packages { + pr, err := i.sm.DeduceProjectRoot(pkg.Name) + if err != nil { + return nil, errors.Wrapf(err, "Cannot determine the project root for %s", pkg.Name) + } + pkg.Name = string(pr) + + prj, exists := projects[pr] + if !exists { + prj := importedProject{pr, pkg} + orderedProjects = append(orderedProjects, prj) + projects[pr] = &orderedProjects[len(orderedProjects)-1] + continue + } + + // The config found first "wins", though we allow for incrementally + // setting each field because some importers have a config and lock file. + if prj.Source == "" && pkg.Source != "" { + prj.Source = pkg.Source + } + + if prj.ConstraintHint == "" && pkg.ConstraintHint != "" { + prj.ConstraintHint = pkg.ConstraintHint + } + + if prj.LockHint == "" && pkg.LockHint != "" { + prj.LockHint = pkg.LockHint + } + } + + return orderedProjects, nil +} + +// importPackages loads imported packages into the manifest and lock. +// - defaultConstraintFromLock specifies if a constraint should be defaulted +// based on the locked version when there wasn't a constraint hint. +// +// Rules: +// * When a constraint is ignored, default to *. +// * HEAD revisions default to the matching branch. +// * Semantic versions default to ^VERSION. +// * Revision constraints are ignored. +// * Versions that don't satisfy the constraint, drop the constraint. +// * Untagged revisions ignore non-branch constraint hints. +func (i *baseImporter) importPackages(packages []importedPackage, defaultConstraintFromLock bool) (err error) { + projects, err := i.loadPackages(packages) + if err != nil { + return err + } + + for _, prj := range projects { + pc := gps.ProjectConstraint{ + Ident: gps.ProjectIdentifier{ + ProjectRoot: prj.Root, + Source: prj.Source, + }, + } + + pc.Constraint, err = i.sm.InferConstraint(prj.ConstraintHint, pc.Ident) + if err != nil { + pc.Constraint = gps.Any() + } + + var version gps.Version + if prj.LockHint != "" { + var isTag bool + // Determine if the lock hint is a revision or tag + isTag, version, err = i.isTag(pc.Ident, prj.LockHint) + if err != nil { + return err + } + + // If the hint is a revision, check if it is tagged + if !isTag { + revision := gps.Revision(prj.LockHint) + version, err = i.lookupVersionForLockedProject(pc.Ident, pc.Constraint, revision) + if err != nil { + version = nil + i.logger.Println(err) + } + } + + // Default the constraint based on the locked version + if defaultConstraintFromLock && prj.ConstraintHint == "" && version != nil { + props := getProjectPropertiesFromVersion(version) + if props.Constraint != nil { + pc.Constraint = props.Constraint + } + } + } + + // Ignore pinned constraints + if i.isConstraintPinned(pc.Constraint) { + if i.verbose { + i.logger.Printf(" Ignoring pinned constraint %v for %v.\n", pc.Constraint, pc.Ident) + } + pc.Constraint = gps.Any() + } + + // Ignore constraints which conflict with the locked revision, so that + // solve doesn't later change the revision to satisfy the constraint. + if !i.testConstraint(pc.Constraint, version) { + if i.verbose { + i.logger.Printf(" Ignoring constraint %v for %v because it would invalidate the locked version %v.\n", pc.Constraint, pc.Ident, version) + } + pc.Constraint = gps.Any() + } + + i.manifest.Constraints[pc.Ident.ProjectRoot] = gps.ProjectProperties{ + Source: pc.Ident.Source, + Constraint: pc.Constraint, + } + fb.NewConstraintFeedback(pc, fb.DepTypeImported).LogFeedback(i.logger) + + if version != nil { + lp := gps.NewLockedProject(pc.Ident, version, nil) + i.lock.P = append(i.lock.P, lp) + fb.NewLockedProjectFeedback(lp, fb.DepTypeImported).LogFeedback(i.logger) + } + } + + return nil +} + +// isConstraintPinned returns if a constraint is pinned to a specific revision. +func (i *baseImporter) isConstraintPinned(c gps.Constraint) bool { + if version, isVersion := c.(gps.Version); isVersion { + switch version.Type() { + case gps.IsRevision, gps.IsVersion: + return true + } + } + return false +} + +// testConstraint verifies that the constraint won't invalidate the locked version. +func (i *baseImporter) testConstraint(c gps.Constraint, v gps.Version) bool { + // Assume branch constraints are satisfied + if version, isVersion := c.(gps.Version); isVersion { + if version.Type() == gps.IsBranch { + + return true + } + } + + return c.Matches(v) +} diff --git a/cmd/dep/base_importer_test.go b/cmd/dep/base_importer_test.go new file mode 100644 index 0000000000..011df83461 --- /dev/null +++ b/cmd/dep/base_importer_test.go @@ -0,0 +1,571 @@ +// Copyright 2016 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package main + +import ( + "bytes" + "log" + "sort" + "strings" + "testing" + + "github.com/golang/dep" + "github.com/golang/dep/internal/gps" + "github.com/golang/dep/internal/test" + "github.com/pkg/errors" +) + +const ( + importerTestProject = "github.com/carolynvs/deptest-importers" + importerTestProjectSrc = "https://github.com/carolynvs/deptest-importers.git" + importerTestUntaggedRev = "9b670d143bfb4a00f7461451d5c4a62f80e9d11d" + importerTestUntaggedRevAbbrv = "v1.0.0-1-g9b670d1" + importerTestBeta1Tag = "beta1" + importerTestBeta1Rev = "7913ab26988c6fb1e16225f845a178e8849dd254" + importerTestV2Branch = "v2" + importerTestV2Rev = "45dcf5a09c64b48b6e836028a3bc672b19b9d11d" + importerTestV2PatchTag = "v2.0.0-alpha1" + importerTestV2PatchRev = "347760b50204948ea63e531dd6560e56a9adde8f" + importerTestV1Tag = "v1.0.0" + importerTestV1Rev = "d0c29640b17f77426b111f4c1640d716591aa70e" + importerTestV1PatchTag = "v1.0.2" + importerTestV1PatchRev = "788963efe22e3e6e24c776a11a57468bb2fcd780" + importerTestV1Constraint = "^1.0.0" + importerTestMultiTaggedRev = "34cf993cc346f65601fe4356dd68bd54d20a1bfe" + importerTestMultiTaggedSemverTag = "v1.0.4" + importerTestMultiTaggedPlainTag = "stable" +) + +func TestBaseImporter_IsTag(t *testing.T) { + testcases := map[string]struct { + input string + wantIsTag bool + wantTag gps.Version + }{ + "non-semver tag": { + input: importerTestBeta1Tag, + wantIsTag: true, + wantTag: gps.NewVersion(importerTestBeta1Tag).Pair(importerTestBeta1Rev), + }, + "semver-tag": { + input: importerTestV1PatchTag, + wantIsTag: true, + wantTag: gps.NewVersion(importerTestV1PatchTag).Pair(importerTestV1PatchRev)}, + "untagged revision": { + input: importerTestUntaggedRev, + wantIsTag: false, + }, + "branch name": { + input: importerTestV2Branch, + wantIsTag: false, + }, + "empty": { + input: "", + wantIsTag: false, + }, + } + + pi := gps.ProjectIdentifier{ProjectRoot: importerTestProject} + + for name, tc := range testcases { + name := name + tc := tc + t.Run(name, func(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + h.Parallel() + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + i := newBaseImporter(discardLogger, false, sm) + gotIsTag, gotTag, err := i.isTag(pi, tc.input) + h.Must(err) + + if tc.wantIsTag != gotIsTag { + t.Fatalf("unexpected isTag result for %v: \n\t(GOT) %v \n\t(WNT) %v", + tc.input, gotIsTag, tc.wantIsTag) + } + + if tc.wantTag != gotTag { + t.Fatalf("unexpected tag for %v: \n\t(GOT) %v \n\t(WNT) %v", + tc.input, gotTag, tc.wantTag) + } + }) + } +} + +func TestBaseImporter_LookupVersionForLockedProject(t *testing.T) { + testcases := map[string]struct { + revision gps.Revision + constraint gps.Constraint + wantVersion string + }{ + "match revision to tag": { + revision: importerTestV1PatchRev, + wantVersion: importerTestV1PatchTag, + }, + "match revision with multiple tags using constraint": { + revision: importerTestMultiTaggedRev, + constraint: gps.NewVersion(importerTestMultiTaggedPlainTag), + wantVersion: importerTestMultiTaggedPlainTag, + }, + "revision with multiple tags with no constraint defaults to best match": { + revision: importerTestMultiTaggedRev, + wantVersion: importerTestMultiTaggedSemverTag, + }, + "revision with multiple tags with nonmatching constraint defaults to best match": { + revision: importerTestMultiTaggedRev, + constraint: gps.NewVersion("thismatchesnothing"), + wantVersion: importerTestMultiTaggedSemverTag, + }, + "untagged revision fallback to branch constraint": { + revision: importerTestUntaggedRev, + constraint: gps.NewBranch("master"), + wantVersion: "master", + }, + "fallback to revision": { + revision: importerTestUntaggedRev, + wantVersion: importerTestUntaggedRev, + }, + } + + pi := gps.ProjectIdentifier{ProjectRoot: importerTestProject} + + for name, tc := range testcases { + name := name + tc := tc + t.Run(name, func(t *testing.T) { + h := test.NewHelper(t) + defer h.Cleanup() + h.Parallel() + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + i := newBaseImporter(discardLogger, false, sm) + v, err := i.lookupVersionForLockedProject(pi, tc.constraint, tc.revision) + h.Must(err) + + gotVersion := v.String() + if gotVersion != tc.wantVersion { + t.Fatalf("unexpected locked version: \n\t(GOT) %v\n\t(WNT) %v", gotVersion, tc.wantVersion) + } + }) + } +} + +func TestBaseImporter_ImportProjects(t *testing.T) { + testcases := map[string]struct { + convertTestCase + projects []importedPackage + }{ + "tag constraints are ignored": { + convertTestCase{ + wantConstraint: "*", + wantVersion: importerTestBeta1Tag, + wantRevision: importerTestBeta1Rev, + }, + []importedPackage{ + { + Name: importerTestProject, + LockHint: importerTestBeta1Rev, + ConstraintHint: importerTestBeta1Tag, + }, + }, + }, + "tag lock hints lock to tagged revision": { + convertTestCase{ + wantConstraint: "*", + wantVersion: importerTestBeta1Tag, + wantRevision: importerTestBeta1Rev, + }, + []importedPackage{ + { + Name: importerTestProject, + LockHint: importerTestBeta1Tag, + }, + }, + }, + "untagged revision ignores range constraint": { + convertTestCase{ + wantConstraint: "*", + wantRevision: importerTestUntaggedRev, + }, + []importedPackage{ + { + Name: importerTestProject, + LockHint: importerTestUntaggedRev, + ConstraintHint: importerTestV1Constraint, + }, + }, + }, + "untagged revision keeps branch constraint": { + convertTestCase{ + wantConstraint: "master", + wantVersion: "master", + wantRevision: importerTestUntaggedRev, + }, + []importedPackage{ + { + Name: importerTestProject, + LockHint: importerTestUntaggedRev, + ConstraintHint: "master", + }, + }, + }, + "HEAD revisions default constraint to the matching branch": { + convertTestCase{ + defaultConstraintFromLock: true, + wantConstraint: importerTestV2Branch, + wantVersion: importerTestV2Branch, + wantRevision: importerTestV2Rev, + }, + []importedPackage{ + { + Name: importerTestProject, + LockHint: importerTestV2Rev, + }, + }, + }, + "Semver tagged revisions default to ^VERSION": { + convertTestCase{ + defaultConstraintFromLock: true, + wantConstraint: importerTestV1Constraint, + wantVersion: importerTestV1Tag, + wantRevision: importerTestV1Rev, + }, + []importedPackage{ + { + Name: importerTestProject, + LockHint: importerTestV1Rev, + }, + }, + }, + "Semver lock hint defaults constraint to ^VERSION": { + convertTestCase{ + defaultConstraintFromLock: true, + wantConstraint: importerTestV1Constraint, + wantVersion: importerTestV1Tag, + wantRevision: importerTestV1Rev, + }, + []importedPackage{ + { + Name: importerTestProject, + LockHint: importerTestV1Tag, + }, + }, + }, + "Semver constraint hint": { + convertTestCase{ + wantConstraint: importerTestV1Constraint, + wantVersion: importerTestV1PatchTag, + wantRevision: importerTestV1PatchRev, + }, + []importedPackage{ + { + Name: importerTestProject, + LockHint: importerTestV1PatchRev, + ConstraintHint: importerTestV1Constraint, + }, + }, + }, + "Semver prerelease lock hint": { + convertTestCase{ + wantConstraint: importerTestV2Branch, + wantVersion: importerTestV2PatchTag, + wantRevision: importerTestV2PatchRev, + }, + []importedPackage{ + { + Name: importerTestProject, + LockHint: importerTestV2PatchRev, + ConstraintHint: importerTestV2Branch, + }, + }, + }, + "Revision constraints are ignored": { + convertTestCase{ + wantConstraint: "*", + wantVersion: importerTestV1Tag, + wantRevision: importerTestV1Rev, + }, + []importedPackage{ + { + Name: importerTestProject, + LockHint: importerTestV1Rev, + ConstraintHint: importerTestV1Rev, + }, + }, + }, + "Branch constraint hint": { + convertTestCase{ + wantConstraint: "master", + wantVersion: importerTestV1Tag, + wantRevision: importerTestV1Rev, + }, + []importedPackage{ + { + Name: importerTestProject, + LockHint: importerTestV1Rev, + ConstraintHint: "master", + }, + }, + }, + "Non-matching semver constraint is ignored": { + convertTestCase{ + wantConstraint: "*", + wantVersion: importerTestV1Tag, + wantRevision: importerTestV1Rev, + }, + []importedPackage{ + { + Name: importerTestProject, + LockHint: importerTestV1Rev, + ConstraintHint: "^2.0.0", + }, + }, + }, + "git describe constraint is ignored": { + convertTestCase{ + wantConstraint: "*", + wantRevision: importerTestUntaggedRev, + }, + []importedPackage{ + { + Name: importerTestProject, + LockHint: importerTestUntaggedRev, + ConstraintHint: importerTestUntaggedRevAbbrv, + }, + }, + }, + "consolidate subpackages under root": { + convertTestCase{ + wantConstraint: "master", + wantVersion: "master", + wantRevision: importerTestUntaggedRev, + }, + []importedPackage{ + { + Name: importerTestProject + "/subpkA", + ConstraintHint: "master", + }, + { + Name: importerTestProject, + LockHint: importerTestUntaggedRev, + }, + }, + }, + "ignore duplicate packages": { + convertTestCase{ + wantConstraint: "*", + wantRevision: importerTestUntaggedRev, + }, + []importedPackage{ + { + Name: importerTestProject + "/subpkgA", + LockHint: importerTestUntaggedRev, // first wins + }, + { + Name: importerTestProject + "/subpkgB", + LockHint: importerTestV1Rev, + }, + }, + }, + "skip empty lock hints": { + convertTestCase{ + wantConstraint: "*", + wantRevision: "", + }, + []importedPackage{ + { + Name: importerTestProject, + LockHint: "", + }, + }, + }, + } + + for name, tc := range testcases { + name := name + tc := tc + t.Run(name, func(t *testing.T) { + t.Parallel() + + err := tc.Exec(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock, error) { + i := newBaseImporter(logger, true, sm) + convertErr := i.importPackages(tc.projects, tc.defaultConstraintFromLock) + return i.manifest, i.lock, convertErr + }) + if err != nil { + t.Fatalf("%#v", err) + } + }) + } +} + +// convertTestCase is a common set of validations applied to the result +// of an importer converting from an external config format to dep's. +type convertTestCase struct { + defaultConstraintFromLock bool + wantConvertErr bool + wantSourceRepo string + wantConstraint string + wantRevision gps.Revision + wantVersion string + wantIgnored []string + wantWarning string +} + +func (tc convertTestCase) Exec(t *testing.T, convert func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock, error)) error { + h := test.NewHelper(t) + defer h.Cleanup() + + ctx := newTestContext(h) + sm, err := ctx.SourceManager() + h.Must(err) + defer sm.Release() + + // Capture stderr so we can verify warnings + output := &bytes.Buffer{} + ctx.Err = log.New(output, "", 0) + + manifest, lock, convertErr := convert(ctx.Err, sm) + return tc.validate(manifest, lock, convertErr, output) +} + +// validate returns an error if any of the testcase validations failed. +func (tc convertTestCase) validate(manifest *dep.Manifest, lock *dep.Lock, convertErr error, output *bytes.Buffer) error { + if tc.wantConvertErr { + if convertErr == nil { + return errors.New("Expected the conversion to fail, but it did not return an error") + } + return nil + } + + if convertErr != nil { + return errors.Wrap(convertErr, "Expected the conversion to pass, but it returned an error") + } + + if !equalSlice(manifest.Ignored, tc.wantIgnored) { + return errors.Errorf("unexpected set of ignored projects: \n\t(GOT) %v \n\t(WNT) %v", + manifest.Ignored, tc.wantIgnored) + } + + wantConstraintCount := 0 + if tc.wantConstraint != "" { + wantConstraintCount = 1 + } + gotConstraintCount := len(manifest.Constraints) + if gotConstraintCount != wantConstraintCount { + return errors.Errorf("unexpected number of constraints: \n\t(GOT) %v \n\t(WNT) %v", + gotConstraintCount, wantConstraintCount) + } + + if tc.wantConstraint != "" { + d, ok := manifest.Constraints[importerTestProject] + if !ok { + return errors.Errorf("Expected the manifest to have a dependency for '%v'", + importerTestProject) + } + + gotConstraint := d.Constraint.String() + if gotConstraint != tc.wantConstraint { + return errors.Errorf("unexpected constraint: \n\t(GOT) %v \n\t(WNT) %v", + gotConstraint, tc.wantConstraint) + } + + } + + // Lock checks. + wantLockCount := 0 + if tc.wantRevision != "" { + wantLockCount = 1 + } + gotLockCount := 0 + if lock != nil { + gotLockCount = len(lock.P) + } + if gotLockCount != wantLockCount { + return errors.Errorf("unexpected number of locked projects: \n\t(GOT) %v \n\t(WNT) %v", + gotLockCount, wantLockCount) + } + + if tc.wantRevision != "" { + lp := lock.P[0] + + gotProjectRoot := lp.Ident().ProjectRoot + if gotProjectRoot != importerTestProject { + return errors.Errorf("unexpected root project in lock: \n\t(GOT) %v \n\t(WNT) %v", + gotProjectRoot, importerTestProject) + } + + gotSource := lp.Ident().Source + if gotSource != tc.wantSourceRepo { + return errors.Errorf("unexpected source repository: \n\t(GOT) %v \n\t(WNT) %v", + gotSource, tc.wantSourceRepo) + } + + // Break down the locked "version" into a version (optional) and revision + var gotVersion string + var gotRevision gps.Revision + if lpv, ok := lp.Version().(gps.PairedVersion); ok { + gotVersion = lpv.String() + gotRevision = lpv.Revision() + } else if lr, ok := lp.Version().(gps.Revision); ok { + gotRevision = lr + } else { + return errors.New("could not determine the type of the locked version") + } + + if gotRevision != tc.wantRevision { + return errors.Errorf("unexpected locked revision: \n\t(GOT) %v \n\t(WNT) %v", + gotRevision, + tc.wantRevision) + } + if gotVersion != tc.wantVersion { + return errors.Errorf("unexpected locked version: \n\t(GOT) %v \n\t(WNT) %v", + gotVersion, + tc.wantVersion) + } + } + + if tc.wantWarning != "" { + if !strings.Contains(output.String(), tc.wantWarning) { + return errors.Errorf("Expected the output to include the warning '%s'", tc.wantWarning) + } + } + + return nil +} + +// equalSlice is comparing two string slices for equality. +func equalSlice(a, b []string) bool { + if a == nil && b == nil { + return true + } + + if a == nil || b == nil { + return false + } + + if len(a) != len(b) { + return false + } + + sort.Strings(a) + sort.Strings(b) + for i := range a { + if a[i] != b[i] { + return false + } + } + + return true +} diff --git a/cmd/dep/glide_importer.go b/cmd/dep/glide_importer.go index 17503cfec3..e5dae70bff 100644 --- a/cmd/dep/glide_importer.go +++ b/cmd/dep/glide_importer.go @@ -14,7 +14,6 @@ import ( "github.com/go-yaml/yaml" "github.com/golang/dep" - fb "github.com/golang/dep/internal/feedback" "github.com/golang/dep/internal/fs" "github.com/golang/dep/internal/gps" "github.com/pkg/errors" @@ -25,20 +24,14 @@ const glideLockName = "glide.lock" // glideImporter imports glide configuration into the dep configuration format. type glideImporter struct { - yaml glideYaml - lock *glideLock - - logger *log.Logger - verbose bool - sm gps.SourceManager + *baseImporter + glideConfig glideYaml + glideLock glideLock + lockFound bool } func newGlideImporter(logger *log.Logger, verbose bool, sm gps.SourceManager) *glideImporter { - return &glideImporter{ - logger: logger, - verbose: verbose, - sm: sm, - } + return &glideImporter{baseImporter: newBaseImporter(logger, verbose, sm)} } type glideYaml struct { @@ -56,7 +49,7 @@ type glideLock struct { type glidePackage struct { Name string `yaml:"package"` - Reference string `yaml:"version"` + Reference string `yaml:"version"` // could contain a semver, tag or branch Repository string `yaml:"repo"` // Unsupported fields that we will warn if used @@ -67,7 +60,7 @@ type glidePackage struct { type glideLockedPackage struct { Name string `yaml:"name"` - Reference string `yaml:"version"` + Revision string `yaml:"version"` Repository string `yaml:"repo"` } @@ -103,11 +96,11 @@ func (g *glideImporter) load(projectDir string) error { } yb, err := ioutil.ReadFile(y) if err != nil { - return errors.Wrapf(err, "Unable to read %s", y) + return errors.Wrapf(err, "unable to read %s", y) } - err = yaml.Unmarshal(yb, &g.yaml) + err = yaml.Unmarshal(yb, &g.glideConfig) if err != nil { - return errors.Wrapf(err, "Unable to parse %s", y) + return errors.Wrapf(err, "unable to parse %s", y) } l := filepath.Join(projectDir, glideLockName) @@ -115,16 +108,17 @@ func (g *glideImporter) load(projectDir string) error { if g.verbose { g.logger.Printf(" Loading %s", l) } + g.lockFound = true lb, err := ioutil.ReadFile(l) if err != nil { - return errors.Wrapf(err, "Unable to read %s", l) + return errors.Wrapf(err, "unable to read %s", l) } - lock := &glideLock{} - err = yaml.Unmarshal(lb, lock) + lock := glideLock{} + err = yaml.Unmarshal(lb, &lock) if err != nil { - return errors.Wrapf(err, "Unable to parse %s", l) + return errors.Wrapf(err, "unable to parse %s", l) } - g.lock = lock + g.glideLock = lock } return nil @@ -135,114 +129,72 @@ func (g *glideImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, e projectName := string(pr) task := bytes.NewBufferString("Converting from glide.yaml") - if g.lock != nil { + if g.lockFound { task.WriteString(" and glide.lock") } task.WriteString("...") g.logger.Println(task) - manifest := dep.NewManifest() - - for _, pkg := range append(g.yaml.Imports, g.yaml.TestImports...) { - pc, err := g.buildProjectConstraint(pkg) - if err != nil { - return nil, nil, err - } - - if _, isDup := manifest.Constraints[pc.Ident.ProjectRoot]; isDup { - continue - } - - manifest.Constraints[pc.Ident.ProjectRoot] = gps.ProjectProperties{Source: pc.Ident.Source, Constraint: pc.Constraint} - } - - manifest.Ignored = append(manifest.Ignored, g.yaml.Ignores...) + numPkgs := len(g.glideConfig.Imports) + len(g.glideConfig.TestImports) + len(g.glideLock.Imports) + len(g.glideLock.TestImports) + packages := make([]importedPackage, 0, numPkgs) - if len(g.yaml.ExcludeDirs) > 0 { - if g.yaml.Name != "" && g.yaml.Name != projectName { - g.logger.Printf(" Glide thinks the package is '%s' but dep thinks it is '%s', using dep's value.\n", g.yaml.Name, projectName) + // Constraints + for _, pkg := range append(g.glideConfig.Imports, g.glideConfig.TestImports...) { + // Validate + if pkg.Name == "" { + return nil, nil, errors.New("invalid glide configuration: Name is required") } - for _, dir := range g.yaml.ExcludeDirs { - pkg := path.Join(projectName, dir) - manifest.Ignored = append(manifest.Ignored, pkg) - } - } - - var lock *dep.Lock - if g.lock != nil { - lock = &dep.Lock{} - - for _, pkg := range append(g.lock.Imports, g.lock.TestImports...) { - lp, err := g.buildLockedProject(pkg, manifest) - if err != nil { - return nil, nil, err + // Warn + if g.verbose { + if pkg.OS != "" { + g.logger.Printf(" The %s package specified an os, but that isn't supported by dep yet, and will be ignored. See https://github.com/golang/dep/issues/291.\n", pkg.Name) } - - if projectExistsInLock(lock, lp.Ident().ProjectRoot) { - continue + if pkg.Arch != "" { + g.logger.Printf(" The %s package specified an arch, but that isn't supported by dep yet, and will be ignored. See https://github.com/golang/dep/issues/291.\n", pkg.Name) } - - lock.P = append(lock.P, lp) } - } - return manifest, lock, nil -} - -func (g *glideImporter) buildProjectConstraint(pkg glidePackage) (pc gps.ProjectConstraint, err error) { - pr, err := g.sm.DeduceProjectRoot(pkg.Name) - if err != nil { - return - } - - pc.Ident = gps.ProjectIdentifier{ - ProjectRoot: pr, - Source: pkg.Repository, - } - pc.Constraint, err = g.sm.InferConstraint(pkg.Reference, pc.Ident) - if err != nil { - return + ip := importedPackage{ + Name: pkg.Name, + Source: pkg.Repository, + ConstraintHint: pkg.Reference, + } + packages = append(packages, ip) } - if g.verbose { - if pkg.OS != "" { - g.logger.Printf(" The %s package specified an os, but that isn't supported by dep yet, and will be ignored. See https://github.com/golang/dep/issues/291.\n", pkg.Name) + // Locks + for _, pkg := range append(g.glideLock.Imports, g.glideLock.TestImports...) { + // Validate + if pkg.Name == "" { + return nil, nil, errors.New("invalid glide lock: Name is required") } - if pkg.Arch != "" { - g.logger.Printf(" The %s package specified an arch, but that isn't supported by dep yet, and will be ignored. See https://github.com/golang/dep/issues/291.\n", pkg.Name) + + ip := importedPackage{ + Name: pkg.Name, + Source: pkg.Repository, + LockHint: pkg.Revision, } + packages = append(packages, ip) } - f := fb.NewConstraintFeedback(pc, fb.DepTypeImported) - f.LogFeedback(g.logger) - - return -} - -func (g *glideImporter) buildLockedProject(pkg glideLockedPackage, manifest *dep.Manifest) (lp gps.LockedProject, err error) { - pr, err := g.sm.DeduceProjectRoot(pkg.Name) + err := g.importPackages(packages, false) if err != nil { - return + return nil, nil, errors.Wrap(err, "invalid glide configuration") } - pi := gps.ProjectIdentifier{ - ProjectRoot: pr, - Source: pkg.Repository, - } - revision := gps.Revision(pkg.Reference) - pp := manifest.Constraints[pi.ProjectRoot] + // Ignores + g.manifest.Ignored = append(g.manifest.Ignored, g.glideConfig.Ignores...) + if len(g.glideConfig.ExcludeDirs) > 0 { + if g.glideConfig.Name != "" && g.glideConfig.Name != projectName { + g.logger.Printf(" Glide thinks the package is '%s' but dep thinks it is '%s', using dep's value.\n", g.glideConfig.Name, projectName) + } - version, err := lookupVersionForLockedProject(pi, pp.Constraint, revision, g.sm) - if err != nil { - // Only warn about the problem, it is not enough to warrant failing - g.logger.Println(err.Error()) + for _, dir := range g.glideConfig.ExcludeDirs { + pkg := path.Join(projectName, dir) + g.manifest.Ignored = append(g.manifest.Ignored, pkg) + } } - lp = gps.NewLockedProject(pi, version, nil) - - f := fb.NewLockedProjectFeedback(lp, fb.DepTypeImported) - f.LogFeedback(g.logger) - - return + return g.manifest, g.lock, nil } diff --git a/cmd/dep/glide_importer_test.go b/cmd/dep/glide_importer_test.go index 11a813499c..d5d4c1b2e2 100644 --- a/cmd/dep/glide_importer_test.go +++ b/cmd/dep/glide_importer_test.go @@ -9,7 +9,6 @@ import ( "io/ioutil" "log" "path/filepath" - "strings" "testing" "github.com/golang/dep" @@ -34,147 +33,157 @@ func newTestContext(h *test.Helper) *dep.Ctx { func TestGlideConfig_Convert(t *testing.T) { testCases := map[string]struct { - *convertTestCase yaml glideYaml - lock *glideLock + lock glideLock + convertTestCase }{ "project": { - yaml: glideYaml{ + glideYaml{ Imports: []glidePackage{ { - Name: "github.com/sdboyer/deptest", - Repository: "https://github.com/sdboyer/deptest.git", - Reference: "v1.0.0", + Name: importerTestProject, + Repository: importerTestProjectSrc, + Reference: importerTestV2Branch, }, }, }, - lock: &glideLock{ + glideLock{ Imports: []glideLockedPackage{ { - Name: "github.com/sdboyer/deptest", - Repository: "https://github.com/sdboyer/deptest.git", - Reference: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + Name: importerTestProject, + Repository: importerTestProjectSrc, + Revision: importerTestV2PatchRev, }, }, }, - convertTestCase: &convertTestCase{ - projectRoot: "github.com/sdboyer/deptest", - wantSourceRepo: "https://github.com/sdboyer/deptest.git", - wantConstraint: "^1.0.0", - wantLockCount: 1, - wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), - wantVersion: "v1.0.0", + convertTestCase{ + wantSourceRepo: importerTestProjectSrc, + wantConstraint: importerTestV2Branch, + wantRevision: importerTestV2PatchRev, + wantVersion: importerTestV2PatchTag, }, }, "test project": { - yaml: glideYaml{ + glideYaml{ Imports: []glidePackage{ { - Name: "github.com/sdboyer/deptest", - Reference: "v1.0.0", + Name: importerTestProject, + Repository: importerTestProjectSrc, + Reference: importerTestV2Branch, }, }, }, - lock: &glideLock{ + glideLock{ Imports: []glideLockedPackage{ { - Name: "github.com/sdboyer/deptest", - Reference: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + Name: importerTestProject, + Repository: importerTestProjectSrc, + Revision: importerTestV2PatchRev, }, }, }, - convertTestCase: &convertTestCase{ - projectRoot: "github.com/sdboyer/deptest", - wantLockCount: 1, - wantConstraint: "^1.0.0", - wantVersion: "v1.0.0", - wantRevision: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + convertTestCase{ + wantSourceRepo: importerTestProjectSrc, + wantConstraint: importerTestV2Branch, + wantRevision: importerTestV2PatchRev, + wantVersion: importerTestV2PatchTag, }, }, - "revision only": { - yaml: glideYaml{ + "yaml only": { + glideYaml{ Imports: []glidePackage{ { - Name: "github.com/sdboyer/deptest", + Name: importerTestProject, + Repository: importerTestProjectSrc, + Reference: importerTestV2Branch, }, }, }, - lock: &glideLock{ - Imports: []glideLockedPackage{ - { - Name: "github.com/sdboyer/deptest", - Reference: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", - }, - }, - }, - convertTestCase: &convertTestCase{ - projectRoot: "github.com/sdboyer/deptest", - wantLockCount: 1, - wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), - wantVersion: "v1.0.0", + glideLock{}, + convertTestCase{ + wantSourceRepo: importerTestProjectSrc, + wantConstraint: importerTestV2Branch, }, }, - "with ignored package": { - yaml: glideYaml{ - Ignores: []string{"github.com/sdboyer/deptest"}, + "ignored package": { + glideYaml{ + Ignores: []string{importerTestProject}, }, - convertTestCase: &convertTestCase{ - projectRoot: "github.com/sdboyer/deptest", - wantIgnoreCount: 1, - wantIgnoredPackages: []string{"github.com/sdboyer/deptest"}, + glideLock{}, + convertTestCase{ + wantIgnored: []string{importerTestProject}, }, }, - "with exclude dir": { - yaml: glideYaml{ + "exclude dir": { + glideYaml{ ExcludeDirs: []string{"samples"}, }, - convertTestCase: &convertTestCase{ - projectRoot: testProjectRoot, - wantIgnoreCount: 1, - wantIgnoredPackages: []string{"github.com/golang/notexist/samples"}, + glideLock{}, + convertTestCase{ + wantIgnored: []string{testProjectRoot + "/samples"}, }, }, "exclude dir ignores mismatched package name": { - yaml: glideYaml{ + glideYaml{ Name: "github.com/golang/mismatched-package-name", ExcludeDirs: []string{"samples"}, }, - convertTestCase: &convertTestCase{ - projectRoot: testProjectRoot, - wantIgnoreCount: 1, - wantIgnoredPackages: []string{"github.com/golang/notexist/samples"}, + glideLock{}, + convertTestCase{ + wantIgnored: []string{testProjectRoot + "/samples"}, }, }, - "bad input, empty package name": { - yaml: glideYaml{ + "missing package name": { + glideYaml{ Imports: []glidePackage{{Name: ""}}, }, - convertTestCase: &convertTestCase{ - projectRoot: testProjectRoot, + glideLock{}, + convertTestCase{ wantConvertErr: true, }, }, + "warn unused os field": { + glideYaml{ + Imports: []glidePackage{ + { + Name: importerTestProject, + OS: "windows", + }, + }}, + glideLock{}, + convertTestCase{ + wantConstraint: "*", + wantWarning: "specified an os", + }, + }, + "warn unused arch field": { + glideYaml{ + Imports: []glidePackage{ + { + Name: importerTestProject, + Arch: "i686", + }, + }}, + glideLock{}, + convertTestCase{ + wantConstraint: "*", + wantWarning: "specified an arch", + }, + }, } - h := test.NewHelper(t) - defer h.Cleanup() - - ctx := newTestContext(h) - sm, err := ctx.SourceManager() - h.Must(err) - defer sm.Release() - for name, testCase := range testCases { + name := name + testCase := testCase t.Run(name, func(t *testing.T) { - g := newGlideImporter(discardLogger, true, sm) - g.yaml = testCase.yaml - - if testCase.lock != nil { - g.lock = testCase.lock - } - - manifest, lock, convertErr := g.convert(testCase.projectRoot) - err := validateConvertTestCase(testCase.convertTestCase, manifest, lock, convertErr) + t.Parallel() + + err := testCase.Exec(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock, error) { + g := newGlideImporter(logger, true, sm) + g.glideConfig = testCase.yaml + g.glideLock = testCase.lock + return g.convert(testProjectRoot) + }) if err != nil { t.Fatalf("%#v", err) } @@ -229,144 +238,3 @@ func TestGlideConfig_Import(t *testing.T) { } } } - -func TestGlideConfig_Import_MissingLockFile(t *testing.T) { - h := test.NewHelper(t) - defer h.Cleanup() - - ctx := newTestContext(h) - sm, err := ctx.SourceManager() - h.Must(err) - defer sm.Release() - - h.TempDir(filepath.Join("src", testProjectRoot)) - h.TempCopy(filepath.Join(testProjectRoot, glideYamlName), "init/glide/glide.yaml") - projectRoot := h.Path(testProjectRoot) - - g := newGlideImporter(ctx.Err, true, sm) - if !g.HasDepMetadata(projectRoot) { - t.Fatal("The glide importer should gracefully handle when only glide.yaml is present") - } - - m, l, err := g.Import(projectRoot, testProjectRoot) - h.Must(err) - - if m == nil { - t.Fatal("Expected the manifest to be generated") - } - - if l != nil { - t.Fatal("Expected the lock to not be generated") - } -} - -func TestGlideConfig_Convert_WarnsForUnusedFields(t *testing.T) { - testCases := map[string]glidePackage{ - "specified an os": {OS: "windows"}, - "specified an arch": {Arch: "i686"}, - } - - for wantWarning, pkg := range testCases { - t.Run(wantWarning, func(t *testing.T) { - h := test.NewHelper(t) - defer h.Cleanup() - - pkg.Name = "github.com/sdboyer/deptest" - pkg.Reference = "v1.0.0" - - ctx := newTestContext(h) - sm, err := ctx.SourceManager() - h.Must(err) - defer sm.Release() - - // Capture stderr so we can verify warnings - verboseOutput := &bytes.Buffer{} - ctx.Err = log.New(verboseOutput, "", 0) - - g := newGlideImporter(ctx.Err, true, sm) - g.yaml = glideYaml{ - Imports: []glidePackage{pkg}, - } - - _, _, err = g.convert(testProjectRoot) - if err != nil { - t.Fatal(err) - } - - warnings := verboseOutput.String() - if !strings.Contains(warnings, wantWarning) { - t.Errorf("Expected the output to include the warning '%s'", wantWarning) - } - }) - } -} - -// equalSlice is comparing two slices for equality. -func equalSlice(a, b []string) bool { - if a == nil && b == nil { - return true - } - - if a == nil || b == nil { - return false - } - - if len(a) != len(b) { - return false - } - - for i := range a { - if a[i] != b[i] { - return false - } - } - - return true -} - -func TestGlideConfig_Convert_ConsolidateRootPackages(t *testing.T) { - h := test.NewHelper(t) - defer h.Cleanup() - - ctx := newTestContext(h) - sm, err := ctx.SourceManager() - h.Must(err) - defer sm.Release() - - g := newGlideImporter(ctx.Err, true, sm) - g.yaml = glideYaml{ - Imports: []glidePackage{ - {Name: "github.com/carolynvs/deptest-subpkg/subby"}, - {Name: "github.com/carolynvs/deptest-subpkg"}, - }, - } - g.lock = &glideLock{ - Imports: []glideLockedPackage{ - {Name: "github.com/carolynvs/deptest-subpkg/subby"}, - {Name: "github.com/carolynvs/deptest-subpkg"}, - }, - } - - manifest, lock, err := g.convert(testProjectRoot) - h.Must(err) - - gotMLen := len(manifest.Constraints) - if gotMLen != 1 { - t.Fatalf("Expected manifest to contain 1 constraint, got %d", gotMLen) - } - - wantRoot := gps.ProjectRoot("github.com/carolynvs/deptest-subpkg") - if _, has := manifest.Constraints[wantRoot]; !has { - t.Fatalf("Expected manifest to contain a constraint for %s, got %v", wantRoot, manifest.Constraints) - } - - gotLLen := len(lock.P) - if gotLLen != 1 { - t.Fatalf("Expected lock to contain 1 project, got %d", gotLLen) - } - - gotRoot := lock.P[0].Ident().ProjectRoot - if gotRoot != wantRoot { - t.Fatalf("Expected lock to contain a project for %s, got %v", wantRoot, gotRoot) - } -} diff --git a/cmd/dep/godep_importer.go b/cmd/dep/godep_importer.go index 1baa8aef04..9d84e3e56d 100644 --- a/cmd/dep/godep_importer.go +++ b/cmd/dep/godep_importer.go @@ -12,7 +12,6 @@ import ( "path/filepath" "github.com/golang/dep" - fb "github.com/golang/dep/internal/feedback" "github.com/golang/dep/internal/gps" "github.com/pkg/errors" ) @@ -20,19 +19,12 @@ import ( const godepPath = "Godeps" + string(os.PathSeparator) + "Godeps.json" type godepImporter struct { + *baseImporter json godepJSON - - logger *log.Logger - verbose bool - sm gps.SourceManager } func newGodepImporter(logger *log.Logger, verbose bool, sm gps.SourceManager) *godepImporter { - return &godepImporter{ - logger: logger, - verbose: verbose, - sm: sm, - } + return &godepImporter{baseImporter: newBaseImporter(logger, verbose, sm)} } type godepJSON struct { @@ -75,11 +67,11 @@ func (g *godepImporter) load(projectDir string) error { } jb, err := ioutil.ReadFile(j) if err != nil { - return errors.Wrapf(err, "Unable to read %s", j) + return errors.Wrapf(err, "unable to read %s", j) } err = json.Unmarshal(jb, &g.json) if err != nil { - return errors.Wrapf(err, "Unable to parse %s", j) + return errors.Wrapf(err, "unable to parse %s", j) } return nil @@ -88,100 +80,31 @@ func (g *godepImporter) load(projectDir string) error { func (g *godepImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { g.logger.Println("Converting from Godeps.json ...") - manifest := dep.NewManifest() - lock := &dep.Lock{} - + packages := make([]importedPackage, 0, len(g.json.Imports)) for _, pkg := range g.json.Imports { - // ImportPath must not be empty + // Validate if pkg.ImportPath == "" { - err := errors.New("Invalid godep configuration, ImportPath is required") - return nil, nil, err - } - - // Obtain ProjectRoot. Required for avoiding sub-package imports. - ip, err := g.sm.DeduceProjectRoot(pkg.ImportPath) - if err != nil { + err := errors.New("invalid godep configuration, ImportPath is required") return nil, nil, err } - pkg.ImportPath = string(ip) - // Check if it already existing in locked projects - if projectExistsInLock(lock, ip) { - continue - } - - // Rev must not be empty if pkg.Rev == "" { - err := errors.New("Invalid godep configuration, Rev is required") + err := errors.New("invalid godep configuration, Rev is required") return nil, nil, err } - if pkg.Comment == "" { - // When there's no comment, try to get corresponding version for the Rev - // and fill Comment. - pi := gps.ProjectIdentifier{ - ProjectRoot: gps.ProjectRoot(pkg.ImportPath), - } - revision := gps.Revision(pkg.Rev) - - version, err := lookupVersionForLockedProject(pi, nil, revision, g.sm) - if err != nil { - // Only warn about the problem, it is not enough to warrant failing - g.logger.Println(err.Error()) - } else { - pp := getProjectPropertiesFromVersion(version) - if pp.Constraint != nil { - pkg.Comment = pp.Constraint.String() - } - } + ip := importedPackage{ + Name: pkg.ImportPath, + LockHint: pkg.Rev, + ConstraintHint: pkg.Comment, } - - if pkg.Comment != "" { - // If there's a comment, use it to create project constraint - pc, err := g.buildProjectConstraint(pkg) - if err != nil { - return nil, nil, err - } - manifest.Constraints[pc.Ident.ProjectRoot] = gps.ProjectProperties{Constraint: pc.Constraint} - } - - lp := g.buildLockedProject(pkg, manifest) - lock.P = append(lock.P, lp) + packages = append(packages, ip) } - return manifest, lock, nil -} - -// buildProjectConstraint uses the provided package ImportPath and Comment to -// create a project constraint -func (g *godepImporter) buildProjectConstraint(pkg godepPackage) (pc gps.ProjectConstraint, err error) { - pc.Ident = gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.ImportPath)} - pc.Constraint, err = g.sm.InferConstraint(pkg.Comment, pc.Ident) + err := g.importPackages(packages, true) if err != nil { - return - } - - f := fb.NewConstraintFeedback(pc, fb.DepTypeImported) - f.LogFeedback(g.logger) - - return -} - -// buildLockedProject uses the package Rev and Comment to create lock project -func (g *godepImporter) buildLockedProject(pkg godepPackage, manifest *dep.Manifest) gps.LockedProject { - pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.ImportPath)} - revision := gps.Revision(pkg.Rev) - pp := manifest.Constraints[pi.ProjectRoot] - - version, err := lookupVersionForLockedProject(pi, pp.Constraint, revision, g.sm) - if err != nil { - // Only warn about the problem, it is not enough to warrant failing - g.logger.Println(err.Error()) + return nil, nil, err } - lp := gps.NewLockedProject(pi, version, nil) - f := fb.NewLockedProjectFeedback(lp, fb.DepTypeImported) - f.LogFeedback(g.logger) - - return lp + return g.manifest, g.lock, nil } diff --git a/cmd/dep/godep_importer_test.go b/cmd/dep/godep_importer_test.go index db46f5798d..bb231d44ac 100644 --- a/cmd/dep/godep_importer_test.go +++ b/cmd/dep/godep_importer_test.go @@ -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" @@ -19,124 +20,73 @@ const testProjectRoot = "github.com/golang/notexist" func TestGodepConfig_Convert(t *testing.T) { testCases := map[string]struct { - *convertTestCase + convertTestCase json godepJSON }{ - "convert project": { - json: godepJSON{ - Imports: []godepPackage{ - { - ImportPath: "github.com/sdboyer/deptest", - // This revision has 2 versions attached to it, v1.0.0 & v0.8.0. - Rev: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", - Comment: "v0.8.0", - }, - }, + "package without comment": { + convertTestCase{ + wantConstraint: importerTestV1Constraint, + wantRevision: importerTestV1Rev, + wantVersion: importerTestV1Tag, }, - convertTestCase: &convertTestCase{ - projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), - wantConstraint: "^0.8.0", - wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), - wantVersion: "v0.8.0", - wantLockCount: 1, - }, - }, - "with semver suffix": { - json: godepJSON{ + godepJSON{ Imports: []godepPackage{ { - ImportPath: "github.com/sdboyer/deptest", - Rev: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", - Comment: "v1.12.0-12-g2fd980e", + ImportPath: importerTestProject, + Rev: importerTestV1Rev, }, }, }, - convertTestCase: &convertTestCase{ - projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), - wantConstraint: "^1.12.0-12-g2fd980e", - wantLockCount: 1, - wantVersion: "v1.0.0", - wantRevision: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", - }, }, - "empty comment": { - json: godepJSON{ + "package with comment": { + convertTestCase{ + wantConstraint: importerTestV2Branch, + wantRevision: importerTestV2PatchRev, + wantVersion: importerTestV2PatchTag, + }, + godepJSON{ Imports: []godepPackage{ { - ImportPath: "github.com/sdboyer/deptest", - // This revision has 2 versions attached to it, v1.0.0 & v0.8.0. - Rev: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + ImportPath: importerTestProject, + Rev: importerTestV2PatchRev, + Comment: importerTestV2Branch, }, }, }, - convertTestCase: &convertTestCase{ - projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), - wantConstraint: "^1.0.0", - wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), - wantVersion: "v1.0.0", - wantLockCount: 1, - }, }, - "bad input - empty package name": { - json: godepJSON{ - Imports: []godepPackage{{ImportPath: ""}}, - }, - convertTestCase: &convertTestCase{ + "missing package name": { + convertTestCase{ wantConvertErr: true, }, - }, - "bad input - empty revision": { - json: godepJSON{ - Imports: []godepPackage{ - { - ImportPath: "github.com/sdboyer/deptest", - }, - }, + godepJSON{ + Imports: []godepPackage{{ImportPath: ""}}, }, - convertTestCase: &convertTestCase{ + }, + "missing revision": { + convertTestCase{ wantConvertErr: true, }, - }, - "sub-packages": { - json: godepJSON{ + godepJSON{ Imports: []godepPackage{ { - ImportPath: "github.com/sdboyer/deptest", - // This revision has 2 versions attached to it, v1.0.0 & v0.8.0. - Rev: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", - }, - { - ImportPath: "github.com/sdboyer/deptest/foo", - // This revision has 2 versions attached to it, v1.0.0 & v0.8.0. - Rev: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + ImportPath: importerTestProject, }, }, }, - convertTestCase: &convertTestCase{ - projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), - wantLockCount: 1, - wantConstraint: "^1.0.0", - wantVersion: "v1.0.0", - wantRevision: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", - }, }, } - h := test.NewHelper(t) - defer h.Cleanup() - - ctx := newTestContext(h) - sm, err := ctx.SourceManager() - h.Must(err) - defer sm.Release() - for name, testCase := range testCases { + name := name + testCase := testCase t.Run(name, func(t *testing.T) { - g := newGodepImporter(discardLogger, true, sm) - g.json = testCase.json + t.Parallel() - manifest, lock, convertErr := g.convert(testCase.projectRoot) - err := validateConvertTestCase(testCase.convertTestCase, manifest, lock, convertErr) + err := testCase.Exec(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock, error) { + g := newGodepImporter(logger, true, sm) + g.json = testCase.json + return g.convert(testProjectRoot) + }) if err != nil { t.Fatalf("%#v", err) } diff --git a/cmd/dep/govend_importer.go b/cmd/dep/govend_importer.go index 0b1d7531ed..3dc0799c65 100644 --- a/cmd/dep/govend_importer.go +++ b/cmd/dep/govend_importer.go @@ -12,7 +12,6 @@ import ( "github.com/go-yaml/yaml" "github.com/golang/dep" - fb "github.com/golang/dep/internal/feedback" "github.com/golang/dep/internal/gps" "github.com/pkg/errors" ) @@ -23,19 +22,12 @@ const govendYAMLName = "vendor.yml" // govendImporter imports govend configuration in to the dep configuration format. type govendImporter struct { + *baseImporter yaml govendYAML - - logger *log.Logger - verbose bool - sm gps.SourceManager } func newGovendImporter(logger *log.Logger, verbose bool, sm gps.SourceManager) *govendImporter { - return &govendImporter{ - logger: logger, - verbose: verbose, - sm: sm, - } + return &govendImporter{baseImporter: newBaseImporter(logger, verbose, sm)} } type govendYAML struct { @@ -78,11 +70,11 @@ func (g *govendImporter) load(projectDir string) error { } yb, err := ioutil.ReadFile(y) if err != nil { - return errors.Wrapf(err, "Unable to read %s", y) + return errors.Wrapf(err, "unable to read %s", y) } err = yaml.Unmarshal(yb, &g.yaml) if err != nil { - return errors.Wrapf(err, "Unable to parse %s", y) + return errors.Wrapf(err, "unable to parse %s", y) } return nil } @@ -91,80 +83,24 @@ func (g *govendImporter) load(projectDir string) error { func (g *govendImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { g.logger.Println("Converting from vendor.yaml...") - manifest := dep.NewManifest() - lock := &dep.Lock{} - + packages := make([]importedPackage, 0, len(g.yaml.Imports)) for _, pkg := range g.yaml.Imports { // Path must not be empty if pkg.Path == "" || pkg.Revision == "" { - return nil, nil, errors.New("Invalid govend configuration, Path and Rev are required") + return nil, nil, errors.New("invalid govend configuration, path and rev are required") } - p, err := g.sm.DeduceProjectRoot(pkg.Path) - if err != nil { - return nil, nil, err + ip := importedPackage{ + Name: pkg.Path, + LockHint: pkg.Revision, } - pkg.Path = string(p) - - // Check if the current project is already existing in locked projects. - if projectExistsInLock(lock, p) { - continue - } - - pi := gps.ProjectIdentifier{ - ProjectRoot: gps.ProjectRoot(pkg.Path), - } - revision := gps.Revision(pkg.Revision) - - version, err := lookupVersionForLockedProject(pi, nil, revision, g.sm) - if err != nil { - g.logger.Println(err.Error()) - } else { - pp := getProjectPropertiesFromVersion(version) - if pp.Constraint != nil { - pc, err := g.buildProjectConstraint(pkg, pp.Constraint.String()) - if err != nil { - g.logger.Printf("Unable to infer a constraint for revision %s for package %s: %s", pkg.Revision, pkg.Path, err.Error()) - continue - } - manifest.Constraints[pc.Ident.ProjectRoot] = gps.ProjectProperties{Constraint: pc.Constraint} - } - } - - lp := g.buildLockedProject(pkg, manifest) - lock.P = append(lock.P, lp) + packages = append(packages, ip) } - return manifest, lock, nil -} - -func (g *govendImporter) buildProjectConstraint(pkg govendPackage, constraint string) (pc gps.ProjectConstraint, err error) { - pc.Ident = gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.Path)} - pc.Constraint, err = g.sm.InferConstraint(constraint, pc.Ident) + err := g.importPackages(packages, true) if err != nil { - return - } - - f := fb.NewConstraintFeedback(pc, fb.DepTypeImported) - f.LogFeedback(g.logger) - - return - -} - -func (g *govendImporter) buildLockedProject(pkg govendPackage, manifest *dep.Manifest) gps.LockedProject { - p := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.Path)} - revision := gps.Revision(pkg.Revision) - pp := manifest.Constraints[p.ProjectRoot] - - version, err := lookupVersionForLockedProject(p, pp.Constraint, revision, g.sm) - if err != nil { - g.logger.Println(err.Error()) + return nil, nil, err } - lp := gps.NewLockedProject(p, version, nil) - f := fb.NewLockedProjectFeedback(lp, fb.DepTypeImported) - f.LogFeedback(g.logger) - - return lp + return g.manifest, g.lock, nil } diff --git a/cmd/dep/govend_importer_test.go b/cmd/dep/govend_importer_test.go index abf4f519fc..4cf9236107 100644 --- a/cmd/dep/govend_importer_test.go +++ b/cmd/dep/govend_importer_test.go @@ -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" @@ -17,68 +18,62 @@ import ( func TestGovendConfig_Convert(t *testing.T) { testCases := map[string]struct { - *convertTestCase yaml govendYAML + convertTestCase }{ - "project": { - yaml: govendYAML{ + "package": { + govendYAML{ Imports: []govendPackage{ { - Path: "github.com/sdboyer/deptest", - Revision: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", + Path: importerTestProject, + Revision: importerTestV1Rev, }, }, }, - convertTestCase: &convertTestCase{ - projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), - wantConstraint: "^1.0.0", - wantLockCount: 1, - wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), - wantVersion: "v1.0.0", + convertTestCase{ + wantConstraint: importerTestV1Constraint, + wantRevision: importerTestV1Rev, + wantVersion: importerTestV1Tag, }, }, - "bad input - empty package name": { - yaml: govendYAML{ + "missing package name": { + govendYAML{ Imports: []govendPackage{ { Path: "", }, }, }, - convertTestCase: &convertTestCase{ + convertTestCase{ wantConvertErr: true, }, }, - "bad input - empty revision": { - yaml: govendYAML{ + "missing revision": { + govendYAML{ Imports: []govendPackage{ { - Path: "github.com/sdboyer/deptest", + Path: importerTestProject, }, }, }, - convertTestCase: &convertTestCase{ + convertTestCase{ wantConvertErr: true, }, }, } - h := test.NewHelper(t) - defer h.Cleanup() - - ctx := newTestContext(h) - sm, err := ctx.SourceManager() - h.Must(err) - defer sm.Release() - for name, testCase := range testCases { + name := name + testCase := testCase t.Run(name, func(t *testing.T) { - g := newGovendImporter(discardLogger, true, sm) - g.yaml = testCase.yaml + t.Parallel() - manifest, lock, convertErr := g.convert(testCase.projectRoot) - err = validateConvertTestCase(testCase.convertTestCase, manifest, lock, convertErr) + err := testCase.Exec(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock, error) { + g := newGovendImporter(logger, true, sm) + g.yaml = testCase.yaml + return g.convert(testProjectRoot) + }) if err != nil { t.Fatalf("%#v", err) } diff --git a/cmd/dep/root_analyzer.go b/cmd/dep/root_analyzer.go index 556ed6ddad..471b673b33 100644 --- a/cmd/dep/root_analyzer.go +++ b/cmd/dep/root_analyzer.go @@ -11,7 +11,6 @@ import ( "github.com/golang/dep" fb "github.com/golang/dep/internal/feedback" "github.com/golang/dep/internal/gps" - "github.com/pkg/errors" ) // importer handles importing configuration from other dependency managers into @@ -168,74 +167,3 @@ func (a *rootAnalyzer) Info() gps.ProjectAnalyzerInfo { Version: 1, } } - -// isVersion determines if the specified value is a version/tag in the project. -func isVersion(pi gps.ProjectIdentifier, value string, sm gps.SourceManager) (bool, gps.Version, error) { - versions, err := sm.ListVersions(pi) - if err != nil { - return false, nil, errors.Wrapf(err, "list versions for %s(%s)", pi.ProjectRoot, pi.Source) // means repo does not exist - } - - for _, version := range versions { - if version.Type() != gps.IsVersion && version.Type() != gps.IsSemver { - continue - } - - if value == version.String() { - return true, version, nil - } - } - - return false, nil, nil -} - -// lookupVersionForLockedProject figures out the appropriate version for a locked -// project based on the locked revision and the constraint from the manifest. -// First try matching the revision to a version, then try the constraint from the -// manifest, then finally the revision. -func lookupVersionForLockedProject(pi gps.ProjectIdentifier, c gps.Constraint, rev gps.Revision, sm gps.SourceManager) (gps.Version, error) { - // Find the version that goes with this revision, if any - versions, err := sm.ListVersions(pi) - if err != nil { - return rev, errors.Wrapf(err, "Unable to lookup the version represented by %s in %s(%s). Falling back to locking the revision only.", rev, pi.ProjectRoot, pi.Source) - } - - gps.SortPairedForUpgrade(versions) // Sort versions in asc order - for _, v := range versions { - if v.Revision() == rev { - // If the constraint is semver, make sure the version is acceptable. - // This prevents us from suggesting an incompatible version, which - // helps narrow the field when there are multiple matching versions. - if c != nil { - _, err := gps.NewSemverConstraint(c.String()) - if err == nil && !c.Matches(v) { - continue - } - } - return v, nil - } - } - - // Use the version from the manifest as long as it wasn't a range - switch tv := c.(type) { - case gps.PairedVersion: - return tv.Unpair().Pair(rev), nil - case gps.UnpairedVersion: - return tv.Pair(rev), nil - } - - // Give up and lock only to a revision - return rev, nil -} - -// projectExistsInLock checks if the given project already exists in -// a lockfile. -func projectExistsInLock(l *dep.Lock, pr gps.ProjectRoot) bool { - for _, lp := range l.P { - if pr == lp.Ident().ProjectRoot { - return true - } - } - - return false -} diff --git a/cmd/dep/root_analyzer_test.go b/cmd/dep/root_analyzer_test.go deleted file mode 100644 index 4bbb26d8d3..0000000000 --- a/cmd/dep/root_analyzer_test.go +++ /dev/null @@ -1,277 +0,0 @@ -// Copyright 2016 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package main - -import ( - "strings" - "testing" - - "github.com/golang/dep" - "github.com/golang/dep/internal/gps" - "github.com/golang/dep/internal/test" - "github.com/pkg/errors" -) - -func TestLookupVersionForLockedProject_MatchRevisionToTag(t *testing.T) { - h := test.NewHelper(t) - defer h.Cleanup() - - ctx := newTestContext(h) - sm, err := ctx.SourceManager() - h.Must(err) - defer sm.Release() - - pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")} - rev := gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf") - v, err := lookupVersionForLockedProject(pi, nil, rev, sm) - h.Must(err) - - wantV := "v1.0.0" - gotV := v.String() - if gotV != wantV { - t.Fatalf("Expected the locked version to be the tag paired with the manifest's pinned revision: wanted '%s', got '%s'", wantV, gotV) - } -} - -func TestLookupVersionForLockedProject_MatchRevisionToMultipleTags(t *testing.T) { - h := test.NewHelper(t) - defer h.Cleanup() - - ctx := newTestContext(h) - sm, err := ctx.SourceManager() - h.Must(err) - defer sm.Release() - - pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")} - // Both 0.8.0 and 1.0.0 use the same rev, force dep to pick the lower version - c, _ := gps.NewSemverConstraint("<1.0.0") - rev := gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf") - v, err := lookupVersionForLockedProject(pi, c, rev, sm) - h.Must(err) - - wantV := "v0.8.0" - gotV := v.String() - if gotV != wantV { - t.Fatalf("Expected the locked version to satisfy the manifest's semver constraint: wanted '%s', got '%s'", wantV, gotV) - } -} - -func TestLookupVersionForLockedProject_FallbackToConstraint(t *testing.T) { - h := test.NewHelper(t) - defer h.Cleanup() - - ctx := newTestContext(h) - sm, err := ctx.SourceManager() - h.Must(err) - defer sm.Release() - - pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")} - c := gps.NewBranch("master") - rev := gps.Revision("c575196502940c07bf89fd6d95e83b999162e051") - v, err := lookupVersionForLockedProject(pi, c, rev, sm) - h.Must(err) - - wantV := c.String() - gotV := v.String() - if gotV != wantV { - t.Fatalf("Expected the locked version to be defaulted from the manifest's branch constraint: wanted '%s', got '%s'", wantV, gotV) - } -} - -func TestLookupVersionForLockedProject_FallbackToRevision(t *testing.T) { - h := test.NewHelper(t) - defer h.Cleanup() - - ctx := newTestContext(h) - sm, err := ctx.SourceManager() - h.Must(err) - defer sm.Release() - - pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")} - rev := gps.Revision("c575196502940c07bf89fd6d95e83b999162e051") - v, err := lookupVersionForLockedProject(pi, nil, rev, sm) - h.Must(err) - - wantV := rev.String() - gotV := v.String() - if gotV != wantV { - t.Fatalf("Expected the locked version to be the manifest's pinned revision: wanted '%s', got '%s'", wantV, gotV) - } -} - -func TestProjectExistsInLock(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 { - name string - importPath string - want bool - }{ - { - name: "root project", - importPath: "github.com/sdboyer/deptest", - want: true, - }, - { - name: "sub package", - importPath: "github.com/sdboyer/deptest/foo", - want: false, - }, - { - name: "nonexisting project", - importPath: "github.com/golang/notexist", - want: false, - }, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - result := projectExistsInLock(lock, gps.ProjectRoot(c.importPath)) - - if result != c.want { - t.Fatalf("projectExistsInLock result is not as want: \n\t(GOT) %v \n\t(WNT) %v", result, c.want) - } - }) - } -} - -func TestIsVersion(t *testing.T) { - testcases := map[string]struct { - wantIsVersion bool - wantVersion gps.Version - }{ - "v1.0.0": {wantIsVersion: true, wantVersion: gps.NewVersion("v1.0.0").Pair("ff2948a2ac8f538c4ecd55962e919d1e13e74baf")}, - "3f4c3bea144e112a69bbe5d8d01c1b09a544253f": {wantIsVersion: false}, - "master": {wantIsVersion: false}, - } - - pi := gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot("github.com/sdboyer/deptest")} - h := test.NewHelper(t) - defer h.Cleanup() - - ctx := newTestContext(h) - sm, err := ctx.SourceManager() - h.Must(err) - defer sm.Release() - - for value, testcase := range testcases { - t.Run(value, func(t *testing.T) { - gotIsVersion, gotVersion, err := isVersion(pi, value, sm) - h.Must(err) - - if testcase.wantIsVersion != gotIsVersion { - t.Fatalf("unexpected isVersion result for %s: \n\t(GOT) %v \n\t(WNT) %v", value, gotIsVersion, testcase.wantIsVersion) - } - - if testcase.wantVersion != gotVersion { - t.Fatalf("unexpected version for %s: \n\t(GOT) %v \n\t(WNT) %v", value, gotVersion, testcase.wantVersion) - } - }) - } -} - -// convertTestCase is a common set of validations applied to the result -// of an importer converting from an external config format to dep's. -type convertTestCase struct { - wantConvertErr bool - projectRoot gps.ProjectRoot - wantSourceRepo string - wantConstraint string - wantRevision gps.Revision - wantVersion string - wantLockCount int - wantIgnoreCount int - wantIgnoredPackages []string -} - -// validateConvertTestCase returns an error if any of the importer's -// conversion validations failed. -func validateConvertTestCase(testCase *convertTestCase, manifest *dep.Manifest, lock *dep.Lock, convertErr error) error { - if testCase.wantConvertErr { - if convertErr == nil { - return errors.New("Expected the conversion to fail, but it did not return an error") - } - return nil - } - - if convertErr != nil { - return errors.Wrap(convertErr, "Expected the conversion to pass, but it returned an error") - } - - // Ignored projects checks. - if len(manifest.Ignored) != testCase.wantIgnoreCount { - return errors.Errorf("Expected manifest to have %d ignored project(s), got %d", - testCase.wantIgnoreCount, - len(manifest.Ignored)) - } - - if !equalSlice(manifest.Ignored, testCase.wantIgnoredPackages) { - return errors.Errorf("Expected manifest to have ignore %s, got %s", - strings.Join(testCase.wantIgnoredPackages, ", "), - strings.Join(manifest.Ignored, ", ")) - } - - // Constraints checks below. - if testCase.wantConstraint != "" { - d, ok := manifest.Constraints[testCase.projectRoot] - if !ok { - return errors.Errorf("Expected the manifest to have a dependency for '%s' but got none", - testCase.projectRoot) - } - - v := d.Constraint.String() - if v != testCase.wantConstraint { - return errors.Errorf("Expected manifest constraint to be %s, got %s", testCase.wantConstraint, v) - } - } - - // Lock checks. - if lock != nil { - if len(lock.P) != testCase.wantLockCount { - return errors.Errorf("Expected lock to have %d project(s), got %d", - testCase.wantLockCount, - len(lock.P)) - } - - p := lock.P[0] - - if p.Ident().ProjectRoot != testCase.projectRoot { - return errors.Errorf("Expected the lock to have a project for '%s' but got '%s'", - testCase.projectRoot, - p.Ident().ProjectRoot) - } - - if p.Ident().Source != testCase.wantSourceRepo { - return errors.Errorf("Expected locked source to be %s, got '%s'", testCase.wantSourceRepo, p.Ident().Source) - } - - // Break down the locked "version" into a version (optional) and revision - var gotVersion string - var gotRevision gps.Revision - if lpv, ok := p.Version().(gps.PairedVersion); ok { - gotVersion = lpv.String() - gotRevision = lpv.Revision() - } else if lr, ok := p.Version().(gps.Revision); ok { - gotRevision = lr - } else { - return errors.New("could not determine the type of the locked version") - } - - if gotRevision != testCase.wantRevision { - return errors.Errorf("unexpected locked revision : \n\t(GOT) %v \n\t(WNT) %v", - gotRevision, - testCase.wantRevision) - } - if gotVersion != testCase.wantVersion { - return errors.Errorf("unexpected locked version: \n\t(GOT) %v \n\t(WNT) %v", - gotVersion, - testCase.wantVersion) - } - } - return nil -} diff --git a/cmd/dep/testdata/init/glide/golden.txt b/cmd/dep/testdata/init/glide/golden.txt index 4155025ef8..30dfd7d429 100644 --- a/cmd/dep/testdata/init/glide/golden.txt +++ b/cmd/dep/testdata/init/glide/golden.txt @@ -1,8 +1,8 @@ Detected glide configuration files... Converting from glide.yaml and glide.lock... Using master as initial constraint for imported dep github.com/sdboyer/deptest - Using ^2.0.0 as initial constraint for imported dep github.com/sdboyer/deptestdos - Using * as initial constraint for imported dep github.com/golang/lint Trying v0.8.1 (3f4c3be) as initial lock for imported dep github.com/sdboyer/deptest + Using ^2.0.0 as initial constraint for imported dep github.com/sdboyer/deptestdos Trying v2.0.0 (5c60720) as initial lock for imported dep github.com/sdboyer/deptestdos + Using * as initial constraint for imported dep github.com/golang/lint Trying * (cb00e56) as initial lock for imported dep github.com/golang/lint diff --git a/cmd/dep/vndr_importer.go b/cmd/dep/vndr_importer.go index 01463ca536..911700a260 100644 --- a/cmd/dep/vndr_importer.go +++ b/cmd/dep/vndr_importer.go @@ -12,7 +12,6 @@ import ( "strings" "github.com/golang/dep" - fb "github.com/golang/dep/internal/feedback" "github.com/golang/dep/internal/gps" "github.com/pkg/errors" ) @@ -22,19 +21,12 @@ func vndrFile(dir string) string { } type vndrImporter struct { + *baseImporter packages []vndrPackage - - logger *log.Logger - verbose bool - sm gps.SourceManager } func newVndrImporter(log *log.Logger, verbose bool, sm gps.SourceManager) *vndrImporter { - return &vndrImporter{ - logger: log, - verbose: verbose, - sm: sm, - } + return &vndrImporter{baseImporter: newBaseImporter(log, verbose, sm)} } func (v *vndrImporter) Name() string { return "vndr" } @@ -60,7 +52,7 @@ func (v *vndrImporter) loadVndrFile(dir string) error { f, err := os.Open(vndrFile(dir)) if err != nil { - return errors.Wrapf(err, "Unable to open %s", vndrFile(dir)) + return errors.Wrapf(err, "unable to open %s", vndrFile(dir)) } defer f.Close() @@ -85,75 +77,32 @@ func (v *vndrImporter) loadVndrFile(dir string) error { } func (v *vndrImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) { - var ( - manifest = dep.NewManifest() - lock = &dep.Lock{} - ) - + packages := make([]importedPackage, 0, len(v.packages)) for _, pkg := range v.packages { + // Validate if pkg.importPath == "" { - err := errors.New("Invalid vndr configuration, import path is required") - return nil, nil, err - } - - // Obtain ProjectRoot. Required for avoiding sub-package imports. - ip, err := v.sm.DeduceProjectRoot(pkg.importPath) - if err != nil { + err := errors.New("invalid vndr configuration: import path is required") return nil, nil, err } - pkg.importPath = string(ip) - - // Check if it already existing in locked projects - if projectExistsInLock(lock, ip) { - continue - } if pkg.reference == "" { - err := errors.New("Invalid vndr configuration, revision is required") + err := errors.New("invalid vndr configuration: revision is required") return nil, nil, err } - pc := gps.ProjectConstraint{ - Ident: gps.ProjectIdentifier{ - ProjectRoot: gps.ProjectRoot(pkg.importPath), - Source: pkg.repository, - }, - Constraint: gps.Any(), + ip := importedPackage{ + Name: pkg.importPath, + Source: pkg.repository, + LockHint: pkg.reference, } - - // A vndr entry could contain either a version or a revision - isVersion, version, err := isVersion(pc.Ident, pkg.reference, v.sm) - if err != nil { - return nil, nil, err - } - - // If the reference is a revision, check if it is tagged with a version - if !isVersion { - revision := gps.Revision(pkg.reference) - version, err = lookupVersionForLockedProject(pc.Ident, nil, revision, v.sm) - if err != nil { - v.logger.Println(err.Error()) - } - } - - // Try to build a constraint from the version - pp := getProjectPropertiesFromVersion(version) - if pp.Constraint != nil { - pc.Constraint = pp.Constraint - } - - manifest.Constraints[pc.Ident.ProjectRoot] = gps.ProjectProperties{ - Source: pc.Ident.Source, - Constraint: pc.Constraint, - } - fb.NewConstraintFeedback(pc, fb.DepTypeImported).LogFeedback(v.logger) - - lp := gps.NewLockedProject(pc.Ident, version, nil) - lock.P = append(lock.P, lp) - fb.NewLockedProjectFeedback(lp, fb.DepTypeImported).LogFeedback(v.logger) + packages = append(packages, ip) + } + err := v.importPackages(packages, true) + if err != nil { + return nil, nil, err } - return manifest, lock, nil + return v.manifest, v.lock, nil } type vndrPackage struct { diff --git a/cmd/dep/vndr_importer_test.go b/cmd/dep/vndr_importer_test.go index 8637e195b9..7eac9fccc4 100644 --- a/cmd/dep/vndr_importer_test.go +++ b/cmd/dep/vndr_importer_test.go @@ -19,83 +19,51 @@ import ( func TestVndrConfig_Convert(t *testing.T) { testCases := map[string]struct { - *convertTestCase packages []vndrPackage + convertTestCase }{ - "semver reference": { - packages: []vndrPackage{{ - importPath: "github.com/sdboyer/deptest", - reference: "v0.8.0", - repository: "https://github.com/sdboyer/deptest.git", + "package": { + []vndrPackage{{ + importPath: importerTestProject, + reference: importerTestV1Rev, + repository: importerTestProjectSrc, }}, - convertTestCase: &convertTestCase{ - projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), - wantSourceRepo: "https://github.com/sdboyer/deptest.git", - wantConstraint: "^0.8.0", - wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), - wantVersion: "v0.8.0", - wantLockCount: 1, - }, - }, - "revision reference": { - packages: []vndrPackage{{ - importPath: "github.com/sdboyer/deptest", - reference: "ff2948a2ac8f538c4ecd55962e919d1e13e74baf", - }}, - convertTestCase: &convertTestCase{ - projectRoot: gps.ProjectRoot("github.com/sdboyer/deptest"), - wantConstraint: "^1.0.0", - wantVersion: "v1.0.0", - wantRevision: gps.Revision("ff2948a2ac8f538c4ecd55962e919d1e13e74baf"), - wantLockCount: 1, - }, - }, - "untagged revision reference": { - packages: []vndrPackage{{ - importPath: "github.com/carolynvs/deptest-subpkg", - reference: "6c41d90f78bb1015696a2ad591debfa8971512d5", - }}, - convertTestCase: &convertTestCase{ - projectRoot: gps.ProjectRoot("github.com/carolynvs/deptest-subpkg"), - wantConstraint: "*", - wantVersion: "", - wantRevision: gps.Revision("6c41d90f78bb1015696a2ad591debfa8971512d5"), - wantLockCount: 1, + convertTestCase{ + wantSourceRepo: importerTestProjectSrc, + wantConstraint: importerTestV1Constraint, + wantRevision: importerTestV1Rev, + wantVersion: importerTestV1Tag, }, }, "missing importPath": { - packages: []vndrPackage{{ - reference: "v1.0.0", + []vndrPackage{{ + reference: importerTestV1Tag, }}, - convertTestCase: &convertTestCase{ + convertTestCase{ wantConvertErr: true, }, }, "missing reference": { - packages: []vndrPackage{{ - importPath: "github.com/sdboyer/deptest", + []vndrPackage{{ + importPath: importerTestProject, }}, - convertTestCase: &convertTestCase{ + convertTestCase{ wantConvertErr: true, }, }, } - h := test.NewHelper(t) - defer h.Cleanup() - - ctx := newTestContext(h) - sm, err := ctx.SourceManager() - h.Must(err) - defer sm.Release() - for name, testCase := range testCases { + name := name + testCase := testCase t.Run(name, func(t *testing.T) { - g := newVndrImporter(discardLogger, true, sm) - g.packages = testCase.packages + t.Parallel() - manifest, lock, convertErr := g.convert(testCase.projectRoot) - err = validateConvertTestCase(testCase.convertTestCase, manifest, lock, convertErr) + err := testCase.Exec(t, func(logger *log.Logger, sm gps.SourceManager) (*dep.Manifest, *dep.Lock, error) { + g := newVndrImporter(logger, true, sm) + g.packages = testCase.packages + return g.convert(testProjectRoot) + }) if err != nil { t.Fatalf("%#v", err) }