Merge branch 'feedback' into detect-branch-vs-tag

Author: carolynvs

cmd/dep/glide_importer.go

@@ -208,6 +208,8 @@ func (g *glideImporter) buildProjectConstraint(pkg glidePackage) (pc gps.Project
 	pc.Ident = gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.Name), Source: pkg.Repository}
 	pc.Constraint, err = deduceConstraint(pkg.Reference, pc.Ident, g.sm)
 
+	f := fb.NewConstraintFeedback(pc.Constraint, pc.Ident.ProjectRoot, fb.DepTypeImported)
+	f.LogFeedback(g.logger)
 	return
 }
 
@@ -226,6 +228,8 @@ func (g *glideImporter) buildLockedProject(pkg glideLockedPackage) gps.LockedPro
 		version = revision
 	}
 
-	feedback(version, pi.ProjectRoot, fb.DepTypeImported, g.logger)
+	f := fb.NewLockedProjectFeedback(version, pi.ProjectRoot, fb.DepTypeImported)
+	f.LogFeedback(g.logger)
+
 	return gps.NewLockedProject(pi, version, nil)
 }

cmd/dep/gopath_scanner.go

@@ -91,7 +91,10 @@ func (g *gopathScanner) overlay(rootM *dep.Manifest, rootL *dep.Lock) {
 		}
 		rootM.Constraints[pkg] = prj
 		v := g.pd.ondisk[pkg]
-		feedback(v, pkg, fb.DepTypeDirect, g.ctx.Err)
+		f := fb.NewConstraintFeedback(v, pkg, fb.DepTypeDirect)
+		f.LogFeedback(g.ctx.Err)
+		f = fb.NewLockedProjectFeedback(v, pkg, fb.DepTypeDirect)
+		f.LogFeedback(g.ctx.Err)
 	}
 
 	// Keep track of which projects have been locked
@@ -110,7 +113,8 @@ func (g *gopathScanner) overlay(rootM *dep.Manifest, rootL *dep.Lock) {
 
 		if _, isDirect := g.directDeps[string(pkg)]; !isDirect {
 			v := g.pd.ondisk[pkg]
-			feedback(v, pkg, fb.DepTypeTransitive, g.ctx.Err)
+			f := fb.NewLockedProjectFeedback(v, pkg, fb.DepTypeTransitive)
+			f.LogFeedback(g.ctx.Err)
 		}
 	}
 

cmd/dep/root_analyzer.go

@@ -5,14 +5,12 @@
 package main
 
 import (
-	"encoding/hex"
+	"io/ioutil"
+	"log"
 
 	"github.com/golang/dep"
-	fb "github.com/golang/dep/internal/feedback"
 	"github.com/golang/dep/internal/gps"
 	"github.com/pkg/errors"
-	"io/ioutil"
-	"log"
 )
 
 // importer handles importing configuration from other dependency managers into
@@ -148,50 +146,6 @@ func (a *rootAnalyzer) Info() (string, int) {
 	return name, version
 }
 
