From b64876c72c33f00c074f30c5c2183c80a7acecc0 Mon Sep 17 00:00:00 2001
From: Cristian Maglie <c.maglie@arduino.cc>
Date: Fri, 18 Aug 2023 09:54:54 +0200
Subject: [PATCH] [skip-changelog] regression: hide include-not-found errors
 during library discovery (#2267)

* 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.

* Added integration test
---
 .../integrationtest/compile_3/compile_test.go | 33 ++++++++++++++-----
 .../testdata/using_Wire/using_Wire.ino        |  4 +++
 legacy/builder/container_find_includes.go     |  1 -
 3 files changed, 29 insertions(+), 9 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 <Wire.h>
+
+void setup() {}
+void loop() {}
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)