From cbded81ec1bcd731462dd7e7d4a4228a927326d4 Mon Sep 17 00:00:00 2001 From: Jordan Krage Date: Tue, 9 May 2017 23:57:14 -0500 Subject: [PATCH] move Logger into Ctx; restore logging --- analyzer.go | 3 ++- cmd/dep/ensure.go | 12 ++++----- cmd/dep/hash_in.go | 4 +-- cmd/dep/init.go | 48 ++++++++++++++++++------------------ cmd/dep/loggers.go | 14 ----------- cmd/dep/main.go | 8 +++--- cmd/dep/prune.go | 6 ++--- cmd/dep/remove.go | 10 ++++---- cmd/dep/status.go | 8 +++--- context.go | 19 +++++++++++--- context_test.go | 15 ++++++++--- manifest.go | 19 ++++++-------- manifest_test.go | 4 +-- project_test.go | 4 +-- test_project_context_test.go | 8 ++++-- 15 files changed, 95 insertions(+), 87 deletions(-) delete mode 100644 cmd/dep/loggers.go diff --git a/analyzer.go b/analyzer.go index b513d370c5..a2076691d9 100644 --- a/analyzer.go +++ b/analyzer.go @@ -27,7 +27,8 @@ func (a Analyzer) DeriveManifestAndLock(path string, n gps.ProjectRoot) (gps.Man } defer f.Close() - m, err := readManifest(f) + // Ignore warnings irrelevant to user. + m, _, err := readManifest(f) if err != nil { return nil, nil, err } diff --git a/cmd/dep/ensure.go b/cmd/dep/ensure.go index 8c9bfa3d27..b10ec0f622 100644 --- a/cmd/dep/ensure.go +++ b/cmd/dep/ensure.go @@ -101,9 +101,9 @@ type ensureCommand struct { overrides stringSlice } -func (cmd *ensureCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) error { +func (cmd *ensureCommand) Run(ctx *dep.Ctx, args []string) error { if cmd.examples { - loggers.Err.Println(strings.TrimSpace(ensureExamples)) + ctx.Loggers.Err.Println(strings.TrimSpace(ensureExamples)) return nil } @@ -120,8 +120,8 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) err defer sm.Release() params := p.MakeParams() - if loggers.Verbose { - params.TraceLogger = loggers.Err + if ctx.Loggers.Verbose { + params.TraceLogger = ctx.Loggers.Err } params.RootPackageTree, err = pkgtree.ListPackages(p.AbsRoot, string(p.ImportRoot)) if err != nil { @@ -135,7 +135,7 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) err if cmd.update { applyUpdateArgs(args, ¶ms) } else { - err := applyEnsureArgs(loggers.Err, args, cmd.overrides, p, sm, ¶ms) + err := applyEnsureArgs(ctx.Loggers.Err, args, cmd.overrides, p, sm, ¶ms) if err != nil { return err } @@ -168,7 +168,7 @@ func (cmd *ensureCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) err return err } if cmd.dryRun { - return sw.PrintPreparedActions(loggers.Out) + return sw.PrintPreparedActions(ctx.Loggers.Out) } return errors.Wrap(sw.Write(p.AbsRoot, sm, true), "grouped write of manifest, lock and vendor") diff --git a/cmd/dep/hash_in.go b/cmd/dep/hash_in.go index b1e2b3d67d..4b76a16ccd 100644 --- a/cmd/dep/hash_in.go +++ b/cmd/dep/hash_in.go @@ -23,7 +23,7 @@ func (cmd *hashinCommand) Register(fs *flag.FlagSet) {} type hashinCommand struct{} -func (hashinCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) error { +func (hashinCommand) Run(ctx *dep.Ctx, args []string) error { p, err := ctx.LoadProject("") if err != nil { return err @@ -51,6 +51,6 @@ func (hashinCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) error { if err != nil { return errors.Wrap(err, "prepare solver") } - loggers.Out.Println(gps.HashingInputsAsString(s)) + ctx.Loggers.Out.Println(gps.HashingInputsAsString(s)) return nil } diff --git a/cmd/dep/init.go b/cmd/dep/init.go index c636568749..76a01d1696 100644 --- a/cmd/dep/init.go +++ b/cmd/dep/init.go @@ -58,7 +58,7 @@ func trimPathPrefix(p1, p2 string) string { return p1 } -func (cmd *initCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) error { +func (cmd *initCommand) Run(ctx *dep.Ctx, args []string) error { if len(args) > 1 { return errors.Errorf("too many args (%d)", len(args)) } @@ -94,15 +94,15 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) error if err != nil { return errors.Wrap(err, "determineProjectRoot") } - if loggers.Verbose { - loggers.Err.Printf("dep: Finding dependencies for %q...\n", cpr) + if ctx.Loggers.Verbose { + ctx.Loggers.Err.Printf("dep: Finding dependencies for %q...\n", cpr) } pkgT, err := pkgtree.ListPackages(root, cpr) if err != nil { return errors.Wrap(err, "gps.ListPackages") } - if loggers.Verbose { - loggers.Err.Printf("dep: Found %d dependencies.\n", len(pkgT.Packages)) + if ctx.Loggers.Verbose { + ctx.Loggers.Err.Printf("dep: Found %d dependencies.\n", len(pkgT.Packages)) } sm, err := ctx.SourceManager() if err != nil { @@ -111,7 +111,7 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) error sm.UseDefaultSignalHandling() defer sm.Release() - pd, err := getProjectData(ctx, loggers, pkgT, cpr, sm) + pd, err := getProjectData(ctx, pkgT, cpr, sm) if err != nil { return err } @@ -144,8 +144,8 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) error } // Run solver with project versions found on disk - if loggers.Verbose { - loggers.Err.Println("dep: Solving...") + if ctx.Loggers.Verbose { + ctx.Loggers.Err.Println("dep: Solving...") } params := gps.SolveParameters{ RootDir: root, @@ -155,8 +155,8 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) error ProjectAnalyzer: dep.Analyzer{}, } - if loggers.Verbose { - params.TraceLogger = loggers.Err + if ctx.Loggers.Verbose { + params.TraceLogger = ctx.Loggers.Err } s, err := gps.Prepare(params, sm) @@ -190,8 +190,8 @@ func (cmd *initCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) error l.Memo = s.HashInputs() - if loggers.Verbose { - loggers.Err.Println("dep: Writing manifest and lock files.") + if ctx.Loggers.Verbose { + ctx.Loggers.Err.Println("dep: Writing manifest and lock files.") } sw, err := dep.NewSafeWriter(m, nil, l, dep.VendorAlways) @@ -277,7 +277,7 @@ type projectData struct { ondisk map[gps.ProjectRoot]gps.Version // projects that were found on disk } -func getProjectData(ctx *dep.Ctx, loggers *Loggers, pkgT pkgtree.PackageTree, cpr string, sm gps.SourceManager) (projectData, error) { +func getProjectData(ctx *dep.Ctx, pkgT pkgtree.PackageTree, cpr string, sm gps.SourceManager) (projectData, error) { constraints := make(gps.ProjectConstraints) dependencies := make(map[gps.ProjectRoot][]string) packages := make(map[string]bool) @@ -290,7 +290,7 @@ func getProjectData(ctx *dep.Ctx, loggers *Loggers, pkgT pkgtree.PackageTree, cp if err := sm.SyncSourceFor(gps.ProjectIdentifier{ProjectRoot: pr}); err != nil { message = "Unable to cache" } - loggers.Err.Printf("%s %s\n", message, pr) + ctx.Loggers.Err.Printf("%s %s\n", message, pr) syncDepGroup.Done() } @@ -299,8 +299,8 @@ func getProjectData(ctx *dep.Ctx, loggers *Loggers, pkgT pkgtree.PackageTree, cp return projectData{}, nil } - if loggers.Verbose { - loggers.Err.Println("dep: Building dependency graph...") + if ctx.Loggers.Verbose { + ctx.Loggers.Err.Println("dep: Building dependency graph...") } // Exclude stdlib imports from the list returned from Flatten(). const omitStdlib = false @@ -318,16 +318,16 @@ func getProjectData(ctx *dep.Ctx, loggers *Loggers, pkgT pkgtree.PackageTree, cp syncDepGroup.Add(1) go syncDep(pr, sm) - if loggers.Verbose { - loggers.Err.Printf("dep: Found import of %q, analyzing...\n", ip) + if ctx.Loggers.Verbose { + ctx.Loggers.Err.Printf("dep: Found import of %q, analyzing...\n", ip) } dependencies[pr] = []string{ip} v, err := ctx.VersionInWorkspace(pr) if err != nil { notondisk[pr] = true - if loggers.Verbose { - loggers.Err.Printf("dep: Could not determine version for %q, omitting from generated manifest\n", pr) + if ctx.Loggers.Verbose { + ctx.Loggers.Err.Printf("dep: Could not determine version for %q, omitting from generated manifest\n", pr) } continue } @@ -336,8 +336,8 @@ func getProjectData(ctx *dep.Ctx, loggers *Loggers, pkgT pkgtree.PackageTree, cp constraints[pr] = getProjectPropertiesFromVersion(v) } - if loggers.Verbose { - loggers.Err.Printf("dep: Analyzing transitive imports...\n") + if ctx.Loggers.Verbose { + ctx.Loggers.Err.Printf("dep: Analyzing transitive imports...\n") } // Explore the packages we've found for transitive deps, either // completing the lock or identifying (more) missing projects that we'll @@ -357,8 +357,8 @@ func getProjectData(ctx *dep.Ctx, loggers *Loggers, pkgT pkgtree.PackageTree, cp dft = func(pkg string) error { switch colors[pkg] { case white: - if loggers.Verbose { - loggers.Err.Printf("dep: Analyzing %q...\n", pkg) + if ctx.Loggers.Verbose { + ctx.Loggers.Err.Printf("dep: Analyzing %q...\n", pkg) } colors[pkg] = grey diff --git a/cmd/dep/loggers.go b/cmd/dep/loggers.go deleted file mode 100644 index 7e63bcda6a..0000000000 --- a/cmd/dep/loggers.go +++ /dev/null @@ -1,14 +0,0 @@ -// 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" - -// Loggers holds standard loggers and a verbosity flag. -type Loggers struct { - Out, Err *log.Logger - // Whether verbose logging is enabled. - Verbose bool -} diff --git a/cmd/dep/main.go b/cmd/dep/main.go index aceee7055a..ae4cf3ab94 100644 --- a/cmd/dep/main.go +++ b/cmd/dep/main.go @@ -25,7 +25,7 @@ type command interface { LongHelp() string // "Foo the first bar meeting the following conditions..." Register(*flag.FlagSet) // command-specific flags Hidden() bool // indicates whether the command should be hidden from help output - Run(*dep.Ctx, *Loggers, []string) error + Run(*dep.Ctx, []string) error } func main() { @@ -141,14 +141,14 @@ func (c *Config) Run() (exitCode int) { return } - loggers := &Loggers{ + loggers := &dep.Loggers{ Out: log.New(c.Stdout, "", 0), Err: errLogger, Verbose: *verbose, } // Set up the dep context. - ctx, err := dep.NewContext(c.WorkingDir, c.Env) + ctx, err := dep.NewContext(c.WorkingDir, c.Env, loggers) if err != nil { loggers.Err.Println(err) exitCode = 1 @@ -156,7 +156,7 @@ func (c *Config) Run() (exitCode int) { } // Run the command with the post-flag-processing args. - if err := cmd.Run(ctx, loggers, fs.Args()); err != nil { + if err := cmd.Run(ctx, fs.Args()); err != nil { errLogger.Printf("%v\n", err) exitCode = 1 return diff --git a/cmd/dep/prune.go b/cmd/dep/prune.go index 89aa899160..cd59655160 100644 --- a/cmd/dep/prune.go +++ b/cmd/dep/prune.go @@ -35,7 +35,7 @@ func (cmd *pruneCommand) Hidden() bool { return false } func (cmd *pruneCommand) Register(fs *flag.FlagSet) { } -func (cmd *pruneCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) error { +func (cmd *pruneCommand) Run(ctx *dep.Ctx, args []string) error { p, err := ctx.LoadProject("") if err != nil { return err @@ -59,8 +59,8 @@ func (cmd *pruneCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) erro params := p.MakeParams() params.RootPackageTree = ptree - if loggers.Verbose { - params.TraceLogger = loggers.Err + if ctx.Loggers.Verbose { + params.TraceLogger = ctx.Loggers.Err } s, err := gps.Prepare(params, sm) diff --git a/cmd/dep/remove.go b/cmd/dep/remove.go index 6dddb15dba..e53e68e7da 100644 --- a/cmd/dep/remove.go +++ b/cmd/dep/remove.go @@ -41,7 +41,7 @@ type removeCommand struct { keepSource bool } -func (cmd *removeCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) error { +func (cmd *removeCommand) Run(ctx *dep.Ctx, args []string) error { p, err := ctx.LoadProject("") if err != nil { return err @@ -89,7 +89,7 @@ func (cmd *removeCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) err // not being able to detect the root for an import path that's // actually in the import list is a deeper problem. However, // it's not our direct concern here, so we just warn. - loggers.Err.Printf("dep: could not infer root for %q\n", pr) + ctx.Loggers.Err.Printf("dep: could not infer root for %q\n", pr) continue } otherroots[pr] = true @@ -104,7 +104,7 @@ func (cmd *removeCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) err } if len(rm) == 0 { - loggers.Err.Println("dep: nothing to do") + ctx.Loggers.Err.Println("dep: nothing to do") return nil } } else { @@ -162,8 +162,8 @@ func (cmd *removeCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) err params := p.MakeParams() params.RootPackageTree = pkgT - if loggers.Verbose { - params.TraceLogger = loggers.Err + if ctx.Loggers.Verbose { + params.TraceLogger = ctx.Loggers.Err } s, err := gps.Prepare(params, sm) if err != nil { diff --git a/cmd/dep/status.go b/cmd/dep/status.go index d9f7306db0..57d5440a50 100644 --- a/cmd/dep/status.go +++ b/cmd/dep/status.go @@ -180,7 +180,7 @@ func (out *dotOutput) MissingHeader() {} func (out *dotOutput) MissingLine(ms *MissingStatus) {} func (out *dotOutput) MissingFooter() {} -func (cmd *statusCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) error { +func (cmd *statusCommand) Run(ctx *dep.Ctx, args []string) error { p, err := ctx.LoadProject("") if err != nil { return err @@ -214,11 +214,11 @@ func (cmd *statusCommand) Run(ctx *dep.Ctx, loggers *Loggers, args []string) err } } - if err := runStatusAll(loggers, out, p, sm); err != nil { + if err := runStatusAll(ctx.Loggers, out, p, sm); err != nil { return err } - loggers.Out.Print(buf.String()) + ctx.Loggers.Out.Print(buf.String()) return nil } @@ -239,7 +239,7 @@ type MissingStatus struct { MissingPackages []string } -func runStatusAll(loggers *Loggers, out outputter, p *dep.Project, sm gps.SourceManager) error { +func runStatusAll(loggers *dep.Loggers, out outputter, p *dep.Project, sm gps.SourceManager) error { if p.Lock == nil { // TODO if we have no lock file, do...other stuff return nil diff --git a/context.go b/context.go index d8db34a751..c915bc9608 100644 --- a/context.go +++ b/context.go @@ -5,6 +5,7 @@ package dep import ( + "log" "os" "path/filepath" "runtime" @@ -21,12 +22,20 @@ type Ctx struct { GOPATH string // Selected Go path GOPATHS []string // Other Go paths WorkingDir string + *Loggers +} + +// Loggers holds standard loggers and a verbosity flag. +type Loggers struct { + Out, Err *log.Logger + // Whether verbose logging is enabled. + Verbose bool } // NewContext creates a struct with the project's GOPATH. It assumes // that of your "GOPATH"'s we want the one we are currently in. -func NewContext(wd string, env []string) (*Ctx, error) { - ctx := &Ctx{WorkingDir: wd} +func NewContext(wd string, env []string, loggers *Loggers) (*Ctx, error) { + ctx := &Ctx{WorkingDir: wd, Loggers: loggers} GOPATH := getEnv(env, "GOPATH") if GOPATH == "" { @@ -144,7 +153,11 @@ func (c *Ctx) LoadProject(path string) (*Project, error) { } defer mf.Close() - p.Manifest, err = readManifest(mf) + var warns []error + p.Manifest, warns, err = readManifest(mf) + for _, warn := range warns { + c.Loggers.Err.Printf("dep: WARNING: %v\n", warn) + } if err != nil { return nil, errors.Errorf("error while parsing %s: %s", mp, err) } diff --git a/context_test.go b/context_test.go index 347f2aeafa..951c233da4 100644 --- a/context_test.go +++ b/context_test.go @@ -5,6 +5,8 @@ package dep import ( + "io/ioutil" + "log" "os" "path/filepath" "runtime" @@ -16,6 +18,11 @@ import ( "github.com/golang/dep/internal/test" ) +var ( + discardLogger = log.New(ioutil.Discard, "", 0) + discardLoggers = &Loggers{Out: discardLogger, Err: discardLogger} +) + func TestNewContextNoGOPATH(t *testing.T) { h := test.NewHelper(t) defer h.Cleanup() @@ -28,7 +35,7 @@ func TestNewContextNoGOPATH(t *testing.T) { t.Fatal("failed to get work directory:", err) } - c, err := NewContext(wd, os.Environ()) + c, err := NewContext(wd, os.Environ(), nil) if err == nil { t.Fatal("error should not have been nil") } @@ -211,7 +218,7 @@ func TestLoadProject(t *testing.T) { t.Fatal("failed to get working directory", err) } - ctx := &Ctx{GOPATH: tg.Path("."), WorkingDir: wd} + ctx := &Ctx{GOPATH: tg.Path("."), WorkingDir: wd, Loggers: discardLoggers} proj, err := ctx.LoadProject(path) tg.Must(err) @@ -287,7 +294,7 @@ func TestLoadProjectManifestParseError(t *testing.T) { t.Fatal("failed to get working directory", err) } - ctx := &Ctx{GOPATH: tg.Path("."), WorkingDir: wd} + ctx := &Ctx{GOPATH: tg.Path("."), WorkingDir: wd, Loggers: discardLoggers} _, err = ctx.LoadProject("") if err == nil { @@ -313,7 +320,7 @@ func TestLoadProjectLockParseError(t *testing.T) { t.Fatal("failed to get working directory", err) } - ctx := &Ctx{GOPATH: tg.Path("."), WorkingDir: wd} + ctx := &Ctx{GOPATH: tg.Path("."), WorkingDir: wd, Loggers: discardLoggers} _, err = ctx.LoadProject("") if err == nil { diff --git a/manifest.go b/manifest.go index 118d700ecc..557659bdc4 100644 --- a/manifest.go +++ b/manifest.go @@ -92,30 +92,27 @@ func validateManifest(s string) ([]error, error) { return errs, nil } -func readManifest(r io.Reader) (*Manifest, error) { +// readManifest returns a Manifest read from r and a slice of validation warnings. +func readManifest(r io.Reader) (*Manifest, []error, error) { buf := &bytes.Buffer{} _, err := buf.ReadFrom(r) if err != nil { - return nil, errors.Wrap(err, "Unable to read byte stream") + return nil, nil, errors.Wrap(err, "Unable to read byte stream") } - // Validate manifest and log warnings - _, err = validateManifest(buf.String()) + warns, err := validateManifest(buf.String()) if err != nil { - return nil, errors.Wrap(err, "Manifest validation failed") + return nil, warns, errors.Wrap(err, "Manifest validation failed") } - //for _, e := range errs { - // FIXME(sdboyer) need to adapt this to use an injected *Loggers - //internal.Logf("WARNING: %v", e) - //} raw := rawManifest{} err = toml.Unmarshal(buf.Bytes(), &raw) if err != nil { - return nil, errors.Wrap(err, "Unable to parse the manifest as TOML") + return nil, warns, errors.Wrap(err, "Unable to parse the manifest as TOML") } - return fromRawManifest(raw) + m, err := fromRawManifest(raw) + return m, warns, err } func fromRawManifest(raw rawManifest) (*Manifest, error) { diff --git a/manifest_test.go b/manifest_test.go index 0c23fbe629..3eb87df105 100644 --- a/manifest_test.go +++ b/manifest_test.go @@ -20,7 +20,7 @@ func TestReadManifest(t *testing.T) { mf := h.GetTestFile("manifest/golden.toml") defer mf.Close() - got, err := readManifest(mf) + got, _, err := readManifest(mf) if err != nil { t.Fatalf("Should have read Manifest correctly, but got err %q", err) } @@ -112,7 +112,7 @@ func TestReadManifestErrors(t *testing.T) { for _, tst := range tests { mf := h.GetTestFile(tst.file) defer mf.Close() - _, err = readManifest(mf) + _, _, err = readManifest(mf) if err == nil { t.Errorf("Reading manifest with %s should have caused error, but did not", tst.name) } else if !strings.Contains(err.Error(), tst.name) { diff --git a/project_test.go b/project_test.go index 17fdd97617..d466309dc9 100644 --- a/project_test.go +++ b/project_test.go @@ -92,13 +92,13 @@ func TestSlashedGOPATH(t *testing.T) { env := os.Environ() h.Setenv("GOPATH", filepath.ToSlash(h.Path("."))) - _, err = NewContext(wd, env) + _, err = NewContext(wd, env, nil) if err != nil { t.Fatal(err) } h.Setenv("GOPATH", filepath.FromSlash(h.Path("."))) - _, err = NewContext(wd, env) + _, err = NewContext(wd, env, nil) if err != nil { t.Fatal(err) } diff --git a/test_project_context_test.go b/test_project_context_test.go index b8a08f3efc..3ae86ec320 100644 --- a/test_project_context_test.go +++ b/test_project_context_test.go @@ -37,7 +37,7 @@ func NewTestProjectContext(h *test.Helper, projectName string) *TestProjectConte // Set up a Source Manager var err error - pc.Context = &Ctx{GOPATH: pc.tempDir} + pc.Context = &Ctx{GOPATH: pc.tempDir, Loggers: discardLoggers} pc.SourceManager, err = pc.Context.SourceManager() h.Must(errors.Wrap(err, "Unable to create a SourceManager")) @@ -61,7 +61,11 @@ func (pc *TestProjectContext) Load() { if pc.h.Exist(mp) { mf := pc.h.GetFile(mp) defer mf.Close() - m, err = readManifest(mf) + var warns []error + m, warns, err = readManifest(mf) + for _, warn := range warns { + pc.Context.Loggers.Err.Printf("dep: WARNING: %v\n", warn) + } pc.h.Must(errors.Wrapf(err, "Unable to read manifest at %s", mp)) } var l *Lock