-// feedback logs project constraint as feedback to the user.
-func feedback(v gps.Version, pr gps.ProjectRoot, depType string, logger *log.Logger) {
-	rev, version, branch := gps.VersionComponentStrings(v)
-
-	// Check if it's a valid SHA1 digest and trim to 7 characters.
-	if len(rev) == 40 {
-		if _, err := hex.DecodeString(rev); err == nil {
-			// Valid SHA1 digest
-			rev = rev[0:7]
-		}
-	}
-
-	// Get LockedVersion
-	var ver string
-	if version != "" {
-		ver = version
-	} else if branch != "" {
-		ver = branch
-	}
-
-	cf := &fb.ConstraintFeedback{
-		LockedVersion:  ver,
-		Revision:       rev,
-		ProjectPath:    string(pr),
-		DependencyType: depType,
-	}
-
-	// Get non-revision constraint if available
-	if c := getProjectPropertiesFromVersion(v).Constraint; c != nil {
-		cf.Version = c.String()
-	}
-
-	// Attach ConstraintType for direct/imported deps based on locked version
-	if cf.DependencyType == fb.DepTypeDirect || cf.DependencyType == fb.DepTypeImported {
-		if cf.LockedVersion != "" {
-			cf.ConstraintType = fb.ConsTypeConstraint
-		} else {
-			cf.ConstraintType = fb.ConsTypeHint
-		}
-	}
-
-	cf.LogFeedback(logger)
-}
-
 func lookupVersionForRevision(rev gps.Revision, pi gps.ProjectIdentifier, sm gps.SourceManager) (gps.Version, error) {
 	// Find the version that goes with this revision, if any
 	versions, err := sm.ListVersions(pi)

cmd/dep/testdata/glide/expected_import_output.txt

@@ -1,7 +1,7 @@
 Detected glide configuration files...
 Converting from glide.yaml and glide.lock...
-  Using ^0.8.1 as initial constraint for imported dep github.com/sdboyer/deptest
-  Trying v0.8.1 (3f4c3be) as initial lock for imported dep github.com/sdboyer/deptest
+  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 master 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
   Trying v2.0.0 (5c60720) as initial lock for imported dep github.com/sdboyer/deptestdos
-  Using cb00e56 as initial hint for imported dep github.com/golang/lint

internal/feedback/feedback.go

@@ -5,8 +5,11 @@
 package feedback
 
 import (
+	"encoding/hex"
 	"fmt"
 	"log"
+
+	"github.com/golang/dep/internal/gps"
 )
 
 // Constraint types
@@ -25,23 +28,50 @@ const DepTypeImported = "imported dep"
 
 // ConstraintFeedback holds project constraint feedback data
 type ConstraintFeedback struct {
-	Version, LockedVersion, Revision, ConstraintType, DependencyType, ProjectPath string
+	Constraint, LockedVersion, Revision, ConstraintType, DependencyType, ProjectPath string
 }
 
-// LogFeedback logs the feedback
+// NewConstraintFeedback builds a feedback entry for a constraint in the manifest.
+func NewConstraintFeedback(c gps.Constraint, pr gps.ProjectRoot, depType string) *ConstraintFeedback {
+	cf := &ConstraintFeedback{
+		Constraint:     c.String(),
+		ProjectPath:    string(pr),
+		DependencyType: depType,
+	}
+
+	if _, ok := c.(gps.Revision); ok {
+		cf.ConstraintType = ConsTypeHint
+	} else {
+		cf.ConstraintType = ConsTypeConstraint
+	}
+
+	return cf
+}
+
+// NewLockedProjectFeedback builds a feedback entry for a project in the lock.
+func NewLockedProjectFeedback(v gps.Version, pr gps.ProjectRoot, depType string) *ConstraintFeedback {
+	cf := &ConstraintFeedback{
+		ProjectPath:    string(pr),
+		DependencyType: depType,
+	}
+
+	switch vt := v.(type) {
+	case gps.PairedVersion:
+		cf.LockedVersion = vt.String()
+		cf.Revision = vt.Underlying().String()
+	case gps.Revision:
+		cf.Revision = vt.String()
+	}
+
+	return cf
+}
+
+// LogFeedback logs feedback on changes made to the manifest or lock.
 func (cf ConstraintFeedback) LogFeedback(logger *log.Logger) {
-	// "Using" feedback for direct dep
-	if cf.DependencyType == DepTypeDirect || cf.DependencyType == DepTypeImported {
-		ver := cf.Version
-		// revision as version for hint
-		if cf.ConstraintType == ConsTypeHint {
-			ver = cf.Revision
-		}
-		logger.Printf("  %v", GetUsingFeedback(ver, cf.ConstraintType, cf.DependencyType, cf.ProjectPath))
+	if cf.Constraint != "" {
+		logger.Printf("  %v", GetUsingFeedback(cf.Constraint, cf.ConstraintType, cf.DependencyType, cf.ProjectPath))
 	}
-	// No "Locking" feedback for hints. "Locking" feedback only for constraint
-	// and transitive dep
-	if cf.ConstraintType != ConsTypeHint {
+	if cf.LockedVersion != "" && cf.Revision != "" {
 		logger.Printf("  %v", GetLockingFeedback(cf.LockedVersion, cf.Revision, cf.DependencyType, cf.ProjectPath))
 	}
 }
@@ -62,6 +92,14 @@ func GetUsingFeedback(version, consType, depType, projectPath string) string {
 // Locking in v1.1.4 (bc29b4f) for direct dep github.com/foo/bar
 // Locking in master (436f39d) for transitive dep github.com/baz/qux
 func GetLockingFeedback(version, revision, depType, projectPath string) string {
+	// Check if it's a valid SHA1 digest and trim to 7 characters.
+	if len(revision) == 40 {
+		if _, err := hex.DecodeString(revision); err == nil {
+			// Valid SHA1 digest
+			revision = revision[0:7]
+		}
+	}
+
 	if depType == DepTypeImported {
 		return fmt.Sprintf("Trying %s (%s) as initial lock for %s %s", version, revision, depType, projectPath)
 	}