From 01ea97042ebeb996ee8af30f268f1b4c1a6ca30b Mon Sep 17 00:00:00 2001 From: Jamie Crisman Date: Thu, 30 Nov 2017 18:55:12 -0600 Subject: [PATCH 1/4] Outputter interface methods now return errors. --- cmd/dep/status.go | 129 +++++++++++++++++++++++++++++----------------- 1 file changed, 81 insertions(+), 48 deletions(-) diff --git a/cmd/dep/status.go b/cmd/dep/status.go index 9cae286bea..9bfe183e7a 100644 --- a/cmd/dep/status.go +++ b/cmd/dep/status.go @@ -85,26 +85,28 @@ type statusCommand struct { } type outputter interface { - BasicHeader() - BasicLine(*BasicStatus) - BasicFooter() - MissingHeader() - MissingLine(*MissingStatus) - MissingFooter() + BasicHeader() error + BasicLine(*BasicStatus) error + BasicFooter() error + MissingHeader() error + MissingLine(*MissingStatus) error + MissingFooter() error } type tableOutput struct{ w *tabwriter.Writer } -func (out *tableOutput) BasicHeader() { - fmt.Fprintf(out.w, "PROJECT\tCONSTRAINT\tVERSION\tREVISION\tLATEST\tPKGS USED\n") +func (out *tableOutput) BasicHeader() error { + _, err := fmt.Fprintf(out.w, "PROJECT\tCONSTRAINT\tVERSION\tREVISION\tLATEST\tPKGS USED\n") + return err } -func (out *tableOutput) BasicFooter() { - out.w.Flush() +func (out *tableOutput) BasicFooter() error { + err := out.w.Flush() + return err } -func (out *tableOutput) BasicLine(bs *BasicStatus) { - fmt.Fprintf(out.w, +func (out *tableOutput) BasicLine(bs *BasicStatus) error { + _, err := fmt.Fprintf(out.w, "%s\t%s\t%s\t%s\t%s\t%d\t\n", bs.ProjectRoot, bs.getConsolidatedConstraint(), @@ -113,22 +115,26 @@ func (out *tableOutput) BasicLine(bs *BasicStatus) { bs.getConsolidatedLatest(shortRev), bs.PackageCount, ) + return err } -func (out *tableOutput) MissingHeader() { - fmt.Fprintln(out.w, "PROJECT\tMISSING PACKAGES") +func (out *tableOutput) MissingHeader() error { + _, err := fmt.Fprintln(out.w, "PROJECT\tMISSING PACKAGES") + return err } -func (out *tableOutput) MissingLine(ms *MissingStatus) { - fmt.Fprintf(out.w, +func (out *tableOutput) MissingLine(ms *MissingStatus) error { + _, err := fmt.Fprintf(out.w, "%s\t%s\t\n", ms.ProjectRoot, ms.MissingPackages, ) + return err } -func (out *tableOutput) MissingFooter() { - out.w.Flush() +func (out *tableOutput) MissingFooter() error { + err := out.w.Flush() + return err } type jsonOutput struct { @@ -137,28 +143,34 @@ type jsonOutput struct { missing []*MissingStatus } -func (out *jsonOutput) BasicHeader() { +func (out *jsonOutput) BasicHeader() error { out.basic = []*rawStatus{} + return nil } -func (out *jsonOutput) BasicFooter() { - json.NewEncoder(out.w).Encode(out.basic) +func (out *jsonOutput) BasicFooter() error { + err := json.NewEncoder(out.w).Encode(out.basic) + return err } -func (out *jsonOutput) BasicLine(bs *BasicStatus) { +func (out *jsonOutput) BasicLine(bs *BasicStatus) error { out.basic = append(out.basic, bs.marshalJSON()) + return nil } -func (out *jsonOutput) MissingHeader() { +func (out *jsonOutput) MissingHeader() error { out.missing = []*MissingStatus{} + return nil } -func (out *jsonOutput) MissingLine(ms *MissingStatus) { +func (out *jsonOutput) MissingLine(ms *MissingStatus) error { out.missing = append(out.missing, ms) + return nil } -func (out *jsonOutput) MissingFooter() { - json.NewEncoder(out.w).Encode(out.missing) +func (out *jsonOutput) MissingFooter() error { + err := json.NewEncoder(out.w).Encode(out.missing) + return err } type dotOutput struct { @@ -168,46 +180,52 @@ type dotOutput struct { p *dep.Project } -func (out *dotOutput) BasicHeader() { +func (out *dotOutput) BasicHeader() error { out.g = new(graphviz).New() - ptree, _ := out.p.ParseRootPackageTree() + ptree, err := out.p.ParseRootPackageTree() // TODO(sdboyer) should be true, true, false, out.p.Manifest.IgnoredPackages() prm, _ := ptree.ToReachMap(true, false, false, nil) out.g.createNode(string(out.p.ImportRoot), "", prm.FlattenFn(paths.IsStandardImportPath)) + + return err } -func (out *dotOutput) BasicFooter() { +func (out *dotOutput) BasicFooter() error { gvo := out.g.output() - fmt.Fprintf(out.w, gvo.String()) + _, err := fmt.Fprintf(out.w, gvo.String()) + return err } -func (out *dotOutput) BasicLine(bs *BasicStatus) { +func (out *dotOutput) BasicLine(bs *BasicStatus) error { out.g.createNode(bs.ProjectRoot, bs.getConsolidatedVersion(), bs.Children) + return nil } -func (out *dotOutput) MissingHeader() {} -func (out *dotOutput) MissingLine(ms *MissingStatus) {} -func (out *dotOutput) MissingFooter() {} +func (out *dotOutput) MissingHeader() error { return nil } +func (out *dotOutput) MissingLine(ms *MissingStatus) error { return nil } +func (out *dotOutput) MissingFooter() error { return nil } type templateOutput struct { w io.Writer tmpl *template.Template } -func (out *templateOutput) BasicHeader() {} -func (out *templateOutput) BasicFooter() {} +func (out *templateOutput) BasicHeader() error { return nil } +func (out *templateOutput) BasicFooter() error { return nil } -func (out *templateOutput) BasicLine(bs *BasicStatus) { - out.tmpl.Execute(out.w, bs) +func (out *templateOutput) BasicLine(bs *BasicStatus) error { + err := out.tmpl.Execute(out.w, bs) + return err } -func (out *templateOutput) MissingHeader() {} -func (out *templateOutput) MissingFooter() {} +func (out *templateOutput) MissingHeader() error { return nil } +func (out *templateOutput) MissingFooter() error { return nil } -func (out *templateOutput) MissingLine(ms *MissingStatus) { - out.tmpl.Execute(out.w, ms) +func (out *templateOutput) MissingLine(ms *MissingStatus) error { + err := out.tmpl.Execute(out.w, ms) + return err } func (cmd *statusCommand) Run(ctx *dep.Ctx, args []string) error { @@ -432,7 +450,10 @@ func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceMana // complete picture of all deps. That eliminates the need for at least // some checks. - out.BasicHeader() + err := out.BasicHeader() + if err != nil { + return false, 0, err + } logger.Println("Checking upstream projects:") @@ -585,10 +606,13 @@ func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceMana // Use the collected BasicStatus in outputter. for _, proj := range slp { - out.BasicLine(bsMap[string(proj.Ident().ProjectRoot)]) + err := out.BasicLine(bsMap[string(proj.Ident().ProjectRoot)]) + if err != nil { + return false, 0, err + } } - out.BasicFooter() + err = out.BasicFooter() return false, errCount, err } @@ -634,7 +658,10 @@ func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceMana return false, 0, errors.New("address issues with undeducible import paths to get more status information") } - out.MissingHeader() + err = out.MissingHeader() + if err != nil { + return false, 0, err + } outer: for root, pkgs := range roots { @@ -647,9 +674,15 @@ outer: } hasMissingPkgs = true - out.MissingLine(&MissingStatus{ProjectRoot: string(root), MissingPackages: pkgs}) + err := out.MissingLine(&MissingStatus{ProjectRoot: string(root), MissingPackages: pkgs}) + if err != nil { + return false, 0, err + } + } + err = out.MissingFooter() + if err != nil { + return false, 0, err } - out.MissingFooter() // We are here because of an input-digest mismatch. Return error. return hasMissingPkgs, 0, errInputDigestMismatch From 589f4b59faf272524aed4236f85e0747538d836d Mon Sep 17 00:00:00 2001 From: Jamie Crisman Date: Thu, 30 Nov 2017 20:50:22 -0600 Subject: [PATCH 2/4] Remove unnecessary variables. --- cmd/dep/status.go | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/cmd/dep/status.go b/cmd/dep/status.go index 9bfe183e7a..b91d5d58e9 100644 --- a/cmd/dep/status.go +++ b/cmd/dep/status.go @@ -101,8 +101,7 @@ func (out *tableOutput) BasicHeader() error { } func (out *tableOutput) BasicFooter() error { - err := out.w.Flush() - return err + return out.w.Flush() } func (out *tableOutput) BasicLine(bs *BasicStatus) error { @@ -133,8 +132,7 @@ func (out *tableOutput) MissingLine(ms *MissingStatus) error { } func (out *tableOutput) MissingFooter() error { - err := out.w.Flush() - return err + return out.w.Flush() } type jsonOutput struct { @@ -149,8 +147,7 @@ func (out *jsonOutput) BasicHeader() error { } func (out *jsonOutput) BasicFooter() error { - err := json.NewEncoder(out.w).Encode(out.basic) - return err + return json.NewEncoder(out.w).Encode(out.basic) } func (out *jsonOutput) BasicLine(bs *BasicStatus) error { @@ -169,8 +166,7 @@ func (out *jsonOutput) MissingLine(ms *MissingStatus) error { } func (out *jsonOutput) MissingFooter() error { - err := json.NewEncoder(out.w).Encode(out.missing) - return err + return json.NewEncoder(out.w).Encode(out.missing) } type dotOutput struct { @@ -216,16 +212,14 @@ func (out *templateOutput) BasicHeader() error { return nil } func (out *templateOutput) BasicFooter() error { return nil } func (out *templateOutput) BasicLine(bs *BasicStatus) error { - err := out.tmpl.Execute(out.w, bs) - return err + return out.tmpl.Execute(out.w, bs) } func (out *templateOutput) MissingHeader() error { return nil } func (out *templateOutput) MissingFooter() error { return nil } func (out *templateOutput) MissingLine(ms *MissingStatus) error { - err := out.tmpl.Execute(out.w, ms) - return err + return out.tmpl.Execute(out.w, ms) } func (cmd *statusCommand) Run(ctx *dep.Ctx, args []string) error { From 4938def90bbd468ebf35af61e9dfefa0ed4822f7 Mon Sep 17 00:00:00 2001 From: Jamie Crisman Date: Mon, 4 Dec 2017 11:30:15 -0600 Subject: [PATCH 3/4] Make error return consistently --- CHANGELOG.md | 1 + cmd/dep/status.go | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 10f54c4569..dd87e3e1b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ IMPROVEMENTS: * Make the gps package importable ([#1349](https://github.com/golang/dep/pull/1349)). * Improve file copy performance by not forcing a file sync (PR #1408). * Skip empty constraints during import ([#1414](https://github.com/golang/dep/pull/1349)) +* Handle errors when writing output ([#1420](https://github.com/golang/dep/pull/1420)) # v0.3.2 diff --git a/cmd/dep/status.go b/cmd/dep/status.go index b91d5d58e9..8fa348413c 100644 --- a/cmd/dep/status.go +++ b/cmd/dep/status.go @@ -606,7 +606,10 @@ func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceMana } } - err = out.BasicFooter() + footerErr := out.BasicFooter() + if footerErr != nil { + return false, 0, footerErr + } return false, errCount, err } From fcdb2edaf19280a5673e055a4b6225eca78b6f47 Mon Sep 17 00:00:00 2001 From: Jamie Crisman Date: Thu, 7 Dec 2017 11:40:41 -0600 Subject: [PATCH 4/4] More compact error handling --- CHANGELOG.md | 2 +- cmd/dep/status.go | 15 +++++---------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dd87e3e1b8..163c21cab7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ IMPROVEMENTS: * Make the gps package importable ([#1349](https://github.com/golang/dep/pull/1349)). * Improve file copy performance by not forcing a file sync (PR #1408). * Skip empty constraints during import ([#1414](https://github.com/golang/dep/pull/1349)) -* Handle errors when writing output ([#1420](https://github.com/golang/dep/pull/1420)) +* Handle errors when writing status output ([#1420](https://github.com/golang/dep/pull/1420)) # v0.3.2 diff --git a/cmd/dep/status.go b/cmd/dep/status.go index 8fa348413c..588154c0f3 100644 --- a/cmd/dep/status.go +++ b/cmd/dep/status.go @@ -444,8 +444,7 @@ func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceMana // complete picture of all deps. That eliminates the need for at least // some checks. - err := out.BasicHeader() - if err != nil { + if err := out.BasicHeader(); err != nil { return false, 0, err } @@ -600,14 +599,12 @@ func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceMana // Use the collected BasicStatus in outputter. for _, proj := range slp { - err := out.BasicLine(bsMap[string(proj.Ident().ProjectRoot)]) - if err != nil { + if err := out.BasicLine(bsMap[string(proj.Ident().ProjectRoot)]); err != nil { return false, 0, err } } - footerErr := out.BasicFooter() - if footerErr != nil { + if footerErr := out.BasicFooter(); footerErr != nil { return false, 0, footerErr } @@ -655,8 +652,7 @@ func runStatusAll(ctx *dep.Ctx, out outputter, p *dep.Project, sm gps.SourceMana return false, 0, errors.New("address issues with undeducible import paths to get more status information") } - err = out.MissingHeader() - if err != nil { + if err = out.MissingHeader(); err != nil { return false, 0, err } @@ -676,8 +672,7 @@ outer: return false, 0, err } } - err = out.MissingFooter() - if err != nil { + if err = out.MissingFooter(); err != nil { return false, 0, err }