From 9ee79f628e3c85307065b3ef353a47846daa649c Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 17 Aug 2023 17:09:37 +0200 Subject: [PATCH 1/2] regression: hide include-not-found errors during library discovery This regression was made during a refactoring of the Arduino preprocessor. In particular the wrong change was part of this commit: https://github.com/arduino/arduino-cli/commit/0585435f8b1d05e0117606f69bea42d3f3dfec79#diff-65ff16cbee816c0f443157444d99bcc144beee06c3329aec891894c8aeda7b27L372-R378 - preproc_stderr, preproc_err = GCCPreprocRunner(ctx, sourcePath, targetFilePath, includes) + var preproc_stdout []byte + preproc_stdout, preproc_stderr, preproc_err = preprocessor.GCC(sourcePath, targetFilePath, includes, ctx.BuildProperties) + if ctx.Verbose { + ctx.WriteStdout(preproc_stdout) + ctx.WriteStdout(preproc_stderr) + } Previously GCCPreprocRunner, in verbose modem will show ONLY the stdout of the process, instead the "refactored" code wrongly added also stderr to the output. For reference this is the old GCCPreprocRunner implementation: https://github.com/arduino/arduino-cli/commit/0585435f8b1d05e0117606f69bea42d3f3dfec79#diff-371f93465ca5a66f01cbe876348f67990750091d27a827781c8633456b93ef3bL36 -func GCCPreprocRunner(ctx *types.Context, sourceFilePath *paths.Path, targetFilePath *paths.Path, includes paths.PathList) ([]byte, error) { - cmd, err := prepareGCCPreprocRecipeProperties(ctx, sourceFilePath, targetFilePath, includes) - if err != nil { - return nil, err - } - _, stderr, err := utils.ExecCommand(ctx, cmd, utils.ShowIfVerbose /* stdout */, utils.Capture /* stderr */) - return stderr, err -} This commit fixes the regression. --- legacy/builder/container_find_includes.go | 1 - 1 file changed, 1 deletion(-) diff --git a/legacy/builder/container_find_includes.go b/legacy/builder/container_find_includes.go index 74cc562dde7..69bf06dc9ac 100644 --- a/legacy/builder/container_find_includes.go +++ b/legacy/builder/container_find_includes.go @@ -374,7 +374,6 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFileQu preprocStdout, preprocStderr, preprocErr = preprocessor.GCC(sourcePath, targetFilePath, includeFolders, ctx.BuildProperties) if ctx.Verbose { ctx.WriteStdout(preprocStdout) - ctx.WriteStdout(preprocStderr) } // Unwrap error and see if it is an ExitError. _, isExitErr := errors.Cause(preprocErr).(*exec.ExitError) From 38f33e62f1d59ba95a9e604861d9fcf71f174bca Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Thu, 17 Aug 2023 17:26:25 +0200 Subject: [PATCH 2/2] Added integration test --- .../integrationtest/compile_3/compile_test.go | 33 ++++++++++++++----- .../testdata/using_Wire/using_Wire.ino | 4 +++ 2 files changed, 29 insertions(+), 8 deletions(-) create mode 100644 internal/integrationtest/compile_3/testdata/using_Wire/using_Wire.ino diff --git a/internal/integrationtest/compile_3/compile_test.go b/internal/integrationtest/compile_3/compile_test.go index 8e6aa34296e..2df3d59f40d 100644 --- a/internal/integrationtest/compile_3/compile_test.go +++ b/internal/integrationtest/compile_3/compile_test.go @@ -107,15 +107,32 @@ func TestCompilerErrOutput(t *testing.T) { _, _, err := cli.Run("core", "install", "arduino:avr@1.8.5") require.NoError(t, err) - // prepare sketch - sketch, err := paths.New("testdata", "blink_with_wrong_cpp").Abs() - 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"`) + } - // 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"`) + // Check that library discover do not generate false errors + // https://github.com/arduino/arduino-cli/issues/2263 + { + // prepare sketch + sketch, err := paths.New("testdata", "using_Wire").Abs() + require.NoError(t, err) + + // Run compile and catch err stream + out, _, err := cli.Run("compile", "-b", "arduino:avr:uno", "-v", "--format", "json", sketch.String()) + require.NoError(t, err) + jsonOut := requirejson.Parse(t, out) + jsonOut.Query(".compiler_out").MustNotContain(`"fatal error"`) + jsonOut.Query(".compiler_err").MustNotContain(`"fatal error"`) + } } func TestCompileRelativeLibraryPath(t *testing.T) { diff --git a/internal/integrationtest/compile_3/testdata/using_Wire/using_Wire.ino b/internal/integrationtest/compile_3/testdata/using_Wire/using_Wire.ino new file mode 100644 index 00000000000..2ce144c2f7b --- /dev/null +++ b/internal/integrationtest/compile_3/testdata/using_Wire/using_Wire.ino @@ -0,0 +1,4 @@ +#include + +void setup() {} +void loop() {}