Skip to content

Fixed compiler output capture in JSON mode #2078

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Mar 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions internal/integrationtest/compile_3/compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/arduino/go-paths-helper"
"github.com/arduino/go-properties-orderedmap"
"github.com/stretchr/testify/require"
"go.bug.st/testifyjson/requirejson"
)

func TestRuntimeToolPropertiesGeneration(t *testing.T) {
Expand Down Expand Up @@ -96,3 +97,22 @@ func TestCompileBuildPathInsideSketch(t *testing.T) {
_, _, err = cli.Run("compile", "-b", "arduino:avr:mega", "--build-path", "build-mega")
require.NoError(t, err)
}

func TestCompilerErrOutput(t *testing.T) {
env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t)
defer env.CleanUp()

// Run update-index with our test index
_, _, err := cli.Run("core", "install", "arduino:[email protected]")
require.NoError(t, err)

// prepare sketch
sketch, err := paths.New("testdata", "blink_with_wrong_cpp").Abs()
require.NoError(t, err)

// Run compile and catch err stream
out, _, err := cli.Run("compile", "-b", "arduino:avr:uno", "--format", "json", sketch.String())
require.Error(t, err)
compilerErr := requirejson.Parse(t, out).Query(".compiler_err")
compilerErr.MustContain(`"error"`)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
void setup() {}
void loop() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
void wrong() {
2 changes: 1 addition & 1 deletion legacy/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func (s *Preprocess) Run(ctx *types.Context) error {
}

// Output arduino-preprocessed source
ctx.Stdout.Write([]byte(ctx.Source))
ctx.WriteStdout([]byte(ctx.Source))
return nil
}

Expand Down
10 changes: 9 additions & 1 deletion legacy/builder/builder_utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,15 @@ func compileFileWithRecipe(ctx *types.Context, sourcePath *paths.Path, source *p
ctx.CompilationDatabase.Add(source, command)
}
if !objIsUpToDate && !ctx.OnlyUpdateCompilationDatabase {
_, _, err = utils.ExecCommand(ctx, command, utils.ShowIfVerbose /* stdout */, utils.Show /* stderr */)
// Since this compile could be multithreaded, we first capture the command output
stdout, stderr, err := utils.ExecCommand(ctx, command, utils.Capture, utils.Capture)
// and transfer all at once at the end...
if ctx.Verbose {
ctx.WriteStdout(stdout)
}
ctx.WriteStderr(stderr)

// ...and then return the error
if err != nil {
return nil, errors.WithStack(err)
}
Expand Down
2 changes: 1 addition & 1 deletion legacy/builder/container_find_includes.go
Original file line number Diff line number Diff line change
Expand Up @@ -408,7 +408,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t
return errors.New(tr("Internal error in cache"))
}
}
ctx.Stderr.Write(preproc_stderr)
ctx.WriteStderr(preproc_stderr)
return errors.WithStack(preproc_err)
}

Expand Down
2 changes: 1 addition & 1 deletion legacy/builder/preprocess_sketch.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,6 @@ func (s *OutputCodeCompletions) Run(ctx *types.Context) error {
// we assume it is a json, let's make it compliant at least
ctx.CodeCompletions = "[]"
}
fmt.Fprintln(ctx.Stdout, ctx.CodeCompletions)
ctx.WriteStdout([]byte(ctx.CodeCompletions))
return nil
}
18 changes: 18 additions & 0 deletions legacy/builder/types/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,3 +249,21 @@ func (ctx *Context) Warn(msg string) {
}
ctx.stdLock.Unlock()
}

func (ctx *Context) WriteStdout(data []byte) (int, error) {
ctx.stdLock.Lock()
defer ctx.stdLock.Unlock()
if ctx.Stdout == nil {
return os.Stdout.Write(data)
}
return ctx.Stdout.Write(data)
}

func (ctx *Context) WriteStderr(data []byte) (int, error) {
ctx.stdLock.Lock()
defer ctx.stdLock.Unlock()
if ctx.Stderr == nil {
return os.Stderr.Write(data)
}
return ctx.Stderr.Write(data)
}
23 changes: 12 additions & 11 deletions legacy/builder/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,29 +184,30 @@ const (
)

func ExecCommand(ctx *types.Context, command *exec.Cmd, stdout int, stderr int) ([]byte, []byte, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got inspired by this refactoring, but do what you want with it. Since it took longer than expected it might not be that good

func getWriter(option int, isVerbose bool, candidate io.Writer) io.Writer {
	getCandidateOrStdout := func() io.Writer {
		if candidate != nil {
			return candidate
		}
		return os.Stdout
	}
	mapOptionToWriterFunc := map[int]func() io.Writer{
		Ignore: func() io.Writer {
			return io.Discard
		},
		Show: getCandidateOrStdout,
		ShowIfVerbose: func() io.Writer {
			if !isVerbose {
				return io.Discard
			}
			return getCandidateOrStdout()
		},
		Capture: func() io.Writer {
			return &bytes.Buffer{}
		},
	}
	return mapOptionToWriterFunc[option]()
}

func ExecCommand(ctx *types.Context, command *exec.Cmd, stdout int, stderr int) ([]byte, []byte, error) {
	stdoutWriter := getWriter(stdout, ctx.Verbose, ctx.Stdout)
	stderrWriter := getWriter(stderr, ctx.Verbose, ctx.Stderr)
	return ExecCommandWithWriters(ctx, command, stdoutWriter, stderrWriter)
}

func ExecCommandWithWriters(ctx *types.Context, command *exec.Cmd, stdout io.Writer, stderr io.Writer) ([]byte, []byte, error) {
	if ctx.Verbose {
		ctx.Info(PrintableCommand(command.Args))
	}
	command.Stdout = stdout
	command.Stderr = stderr

	err := command.Start()
	if err != nil {
		return nil, nil, errors.WithStack(err)
	}

	err = command.Wait()

	var outbytes, errbytes []byte
	if buf, ok := command.Stdout.(*bytes.Buffer); ok {
		outbytes = buf.Bytes()
	}
	if buf, ok := command.Stderr.(*bytes.Buffer); ok {
		errbytes = buf.Bytes()
	}

	return outbytes, errbytes, errors.WithStack(err)
}```

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something to keep in mind when this subroutine will be merged with executils and removed from legacy.

if ctx.Stdout == nil {
ctx.Stdout = os.Stdout
}
if ctx.Stderr == nil {
ctx.Stderr = os.Stderr
}

if ctx.Verbose {
ctx.Info(PrintableCommand(command.Args))
}

if stdout == Capture {
buffer := &bytes.Buffer{}
command.Stdout = buffer
} else if stdout == Show || stdout == ShowIfVerbose && ctx.Verbose {
command.Stdout = ctx.Stdout
} else if stdout == Show || (stdout == ShowIfVerbose && ctx.Verbose) {
if ctx.Stdout != nil {
command.Stdout = ctx.Stdout
} else {
command.Stdout = os.Stdout
}
}

if stderr == Capture {
buffer := &bytes.Buffer{}
command.Stderr = buffer
} else if stderr == Show || stderr == ShowIfVerbose && ctx.Verbose {
command.Stderr = ctx.Stderr
} else if stderr == Show || (stderr == ShowIfVerbose && ctx.Verbose) {
if ctx.Stderr != nil {
command.Stderr = ctx.Stderr
} else {
command.Stderr = os.Stderr
}
}

err := command.Start()
Expand Down