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

Commit

Permalink
Suppress logging from importers during solve
Browse files Browse the repository at this point in the history
  • Loading branch information
@carolynvs
carolynvs committed Jun 2, 2017
1 parent 373556d commit 1ef8181
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 75 deletions.
37 changes: 20 additions & 17 deletions cmd/dep/glide_importer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package main
import (
"bytes"
"io/ioutil"
"log"
"os"
"path"
"path/filepath"
Expand All @@ -27,14 +28,16 @@ type glideImporter struct {
yaml glideYaml
lock *glideLock

ctx *dep.Ctx
sm gps.SourceManager
logger *log.Logger
verbose bool
sm gps.SourceManager
}

func newGlideImporter(ctx *dep.Ctx, sm gps.SourceManager) *glideImporter {
func newGlideImporter(logger *log.Logger, verbose bool, sm gps.SourceManager) *glideImporter {
return &glideImporter{
ctx: ctx,
sm: sm,
logger: logger,
verbose: verbose,
sm: sm,
}
}

Expand Down Expand Up @@ -93,10 +96,10 @@ func (g *glideImporter) Import(dir string, pr gps.ProjectRoot) (*dep.Manifest, *

// load the glide configuration files.
func (g *glideImporter) load(projectDir string) error {
g.ctx.Err.Println("Detected glide configuration files...")
g.logger.Println("Detected glide configuration files...")
y := filepath.Join(projectDir, glideYamlName)
if g.ctx.Loggers.Verbose {
g.ctx.Loggers.Err.Printf(" Loading %s", y)
if g.verbose {
g.logger.Printf(" Loading %s", y)
}
yb, err := ioutil.ReadFile(y)
if err != nil {
Expand All @@ -109,8 +112,8 @@ func (g *glideImporter) load(projectDir string) error {

l := filepath.Join(projectDir, glideLockName)
if exists, _ := fs.IsRegular(l); exists {
if g.ctx.Loggers.Verbose {
g.ctx.Loggers.Err.Printf(" Loading %s", l)
if g.verbose {
g.logger.Printf(" Loading %s", l)
}
lb, err := ioutil.ReadFile(l)
if err != nil {
Expand All @@ -136,7 +139,7 @@ func (g *glideImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, e
task.WriteString(" and glide.lock")
}
task.WriteString("...")
g.ctx.Loggers.Err.Println(task)
g.logger.Println(task)

manifest := &dep.Manifest{
Constraints: make(gps.ProjectConstraints),
Expand All @@ -161,7 +164,7 @@ func (g *glideImporter) convert(pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, e

if len(g.yaml.ExcludeDirs) > 0 {
if g.yaml.Name != "" && 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)
g.logger.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 @@ -193,12 +196,12 @@ func (g *glideImporter) buildProjectConstraint(pkg glidePackage) (pc gps.Project
return
}

if g.ctx.Loggers.Verbose {
if g.verbose {
if pkg.OS != "" {
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)
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 pkg.Arch != "" {
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)
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)
}
}

Expand All @@ -219,10 +222,10 @@ func (g *glideImporter) buildLockedProject(pkg glideLockedPackage) gps.LockedPro
if err != nil {
// Warn about the problem, it is not enough to warrant failing
warn := errors.Wrapf(err, "Unable to lookup the version represented by %s in %s(%s). Falling back to locking the revision only.", revision, pi.ProjectRoot, pi.Source)
g.ctx.Err.Printf(warn.Error())
g.logger.Printf(warn.Error())
version = revision
}

feedback(version, pi.ProjectRoot, fb.DepTypeImported, g.ctx)
feedback(version, pi.ProjectRoot, fb.DepTypeImported, g.logger)
return gps.NewLockedProject(pi, version, nil)
}
92 changes: 47 additions & 45 deletions cmd/dep/glide_importer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ func newTestContext(h *test.Helper) *dep.Ctx {
}

func TestGlideConfig_Import(t *testing.T) {
t.Parallel()

h := test.NewHelper(t)
defer h.Cleanup()

Expand All @@ -54,12 +56,10 @@ func TestGlideConfig_Import(t *testing.T) {
defer sm.Release()

// Capture stderr so we can verify output
ctx := newTestContext(h)
ctx.Loggers.Verbose = false // Disable verbose so that we don't print values that change each test run
verboseOutput := &bytes.Buffer{}
ctx.Loggers.Err = log.New(verboseOutput, "", 0)
logger := log.New(verboseOutput, "", 0)

g := newGlideImporter(ctx, sm)
g := newGlideImporter(logger, false, sm) // Disable verbose so that we don't print values that change each test run
if !g.HasDepMetadata(projectRoot) {
t.Fatal("Expected the importer to detect the glide configuration files")
}
Expand Down Expand Up @@ -90,6 +90,8 @@ func TestGlideConfig_Import(t *testing.T) {
}

func TestGlideConfig_Import_MissingLockFile(t *testing.T) {
t.Parallel()

h := test.NewHelper(t)
defer h.Cleanup()

Expand All @@ -104,8 +106,8 @@ func TestGlideConfig_Import_MissingLockFile(t *testing.T) {
h.Must(err)
defer sm.Release()

ctx := newTestContext(h)
g := newGlideImporter(ctx, sm)
logger := log.New(os.Stderr, "", 0)
g := newGlideImporter(logger, true, sm)
if !g.HasDepMetadata(projectRoot) {
t.Fatal("The glide importer should gracefully handle when only glide.yaml is present")
}
Expand All @@ -123,6 +125,8 @@ func TestGlideConfig_Import_MissingLockFile(t *testing.T) {
}

func TestGlideConfig_Convert_Project(t *testing.T) {
t.Parallel()

h := test.NewHelper(t)
defer h.Cleanup()

Expand All @@ -134,7 +138,7 @@ func TestGlideConfig_Convert_Project(t *testing.T) {
h.Must(err)
defer sm.Release()

g := newGlideImporter(ctx, sm)
g := newGlideImporter(ctx.Err, true, sm)
g.yaml = glideYaml{
Imports: []glidePackage{
{
Expand Down Expand Up @@ -212,6 +216,8 @@ func TestGlideConfig_Convert_Project(t *testing.T) {
}

func TestGlideConfig_Convert_TestProject(t *testing.T) {
t.Parallel()

h := test.NewHelper(t)
defer h.Cleanup()

Expand All @@ -222,7 +228,7 @@ func TestGlideConfig_Convert_TestProject(t *testing.T) {
h.Must(err)
defer sm.Release()

g := newGlideImporter(ctx, sm)
g := newGlideImporter(ctx.Err, true, sm)
g.yaml = glideYaml{
TestImports: []glidePackage{
{
Expand Down Expand Up @@ -260,13 +266,12 @@ func TestGlideConfig_Convert_TestProject(t *testing.T) {
}

func TestGlideConfig_Convert_Ignore(t *testing.T) {
h := test.NewHelper(t)
defer h.Cleanup()
t.Parallel()

pkg := "github.com/sdboyer/deptest"

ctx := newTestContext(h)
g := newGlideImporter(ctx, nil)
logger := log.New(os.Stderr, "", 0)
g := newGlideImporter(logger, true, nil)
g.yaml = glideYaml{
Ignores: []string{pkg},
}
Expand All @@ -286,11 +291,10 @@ func TestGlideConfig_Convert_Ignore(t *testing.T) {
}

func TestGlideConfig_Convert_ExcludeDir(t *testing.T) {
h := test.NewHelper(t)
defer h.Cleanup()
t.Parallel()

ctx := newTestContext(h)
g := newGlideImporter(ctx, nil)
logger := log.New(os.Stderr, "", 0)
g := newGlideImporter(logger, true, nil)
g.yaml = glideYaml{
ExcludeDirs: []string{"samples"},
}
Expand All @@ -310,11 +314,10 @@ func TestGlideConfig_Convert_ExcludeDir(t *testing.T) {
}

func TestGlideConfig_Convert_ExcludeDir_IgnoresMismatchedPackageName(t *testing.T) {
h := test.NewHelper(t)
defer h.Cleanup()
t.Parallel()

ctx := newTestContext(h)
g := newGlideImporter(ctx, nil)
logger := log.New(os.Stderr, "", 0)
g := newGlideImporter(logger, true, nil)
g.yaml = glideYaml{
Name: "github.com/golang/mismatched-package-name",
ExcludeDirs: []string{"samples"},
Expand All @@ -335,45 +338,44 @@ func TestGlideConfig_Convert_ExcludeDir_IgnoresMismatchedPackageName(t *testing.
}

func TestGlideConfig_Convert_WarnsForUnusedFields(t *testing.T) {
h := test.NewHelper(t)
defer h.Cleanup()
t.Parallel()

testCases := map[string]glidePackage{
"specified an os": {OS: "windows"},
"specified an arch": {Arch: "i686"},
}

for wantWarning, pkg := range testCases {
pkg.Name = "github.com/sdboyer/deptest"
pkg.Reference = "v1.0.0"

ctx := newTestContext(h)
// Capture stderr so we can verify warnings
verboseOutput := &bytes.Buffer{}
ctx.Loggers.Err = log.New(verboseOutput, "", 0)
g := newGlideImporter(ctx, nil)
g.yaml = glideYaml{
Imports: []glidePackage{pkg},
}
t.Run(wantWarning, func(t *testing.T) {
pkg.Name = "github.com/sdboyer/deptest"
pkg.Reference = "v1.0.0"

// Capture stderr so we can verify warnings
verboseOutput := &bytes.Buffer{}
logger := log.New(verboseOutput, "", 0)
g := newGlideImporter(logger, true, nil)
g.yaml = glideYaml{
Imports: []glidePackage{pkg},
}

_, _, err := g.convert(testGlideProjectRoot)
if err != nil {
t.Fatal(err)
}
_, _, err := g.convert(testGlideProjectRoot)
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)
}
warnings := verboseOutput.String()
if !strings.Contains(warnings, wantWarning) {
t.Errorf("Expected the output to include the warning '%s'", wantWarning)
}
})
}
}

func TestGlideConfig_Convert_BadInput_EmptyPackageName(t *testing.T) {
h := test.NewHelper(t)
defer h.Cleanup()
t.Parallel()

ctx := newTestContext(h)
g := newGlideImporter(ctx, nil)
logger := log.New(os.Stderr, "", 0)
g := newGlideImporter(logger, true, nil)
g.yaml = glideYaml{
Imports: []glidePackage{{Name: ""}},
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/dep/gopath_scanner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ 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)
feedback(v, pkg, fb.DepTypeDirect, g.ctx.Err)
}

// Keep track of which projects have been locked
Expand All @@ -110,7 +110,7 @@ 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)
feedback(v, pkg, fb.DepTypeTransitive, g.ctx.Err)
}
}

Expand Down
19 changes: 13 additions & 6 deletions cmd/dep/root_analyzer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import (
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
Expand Down Expand Up @@ -44,7 +46,7 @@ func newRootAnalyzer(skipTools bool, ctx *dep.Ctx, directDeps map[string]bool, s

func (a *rootAnalyzer) InitializeRootManifestAndLock(dir string, pr gps.ProjectRoot) (rootM *dep.Manifest, rootL *dep.Lock, err error) {
if !a.skipTools {
rootM, rootL, err = a.importManifestAndLock(dir, pr)
rootM, rootL, err = a.importManifestAndLock(dir, pr, false)
if err != nil {
return
}
Expand All @@ -63,9 +65,14 @@ func (a *rootAnalyzer) InitializeRootManifestAndLock(dir string, pr gps.ProjectR
return
}

func (a *rootAnalyzer) importManifestAndLock(dir string, pr gps.ProjectRoot) (*dep.Manifest, *dep.Lock, error) {
func (a *rootAnalyzer) importManifestAndLock(dir string, pr gps.ProjectRoot, suppressLogs bool) (*dep.Manifest, *dep.Lock, error) {
logger := a.ctx.Err
if suppressLogs {
logger = log.New(ioutil.Discard, "", 0)
}

importers := []importer{
newGlideImporter(a.ctx, a.sm),
newGlideImporter(logger, a.ctx.Verbose, a.sm),
}

for _, i := range importers {
Expand Down Expand Up @@ -103,7 +110,7 @@ func (a *rootAnalyzer) DeriveManifestAndLock(dir string, pr gps.ProjectRoot) (gp
// The assignment back to an interface prevents interface-based nil checks from failing later
var manifest gps.Manifest = gps.SimpleManifest{}
var lock gps.Lock
im, il, err := a.importManifestAndLock(dir, pr)
im, il, err := a.importManifestAndLock(dir, pr, true)
if im != nil {
manifest = im
}
Expand Down Expand Up @@ -142,7 +149,7 @@ func (a *rootAnalyzer) Info() (string, int) {
}

// feedback logs project constraint as feedback to the user.
func feedback(v gps.Version, pr gps.ProjectRoot, depType string, ctx *dep.Ctx) {
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.
Expand Down Expand Up @@ -182,7 +189,7 @@ func feedback(v gps.Version, pr gps.ProjectRoot, depType string, ctx *dep.Ctx) {
}
}

cf.LogFeedback(ctx)
cf.LogFeedback(logger)
}

func lookupVersionForRevision(rev gps.Revision, pi gps.ProjectIdentifier, sm gps.SourceManager) (gps.Version, error) {
Expand Down
Loading

0 comments on commit 1ef8181

Please sign in to comment.