Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Commit

Permalink
Standardize import output and use new feedback system
Browse files Browse the repository at this point in the history
  • Loading branch information
carolynvs committed May 19, 2017
1 parent fa92f78 commit 05c8a62
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 95 deletions.
51 changes: 31 additions & 20 deletions cmd/dep/glide_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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),
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}

Expand All @@ -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)
}
64 changes: 18 additions & 46 deletions cmd/dep/glide_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
{
Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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"},
},
Expand All @@ -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"},
},
Expand All @@ -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"},
Expand Down Expand Up @@ -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 {
Expand All @@ -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: ""}},
},
Expand Down
10 changes: 5 additions & 5 deletions cmd/dep/glide_importer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
17 changes: 2 additions & 15 deletions cmd/dep/glide_importer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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")
}
Expand All @@ -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")
}
Expand Down
7 changes: 2 additions & 5 deletions cmd/dep/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
6 changes: 2 additions & 4 deletions cmd/dep/root_analyzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions internal/feedback/feedback.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit 05c8a62

Please sign in to comment.