From fe2044a94b6ae69d5d426acedbebeac3ba4a9402 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 10 Apr 2025 17:47:30 +0200 Subject: [PATCH 1/5] Added integration test --- internal/integrationtest/arduino-cli.go | 19 ++++++-- .../compile_5/recusive_include_test.go | 44 +++++++++++++++++++ .../SketchWithRecursiveIncludes.ino | 1 + .../testdata/SketchWithRecursiveIncludes/a.h | 2 + 4 files changed, 62 insertions(+), 4 deletions(-) create mode 100644 internal/integrationtest/compile_5/recusive_include_test.go create mode 100644 internal/integrationtest/compile_5/testdata/SketchWithRecursiveIncludes/SketchWithRecursiveIncludes.ino create mode 100644 internal/integrationtest/compile_5/testdata/SketchWithRecursiveIncludes/a.h diff --git a/internal/integrationtest/arduino-cli.go b/internal/integrationtest/arduino-cli.go index f035e792f55..5cc9c898844 100644 --- a/internal/integrationtest/arduino-cli.go +++ b/internal/integrationtest/arduino-cli.go @@ -190,6 +190,12 @@ func (cli *ArduinoCLI) Run(args ...string) ([]byte, []byte, error) { return cli.RunWithCustomEnv(cli.cliEnvVars, args...) } +// RunWithContext executes the given arduino-cli command with the given context and returns the output. +// If the context is canceled, the command is killed. +func (cli *ArduinoCLI) RunWithContext(ctx context.Context, args ...string) ([]byte, []byte, error) { + return cli.RunWithCustomEnvContext(ctx, cli.cliEnvVars, args...) +} + // GetDefaultEnv returns a copy of the default execution env used with the Run method. func (cli *ArduinoCLI) GetDefaultEnv() map[string]string { res := map[string]string{} @@ -324,8 +330,13 @@ func (cli *ArduinoCLI) InstallMockedAvrdude(t *testing.T) { // RunWithCustomEnv executes the given arduino-cli command with the given custom env and returns the output. func (cli *ArduinoCLI) RunWithCustomEnv(env map[string]string, args ...string) ([]byte, []byte, error) { + return cli.RunWithCustomEnvContext(context.Background(), env, args...) +} + +// RunWithCustomEnv executes the given arduino-cli command with the given custom env and returns the output. +func (cli *ArduinoCLI) RunWithCustomEnvContext(ctx context.Context, env map[string]string, args ...string) ([]byte, []byte, error) { var stdoutBuf, stderrBuf bytes.Buffer - err := cli.run(&stdoutBuf, &stderrBuf, nil, env, args...) + err := cli.run(ctx, &stdoutBuf, &stderrBuf, nil, env, args...) errBuf := stderrBuf.Bytes() cli.t.NotContains(string(errBuf), "panic: runtime error:", "arduino-cli panicked") @@ -336,7 +347,7 @@ func (cli *ArduinoCLI) RunWithCustomEnv(env map[string]string, args ...string) ( // RunWithCustomInput executes the given arduino-cli command pushing the given input stream and returns the output. func (cli *ArduinoCLI) RunWithCustomInput(in io.Reader, args ...string) ([]byte, []byte, error) { var stdoutBuf, stderrBuf bytes.Buffer - err := cli.run(&stdoutBuf, &stderrBuf, in, cli.cliEnvVars, args...) + err := cli.run(context.Background(), &stdoutBuf, &stderrBuf, in, cli.cliEnvVars, args...) errBuf := stderrBuf.Bytes() cli.t.NotContains(string(errBuf), "panic: runtime error:", "arduino-cli panicked") @@ -344,7 +355,7 @@ func (cli *ArduinoCLI) RunWithCustomInput(in io.Reader, args ...string) ([]byte, return stdoutBuf.Bytes(), errBuf, err } -func (cli *ArduinoCLI) run(stdoutBuff, stderrBuff io.Writer, stdinBuff io.Reader, env map[string]string, args ...string) error { +func (cli *ArduinoCLI) run(ctx context.Context, stdoutBuff, stderrBuff io.Writer, stdinBuff io.Reader, env map[string]string, args ...string) error { if cli.cliConfigPath != nil { args = append([]string{"--config-file", cli.cliConfigPath.String()}, args...) } @@ -402,8 +413,8 @@ func (cli *ArduinoCLI) run(stdoutBuff, stderrBuff io.Writer, stdinBuff io.Reader } }() } + cliErr := cliProc.WaitWithinContext(ctx) wg.Wait() - cliErr := cliProc.Wait() fmt.Fprintln(terminalOut, color.HiBlackString("<<< Run completed (err = %v)", cliErr)) return cliErr diff --git a/internal/integrationtest/compile_5/recusive_include_test.go b/internal/integrationtest/compile_5/recusive_include_test.go new file mode 100644 index 00000000000..59a3c421c7f --- /dev/null +++ b/internal/integrationtest/compile_5/recusive_include_test.go @@ -0,0 +1,44 @@ +// This file is part of arduino-cli. +// +// Copyright 2025 ARDUINO SA (http://www.arduino.cc/) +// +// This software is released under the GNU General Public License version 3, +// which covers the main part of arduino-cli. +// The terms of this license can be found at: +// https://www.gnu.org/licenses/gpl-3.0.en.html +// +// You can be released from the requirements of the above licenses by purchasing +// a commercial license. Buying such a license is mandatory if you want to +// modify or otherwise use the software for commercial activities involving the +// Arduino software without disclosing the source code of your own applications. +// To purchase a commercial license, send an email to license@arduino.cc. + +package compile_test + +import ( + "context" + "testing" + "time" + + "github.com/arduino/arduino-cli/internal/integrationtest" + "github.com/arduino/go-paths-helper" + "github.com/stretchr/testify/require" +) + +func TestCompileWithInfiniteMultipleIncludeRecursion(t *testing.T) { + env, cli := integrationtest.CreateArduinoCLIWithEnvironment(t) + t.Cleanup(env.CleanUp) + + // Install Arduino AVR Boards + _, _, err := cli.Run("core", "install", "arduino:avr@1.8.6") + require.NoError(t, err) + + sketch, err := paths.New("testdata", "SketchWithRecursiveIncludes").Abs() + require.NoError(t, err) + + // Time-limited test to prevent OOM + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + t.Cleanup(cancel) + _, _, _ = cli.RunWithContext(ctx, "compile", "-b", "arduino:avr:uno", sketch.String()) + require.NotErrorIs(t, ctx.Err(), context.DeadlineExceeded, "compilation should not hang") +} diff --git a/internal/integrationtest/compile_5/testdata/SketchWithRecursiveIncludes/SketchWithRecursiveIncludes.ino b/internal/integrationtest/compile_5/testdata/SketchWithRecursiveIncludes/SketchWithRecursiveIncludes.ino new file mode 100644 index 00000000000..2243de1baf9 --- /dev/null +++ b/internal/integrationtest/compile_5/testdata/SketchWithRecursiveIncludes/SketchWithRecursiveIncludes.ino @@ -0,0 +1 @@ +#include "a.h" diff --git a/internal/integrationtest/compile_5/testdata/SketchWithRecursiveIncludes/a.h b/internal/integrationtest/compile_5/testdata/SketchWithRecursiveIncludes/a.h new file mode 100644 index 00000000000..7c626e5beb0 --- /dev/null +++ b/internal/integrationtest/compile_5/testdata/SketchWithRecursiveIncludes/a.h @@ -0,0 +1,2 @@ +#include "a.h" +#include "a.h" From 4f8f9dfa3486c6a7597b36318c77c4c76cfcc908 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 10 Apr 2025 17:48:12 +0200 Subject: [PATCH 2/5] Fixed linter warning --- internal/integrationtest/arduino-cli.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/integrationtest/arduino-cli.go b/internal/integrationtest/arduino-cli.go index 5cc9c898844..4b2b4850450 100644 --- a/internal/integrationtest/arduino-cli.go +++ b/internal/integrationtest/arduino-cli.go @@ -22,6 +22,7 @@ import ( "errors" "fmt" "io" + "maps" "os" "runtime" "strings" @@ -199,9 +200,7 @@ func (cli *ArduinoCLI) RunWithContext(ctx context.Context, args ...string) ([]by // GetDefaultEnv returns a copy of the default execution env used with the Run method. func (cli *ArduinoCLI) GetDefaultEnv() map[string]string { res := map[string]string{} - for k, v := range cli.cliEnvVars { - res[k] = v - } + maps.Copy(res, cli.cliEnvVars) return res } From 7b112c4f9e603c34cb1c519cfade07c8919f2085 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 10 Apr 2025 17:49:48 +0200 Subject: [PATCH 3/5] bugfix: Kill compile processes that generates too much output --- .../builder/internal/preprocessor/gcc.go | 41 +++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/internal/arduino/builder/internal/preprocessor/gcc.go b/internal/arduino/builder/internal/preprocessor/gcc.go index cbf156dfae6..c2398163e39 100644 --- a/internal/arduino/builder/internal/preprocessor/gcc.go +++ b/internal/arduino/builder/internal/preprocessor/gcc.go @@ -16,6 +16,7 @@ package preprocessor import ( + "bytes" "context" "errors" "fmt" @@ -77,10 +78,42 @@ func GCC( if err != nil { return Result{}, err } - stdout, stderr, err := proc.RunAndCaptureOutput(ctx) - // Append gcc arguments to stdout - stdout = append([]byte(fmt.Sprintln(strings.Join(args, " "))), stdout...) + stdout := bytes.NewBuffer(nil) + stderr := bytes.NewBuffer(nil) - return Result{args: proc.GetArgs(), stdout: stdout, stderr: stderr}, err + ctx, cancel := context.WithCancel(ctx) + defer cancel() + count := 0 + stderrLimited := writerFunc(func(p []byte) (int, error) { + // Limit the size of the stderr buffer to 100KB + n, err := stderr.Write(p) + count += n + if count > 100*1024 { + cancel() + fmt.Fprintln(stderr, i18n.Tr("Compiler error output has been truncated.")) + } + return n, err + }) + + proc.RedirectStdoutTo(stdout) + proc.RedirectStderrTo(stderrLimited) + + // Append gcc arguments to stdout before running the command + fmt.Fprintln(stdout, strings.Join(args, " ")) + + if err := proc.Start(); err != nil { + return Result{}, err + } + + // Wait for the process to finish + err = proc.WaitWithinContext(ctx) + + return Result{args: proc.GetArgs(), stdout: stdout.Bytes(), stderr: stderr.Bytes()}, err +} + +type writerFunc func(p []byte) (n int, err error) + +func (f writerFunc) Write(p []byte) (n int, err error) { + return f(p) } From 4e2a6508da481532ef020abe09ab851805618eb7 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Mon, 28 Apr 2025 21:24:23 +0200 Subject: [PATCH 4/5] Update go-paths-helper library This patch fixes how process are killed on MacOS and Windows --- .licenses/go/github.com/arduino/go-paths-helper.dep.yml | 4 ++-- go.mod | 2 +- go.sum | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.licenses/go/github.com/arduino/go-paths-helper.dep.yml b/.licenses/go/github.com/arduino/go-paths-helper.dep.yml index 8a0611c2ee1..c3a80f1c84e 100644 --- a/.licenses/go/github.com/arduino/go-paths-helper.dep.yml +++ b/.licenses/go/github.com/arduino/go-paths-helper.dep.yml @@ -1,8 +1,8 @@ --- name: github.com/arduino/go-paths-helper -version: v1.13.0 +version: v1.13.1 type: go -summary: +summary: homepage: https://pkg.go.dev/github.com/arduino/go-paths-helper license: gpl-2.0-or-later licenses: diff --git a/go.mod b/go.mod index 3510c2e00dc..f90b01e37c2 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ replace github.com/mailru/easyjson => github.com/cmaglie/easyjson v0.8.1 require ( fortio.org/safecast v1.0.0 github.com/ProtonMail/go-crypto v1.2.0 - github.com/arduino/go-paths-helper v1.13.0 + github.com/arduino/go-paths-helper v1.13.1 github.com/arduino/go-properties-orderedmap v1.8.1 github.com/arduino/go-serial-utils v0.1.2 github.com/arduino/go-timeutils v0.0.0-20171220113728-d1dd9e313b1b diff --git a/go.sum b/go.sum index a70cca2d7e4..814c30f0de0 100644 --- a/go.sum +++ b/go.sum @@ -10,8 +10,8 @@ github.com/ProtonMail/go-crypto v1.2.0/go.mod h1:9whxjD8Rbs29b4XWbB8irEcE8KHMqaR github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be h1:9AeTilPcZAjCFIImctFaOjnTIavg87rW78vTPkQqLI8= github.com/anmitsu/go-shlex v0.0.0-20200514113438-38f4b401e2be/go.mod h1:ySMOLuWl6zY27l47sB3qLNK6tF2fkHG55UZxx8oIVo4= github.com/arduino/go-paths-helper v1.0.1/go.mod h1:HpxtKph+g238EJHq4geEPv9p+gl3v5YYu35Yb+w31Ck= -github.com/arduino/go-paths-helper v1.13.0 h1:HIkgg8ChPw1QPNHkB5bQSs+geTj74vf6TFgVhm/9mmw= -github.com/arduino/go-paths-helper v1.13.0/go.mod h1:jcpW4wr0u69GlXhTYydsdsqAjLaYK5n7oWHfKqOG6LM= +github.com/arduino/go-paths-helper v1.13.1 h1:M7SCdLB2VldxOdChnjZkxAZwWZdDtNY4IlHL9nxGQFo= +github.com/arduino/go-paths-helper v1.13.1/go.mod h1:dDodKn2ZX4iwuoBMapdDO+5d0oDLBeM4BS0xS4i40Ak= github.com/arduino/go-properties-orderedmap v1.8.1 h1:nU5S6cXPwMoxZs4ORw61wPTALNfriIduvNB4cxTmNYM= github.com/arduino/go-properties-orderedmap v1.8.1/go.mod h1:DKjD2VXY/NZmlingh4lSFMEYCVubfeArCsGPGDwb2yk= github.com/arduino/go-serial-utils v0.1.2 h1:MRFwME4w/uaVkJ1R+wzz4KSbI9cF9IDVrYorazvjpTk= From c55ec6eec5492515e8671ec16c6bf09d34980a99 Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Wed, 30 Apr 2025 17:06:33 +0200 Subject: [PATCH 5/5] Apply suggestion from code review --- internal/arduino/builder/internal/preprocessor/gcc.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/arduino/builder/internal/preprocessor/gcc.go b/internal/arduino/builder/internal/preprocessor/gcc.go index c2398163e39..f8d6100d10d 100644 --- a/internal/arduino/builder/internal/preprocessor/gcc.go +++ b/internal/arduino/builder/internal/preprocessor/gcc.go @@ -90,8 +90,8 @@ func GCC( n, err := stderr.Write(p) count += n if count > 100*1024 { - cancel() fmt.Fprintln(stderr, i18n.Tr("Compiler error output has been truncated.")) + cancel() } return n, err })