Standardize import output and use new feedback system

carolynvs · carolynvs · commit 41ed16413609 · 2017-05-19T16:48:27.000-05:00
diff --git a/cmd/dep/glide_config.go b/cmd/dep/glide_config.go
@@ -5,24 +5,26 @@
 package main
 
 import (
+	"bytes"
 	"io/ioutil"
 	"path"
 	"path/filepath"
 
 	"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"
 )
 
 type glideFiles struct {
-	yaml    glideYaml
-	lock    *glideLock
-	loggers *dep.Loggers
+	yaml glideYaml
+	lock *glideLock
+	ctx  *dep.Ctx
 }
 
-func newGlideFiles(loggers *dep.Loggers) *glideFiles {
-	return &glideFiles{loggers: loggers}
+func newGlideFiles(ctx *dep.Ctx) *glideFiles {
+	return &glideFiles{ctx: ctx}
 }
 
 type glideYaml struct {
@@ -34,8 +36,8 @@ type glideYaml struct {
 }
 
 type glideLock struct {
-	Imports     []glidePackage `yaml:"import"`
-	TestImports []glidePackage `yaml:"testImport"`
+	Imports     []glidePackage `yaml:"imports"`
+	TestImports []glidePackage `yaml:"testImports"`
 }
 
 type glidePackage struct {
@@ -50,9 +52,10 @@ type glidePackage struct {
 }
 
 func (g *glideFiles) load(projectDir string) error {
+	g.ctx.Err.Println("Detected glide configuration files...")
 	y := filepath.Join(projectDir, glideYamlName)
-	if g.loggers.Verbose {
-		g.loggers.Err.Printf("glide: Loading %s", y)
+	if g.ctx.Loggers.Verbose {
+		g.ctx.Loggers.Err.Printf("  Loading %s", y)
 	}
 	yb, err := ioutil.ReadFile(y)
 	if err != nil {
@@ -65,23 +68,32 @@ func (g *glideFiles) load(projectDir string) error {
 
 	l := filepath.Join(projectDir, glideLockName)
 	if exists, _ := dep.IsRegular(l); exists {
-		if g.loggers.Verbose {
-			g.loggers.Err.Printf("glide: Loading %s", l)
+		if g.ctx.Loggers.Verbose {
+			g.ctx.Loggers.Err.Printf("  Loading %s", l)
 		}
 		lb, err := ioutil.ReadFile(l)
 		if err != nil {
 			return errors.Wrapf(err, "Unable to read %s", l)
 		}
-		err = yaml.Unmarshal(lb, &g.lock)
+		lock := &glideLock{}
+		err = yaml.Unmarshal(lb, lock)
 		if err != nil {
 			return errors.Wrapf(err, "Unable to parse %s", l)
 		}
+		g.lock = lock
 	}
 
 	return nil
 }
 
 func (g *glideFiles) convert(projectName string, sm gps.SourceManager) (*dep.Manifest, *dep.Lock, error) {
+	task := bytes.NewBufferString("Converting from glide.yaml")
+	if g.lock != nil {
+		task.WriteString(" and glide.lock")
+	}
+	task.WriteString("...")
+	g.ctx.Loggers.Err.Println(task)
+
 	manifest := &dep.Manifest{
 		Dependencies: make(gps.ProjectConstraints),
 	}
@@ -105,7 +117,7 @@ func (g *glideFiles) convert(projectName string, sm gps.SourceManager) (*dep.Man
 
 	if len(g.yaml.ExcludeDirs) > 0 {
 		if g.yaml.Name != "" && g.yaml.Name != projectName {
-			g.loggers.Err.Printf("dep: Glide thinks the package is '%s' but dep thinks it is '%s', using dep's value.\n", g.yaml.Name, projectName)
+			g.ctx.Loggers.Err.Printf("  Glide thinks the package is '%s' but dep thinks it is '%s', using dep's value.\n", g.yaml.Name, projectName)
 		}
 
 		for _, dir := range g.yaml.ExcludeDirs {
@@ -133,24 +145,22 @@ func (g *glideFiles) convert(projectName string, sm gps.SourceManager) (*dep.Man
 
 func (g *glideFiles) buildProjectConstraint(pkg glidePackage, sm gps.SourceManager) (pc gps.ProjectConstraint, err error) {
 	if pkg.Name == "" {
-		err = errors.New("glide: Invalid glide configuration, package name is required")
+		err = errors.New("Invalid glide configuration, package name is required")
 		return
 	}
 
-	if g.loggers.Verbose {
+	if g.ctx.Loggers.Verbose {
 		if pkg.OS != "" {
-			g.loggers.Err.Printf("glide: 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)
+			g.ctx.Loggers.Err.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 pkg.Arch != "" {
-			g.loggers.Err.Printf("glide: 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)
-		}
-		if len(pkg.Subpackages) > 0 {
-			g.loggers.Err.Printf("glide: The %s package specified subpackages, but that is calcuated by dep, and will be ignored.\n", pkg.Name)
+			g.ctx.Loggers.Err.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)
 		}
 	}
 
 	pc.Ident = gps.ProjectIdentifier{ProjectRoot: gps.ProjectRoot(pkg.Name), Source: pkg.Repository}
 	pc.Constraint, err = deduceConstraint(pkg.Reference, pc.Ident, sm)
+
 	return
 }
 
@@ -167,5 +177,6 @@ func (g *glideFiles) buildLockedProject(pkg glidePackage, manifest *dep.Manifest
 		version = revision
 	}
 
+	feedback(version, id.ProjectRoot, fb.DepTypeDirect, g.ctx)
 	return gps.NewLockedProject(id, version, nil)
 }
diff --git a/cmd/dep/glide_config_test.go b/cmd/dep/glide_config_test.go
@@ -15,15 +15,22 @@ import (
 	"github.com/golang/dep/internal/gps"
 )
 
-func TestGlideConvertProject(t *testing.T) {
-	loggers := &dep.Loggers{
+func newTestLoggers() *dep.Loggers {
+	return &dep.Loggers{
 		Out:     log.New(os.Stdout, "", 0),
 		Err:     log.New(os.Stderr, "", 0),
 		Verbose: true,
 	}
+}
+func newTestContext() *dep.Ctx {
+	return &dep.Ctx{
+		Loggers: newTestLoggers(),
+	}
+}
 
+func TestGlideConvertProject(t *testing.T) {
 	f := glideFiles{
-		loggers: loggers,
+		ctx: newTestContext(),
 		yaml: glideYaml{
 			Imports: []glidePackage{
 				{
@@ -93,14 +100,8 @@ func TestGlideConvertProject(t *testing.T) {
 }
 
 func TestGlideConvertTestProject(t *testing.T) {
-	loggers := &dep.Loggers{
-		Out:     log.New(os.Stdout, "", 0),
-		Err:     log.New(os.Stderr, "", 0),
-		Verbose: true,
-	}
-
 	f := glideFiles{
-		loggers: loggers,
+		ctx: newTestContext(),
 		yaml: glideYaml{
 			TestImports: []glidePackage{
 				{
@@ -139,14 +140,8 @@ func TestGlideConvertTestProject(t *testing.T) {
 }
 
 func TestGlideConvertIgnore(t *testing.T) {
-	loggers := &dep.Loggers{
-		Out:     log.New(os.Stdout, "", 0),
-		Err:     log.New(os.Stderr, "", 0),
-		Verbose: true,
-	}
-
 	f := glideFiles{
-		loggers: loggers,
+		ctx: newTestContext(),
 		yaml: glideYaml{
 			Ignores: []string{"github.com/sdboyer/deptest"},
 		},
@@ -167,14 +162,8 @@ func TestGlideConvertIgnore(t *testing.T) {
 }
 
 func TestGlideConvertExcludeDir(t *testing.T) {
-	loggers := &dep.Loggers{
-		Out:     log.New(os.Stdout, "", 0),
-		Err:     log.New(os.Stderr, "", 0),
-		Verbose: true,
-	}
-
 	f := glideFiles{
-		loggers: loggers,
+		ctx: newTestContext(),
 		yaml: glideYaml{
 			ExcludeDirs: []string{"samples"},
 		},
@@ -195,14 +184,8 @@ func TestGlideConvertExcludeDir(t *testing.T) {
 }
 
 func TestGlideConvertExcludeDir_IgnoresMismatchedPackageName(t *testing.T) {
-	loggers := &dep.Loggers{
-		Out:     log.New(os.Stdout, "", 0),
-		Err:     log.New(os.Stderr, "", 0),
-		Verbose: true,
-	}
-
 	f := glideFiles{
-		loggers: loggers,
+		ctx: newTestContext(),
 		yaml: glideYaml{
 			Name:        "github.com/golang/mismatched-package-name",
 			ExcludeDirs: []string{"samples"},
@@ -235,19 +218,14 @@ func TestGlideConvertWarnsForUnusedFields(t *testing.T) {
 		pkg.Reference = "v1.0.0"
 
 		// Capture stderr so we can verify warnings
-		verboseOutput := &bytes.Buffer{}
-		loggers := &dep.Loggers{
-			Out:     log.New(os.Stdout, "", 0),
-			Err:     log.New(verboseOutput, "", 0),
-			Verbose: true,
-		}
-
 		f := glideFiles{
-			loggers: loggers,
+			ctx: newTestContext(),
 			yaml: glideYaml{
 				Imports: []glidePackage{pkg},
 			},
 		}
+		verboseOutput := &bytes.Buffer{}
+		f.ctx.Loggers.Err = log.New(verboseOutput, "", 0)
 
 		_, _, err := f.convert("", nil)
 		if err != nil {
@@ -262,14 +240,8 @@ func TestGlideConvertWarnsForUnusedFields(t *testing.T) {
 }
 
 func TestGlideConvertBadInput_EmptyPackageName(t *testing.T) {
-	loggers := &dep.Loggers{
-		Out:     log.New(os.Stdout, "", 0),
-		Err:     log.New(os.Stderr, "", 0),
-		Verbose: true,
-	}
-
 	f := glideFiles{
-		loggers: loggers,
+		ctx: newTestContext(),
 		yaml: glideYaml{
 			Imports: []glidePackage{{Name: ""}},
 		},
diff --git a/cmd/dep/glide_importer.go b/cmd/dep/glide_importer.go
@@ -16,12 +16,12 @@ const glideYamlName = "glide.yaml"
 const glideLockName = "glide.lock"
 
 type glideImporter struct {
-	loggers *dep.Loggers
-	sm      gps.SourceManager
+	ctx *dep.Ctx
+	sm  gps.SourceManager
 }
 
-func newGlideImporter(loggers *dep.Loggers, sm gps.SourceManager) glideImporter {
-	return glideImporter{loggers: loggers, sm: sm}
+func newGlideImporter(ctx *dep.Ctx, sm gps.SourceManager) glideImporter {
+	return glideImporter{ctx: ctx, sm: sm}
 }
 
 func (i glideImporter) HasConfig(dir string) bool {
@@ -35,7 +35,7 @@ func (i glideImporter) HasConfig(dir string) bool {
 }
 
 func (i glideImporter) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) {
-	files := newGlideFiles(i.loggers)
+	files := newGlideFiles(i.ctx)
 	err := files.load(dir)
 	if err != nil {
 		return nil, nil, err
diff --git a/cmd/dep/glide_importer_test.go b/cmd/dep/glide_importer_test.go
@@ -5,12 +5,9 @@
 package main
 
 import (
-	"log"
-	"os"
 	"path/filepath"
 	"testing"
 
-	"github.com/golang/dep"
 	"github.com/golang/dep/internal/gps"
 	"github.com/golang/dep/internal/test"
 )
@@ -28,16 +25,11 @@ func TestGlideImport(t *testing.T) {
 	h.TempCopy(filepath.Join(testGlideProjectRoot, glideYamlName), "glide.yaml")
 	h.TempCopy(filepath.Join(testGlideProjectRoot, glideLockName), "glide.lock")
 
-	loggers := &dep.Loggers{
-		Out:     log.New(os.Stdout, "", 0),
-		Err:     log.New(os.Stderr, "", 0),
-		Verbose: true,
-	}
 	projectRoot := h.Path(testGlideProjectRoot)
 	sm, err := gps.NewSourceManager(h.Path(cacheDir))
 	h.Must(err)
 
-	i := newGlideImporter(loggers, sm)
+	i := newGlideImporter(newTestContext(), sm)
 	if !i.HasConfig(projectRoot) {
 		t.Fatal("Expected the importer to detect the glide configuration files")
 	}
@@ -64,16 +56,11 @@ func TestGlideImport_MissingLockFile(t *testing.T) {
 	h.TempDir(filepath.Join("src", "glidetest"))
 	h.TempCopy(filepath.Join("glidetest", glideYamlName), "glide.yaml")
 
-	loggers := &dep.Loggers{
-		Out:     log.New(os.Stdout, "", 0),
-		Err:     log.New(os.Stderr, "", 0),
-		Verbose: true,
-	}
 	projectRoot := h.Path("glidetest")
 	sm, err := gps.NewSourceManager(h.Path(cacheDir))
 	h.Must(err)
 
-	i := newGlideImporter(loggers, sm)
+	i := newGlideImporter(newTestContext(), sm)
 	if !i.HasConfig(projectRoot) {
 		t.Fatal("The glide importer should gracefully handle when only glide.yaml is present")
 	}
diff --git a/cmd/dep/init.go b/cmd/dep/init.go
@@ -92,15 +92,13 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 	if err != nil {
 		return errors.Wrap(err, "determineProjectRoot")
 	}
-	if ctx.Loggers.Verbose {
-		ctx.Loggers.Err.Printf("dep: Finding dependencies for %q...\n", cpr)
-	}
+	ctx.Loggers.Err.Printf("Finding dependencies of %q...\n", cpr)
 	pkgT, err := pkgtree.ListPackages(root, cpr)
 	if err != nil {
 		return errors.Wrap(err, "gps.ListPackages")
 	}
 	if ctx.Loggers.Verbose {
-		ctx.Loggers.Err.Printf("dep: Found %d dependencies.\n", len(pkgT.Packages))
+		ctx.Loggers.Err.Printf("  Found %d dependencies.\n", len(pkgT.Packages))
 	}
 	sm, err := ctx.SourceManager()
 	if err != nil {
@@ -110,7 +108,6 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error {
 	defer sm.Release()
 
 	// Generate a manifest and lock for the root project
-	ctx.Loggers.Err.Println("Searching GOPATH for projects...")
 	rootAnalyzer := newRootAnalyzer(cmd.skipTools, ctx, pkgT, sm)
 	m, l, err := rootAnalyzer.DeriveRootManifestAndLock(root, gps.ProjectRoot(cpr))
 	if err != nil {
diff --git a/cmd/dep/root_analyzer.go b/cmd/dep/root_analyzer.go
@@ -111,14 +111,11 @@ func overlayManifestAndLock(rootM *dep.Manifest, rootL *dep.Lock, m *dep.Manifes
 
 func (a *rootAnalyzer) importManifestAndLock(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) {
 	importers := []importer{
-		newGlideImporter(a.ctx.Loggers, a.sm),
+		newGlideImporter(a.ctx, a.sm),
 	}
 
 	for _, i := range importers {
 		if i.HasConfig(dir) {
-			if a.ctx.Loggers.Verbose {
-				a.ctx.Loggers.Err.Printf("Importing %T configuration for %s. Run with -skip-tools to skip.", i, pr)
-			}
 			return i.Import(dir, pr)
 		}
 	}
@@ -132,6 +129,7 @@ func (a *rootAnalyzer) importManifestAndLock(dir string, pr gps.ProjectRoot) (*d
 func (a *rootAnalyzer) deriveManifestAndLockFromGopath() (*dep.Manifest, *dep.Lock, error) {
 	var err error
 
+	a.ctx.Loggers.Err.Println("Searching GOPATH for projects...")
 	a.pd, err = getProjectData(a.ctx, a.pkgT, a.sm)
 	if err != nil {
 		return nil, nil, err
diff --git a/internal/feedback/feedback.go b/internal/feedback/feedback.go
@@ -17,6 +17,7 @@ const ConsTypeHint = "hint"
 // Dependency types
 const DepTypeDirect = "direct dep"
 const DepTypeTransitive = "transitive dep"
+const DepTypeImported = "imported dep"
 
 // ConstraintFeedback holds project constraint feedback data
 type ConstraintFeedback struct {
