From e36c77aaf3ed88be1583868288958bb272534fa0 Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Wed, 6 Jul 2022 10:43:32 -0300 Subject: [PATCH] Fix bug with STDOUT and STDERR in the "group" output mode Took the oportunity to refactor a bit how we handle closing of the streams. Fixes #779 --- CHANGELOG.md | 3 +++ internal/output/group.go | 14 +++++++------- internal/output/interleaved.go | 4 ++-- internal/output/output.go | 4 +++- internal/output/output_test.go | 30 ++++++++++++++++++------------ internal/output/prefixed.go | 7 ++++--- task.go | 15 +++------------ 7 files changed, 40 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4976cc08..9da36ec2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## Unreleased +- Fixed bug when using the `output: group` mode where STDOUT and STDERR were + being print in separated blocks instead of in the right order + ([#779](https://github.com/go-task/task/issues/779)). - Starting on this release, ARM architecture binaries are been released to Snap as well ([#795](https://github.com/go-task/task/issues/795)). diff --git a/internal/output/group.go b/internal/output/group.go index bddcdaeb..290fdab9 100644 --- a/internal/output/group.go +++ b/internal/output/group.go @@ -5,24 +5,24 @@ import ( "io" ) -type Group struct{ +type Group struct { Begin, End string } -func (g Group) WrapWriter(w io.Writer, _ string, tmpl Templater) io.Writer { - gw := &groupWriter{writer: w} +func (g Group) WrapWriter(stdOut, _ io.Writer, _ string, tmpl Templater) (io.Writer, io.Writer, CloseFunc) { + gw := &groupWriter{writer: stdOut} if g.Begin != "" { gw.begin = tmpl.Replace(g.Begin) + "\n" } if g.End != "" { gw.end = tmpl.Replace(g.End) + "\n" } - return gw + return gw, gw, func() error { return gw.close() } } type groupWriter struct { - writer io.Writer - buff bytes.Buffer + writer io.Writer + buff bytes.Buffer begin, end string } @@ -30,7 +30,7 @@ func (gw *groupWriter) Write(p []byte) (int, error) { return gw.buff.Write(p) } -func (gw *groupWriter) Close() error { +func (gw *groupWriter) close() error { if gw.buff.Len() == 0 { // don't print begin/end messages if there's no buffered entries return nil diff --git a/internal/output/interleaved.go b/internal/output/interleaved.go index b2e3b05b..ceeb789e 100644 --- a/internal/output/interleaved.go +++ b/internal/output/interleaved.go @@ -6,6 +6,6 @@ import ( type Interleaved struct{} -func (Interleaved) WrapWriter(w io.Writer, _ string, _ Templater) io.Writer { - return w +func (Interleaved) WrapWriter(stdOut, stdErr io.Writer, _ string, _ Templater) (io.Writer, io.Writer, CloseFunc) { + return stdOut, stdErr, func() error { return nil } } diff --git a/internal/output/output.go b/internal/output/output.go index 925c840a..206b8c32 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -15,9 +15,11 @@ type Templater interface { } type Output interface { - WrapWriter(w io.Writer, prefix string, tmpl Templater) io.Writer + WrapWriter(stdOut, stdErr io.Writer, prefix string, tmpl Templater) (io.Writer, io.Writer, CloseFunc) } +type CloseFunc func() error + // Build the Output for the requested taskfile.Output. func BuildFor(o *taskfile.Output) (Output, error) { switch o.Name { diff --git a/internal/output/output_test.go b/internal/output/output_test.go index fbab330e..7a7d5bf7 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -16,7 +16,7 @@ import ( func TestInterleaved(t *testing.T) { var b bytes.Buffer var o output.Output = output.Interleaved{} - var w = o.WrapWriter(&b, "", nil) + var w, _, _ = o.WrapWriter(&b, io.Discard, "", nil) fmt.Fprintln(w, "foo\nbar") assert.Equal(t, "foo\nbar\n", b.String()) @@ -27,14 +27,19 @@ func TestInterleaved(t *testing.T) { func TestGroup(t *testing.T) { var b bytes.Buffer var o output.Output = output.Group{} - var w = o.WrapWriter(&b, "", nil).(io.WriteCloser) + var stdOut, stdErr, cleanup = o.WrapWriter(&b, io.Discard, "", nil) - fmt.Fprintln(w, "foo\nbar") + fmt.Fprintln(stdOut, "out\nout") assert.Equal(t, "", b.String()) - fmt.Fprintln(w, "baz") + fmt.Fprintln(stdErr, "err\nerr") assert.Equal(t, "", b.String()) - assert.NoError(t, w.Close()) - assert.Equal(t, "foo\nbar\nbaz\n", b.String()) + fmt.Fprintln(stdOut, "out") + assert.Equal(t, "", b.String()) + fmt.Fprintln(stdErr, "err") + assert.Equal(t, "", b.String()) + + assert.NoError(t, cleanup()) + assert.Equal(t, "out\nout\nerr\nerr\nout\nerr\n", b.String()) } func TestGroupWithBeginEnd(t *testing.T) { @@ -53,19 +58,19 @@ func TestGroupWithBeginEnd(t *testing.T) { } t.Run("simple", func(t *testing.T) { var b bytes.Buffer - var w = o.WrapWriter(&b, "", &tmpl).(io.WriteCloser) + var w, _, cleanup = o.WrapWriter(&b, io.Discard, "", &tmpl) fmt.Fprintln(w, "foo\nbar") assert.Equal(t, "", b.String()) fmt.Fprintln(w, "baz") assert.Equal(t, "", b.String()) - assert.NoError(t, w.Close()) + assert.NoError(t, cleanup()) assert.Equal(t, "::group::example-value\nfoo\nbar\nbaz\n::endgroup::\n", b.String()) }) t.Run("no output", func(t *testing.T) { var b bytes.Buffer - var w = o.WrapWriter(&b, "", &tmpl).(io.WriteCloser) - assert.NoError(t, w.Close()) + var _, _, cleanup = o.WrapWriter(&b, io.Discard, "", &tmpl) + assert.NoError(t, cleanup()) assert.Equal(t, "", b.String()) }) } @@ -73,7 +78,7 @@ func TestGroupWithBeginEnd(t *testing.T) { func TestPrefixed(t *testing.T) { var b bytes.Buffer var o output.Output = output.Prefixed{} - var w = o.WrapWriter(&b, "prefix", nil).(io.WriteCloser) + var w, _, cleanup = o.WrapWriter(&b, io.Discard, "prefix", nil) t.Run("simple use cases", func(t *testing.T) { b.Reset() @@ -82,6 +87,7 @@ func TestPrefixed(t *testing.T) { assert.Equal(t, "[prefix] foo\n[prefix] bar\n", b.String()) fmt.Fprintln(w, "baz") assert.Equal(t, "[prefix] foo\n[prefix] bar\n[prefix] baz\n", b.String()) + assert.NoError(t, cleanup()) }) t.Run("multiple writes for a single line", func(t *testing.T) { @@ -92,7 +98,7 @@ func TestPrefixed(t *testing.T) { assert.Equal(t, "", b.String()) } - assert.NoError(t, w.Close()) + assert.NoError(t, cleanup()) assert.Equal(t, "[prefix] Test!\n", b.String()) }) } diff --git a/internal/output/prefixed.go b/internal/output/prefixed.go index f7fb7c71..badfef3f 100644 --- a/internal/output/prefixed.go +++ b/internal/output/prefixed.go @@ -9,8 +9,9 @@ import ( type Prefixed struct{} -func (Prefixed) WrapWriter(w io.Writer, prefix string, _ Templater) io.Writer { - return &prefixWriter{writer: w, prefix: prefix} +func (Prefixed) WrapWriter(stdOut, _ io.Writer, prefix string, _ Templater) (io.Writer, io.Writer, CloseFunc) { + pw := &prefixWriter{writer: stdOut, prefix: prefix} + return pw, pw, func() error { return pw.close() } } type prefixWriter struct { @@ -28,7 +29,7 @@ func (pw *prefixWriter) Write(p []byte) (int, error) { return n, pw.writeOutputLines(false) } -func (pw *prefixWriter) Close() error { +func (pw *prefixWriter) close() error { return pw.writeOutputLines(true) } diff --git a/task.go b/task.go index 6714d8e3..60f9cb3d 100644 --- a/task.go +++ b/task.go @@ -449,19 +449,10 @@ func (e *Executor) runCommand(ctx context.Context, t *taskfile.Task, call taskfi if err != nil { return fmt.Errorf("task: failed to get variables: %w", err) } - stdOut := outputWrapper.WrapWriter(e.Stdout, t.Prefix, outputTemplater) - stdErr := outputWrapper.WrapWriter(e.Stderr, t.Prefix, outputTemplater) - + stdOut, stdErr, close := outputWrapper.WrapWriter(e.Stdout, e.Stderr, t.Prefix, outputTemplater) defer func() { - if _, ok := stdOut.(*os.File); !ok { - if closer, ok := stdOut.(io.Closer); ok { - closer.Close() - } - } - if _, ok := stdErr.(*os.File); !ok { - if closer, ok := stdErr.(io.Closer); ok { - closer.Close() - } + if err := close(); err != nil { + e.Logger.Errf(logger.Red, "task: unable to close writter: %v", err) } }()