Improve error handling in include detection

For include detection, the preprocessor is run on all source files,
collecting #included filenames from the stderr output, each of which are
then resolved to a library to include. A caching mechanism is used to
only run the preprocessor when needed.

This commit improves the error handling during include detection in a
number of ways:
 - When the preprocessor runs succesfully, processing stops for the
   current file. Previously, it would always look at stderr to find a
   missing include filename and only stop if none was found.
 - When the preprocessor fails, but no filename can be found, show the
   error preprocessor error. Previously, it would assume that the
   process was done and stop processing the file without any error.
 - When no library can be found for a missing include, show the stored
   error output instead of running the preprocessor again. Previously,
   the preprocessor would be run a second time, to (re)generate the
   error message.

   When the include filename comes from the cache and the preprocessor
   was not run yet, it is still run to show its errors to the user. This
   should be very rare, as normally changes that cause a cached filename
   to become unresolvable to a library also cause the file to be marked
   as changed, bypassing the cache. When this does happen, the
   preprocessor is now run using `GCCPreprocRunnerForDiscoveringIncludes()`
   instead of `GCCPreprocRunner()`, which ensures the preprocessor
   command is always exactly the same.

   Before this change, there could be specific circumstances where the
   first preprocessor run would generate an error, but where the second
   run would not show the error and include detection would continue as
   if nothing happened. One such circumstance is described in #230.

Signed-off-by: Matthijs Kooijman <[email protected]>

Author: matthijskooijman

Diff for: src/arduino.cc/builder/container_find_includes.go

@@ -119,6 +119,8 @@ import (
 	"arduino.cc/builder/i18n"
 	"arduino.cc/builder/types"
 	"arduino.cc/builder/utils"
+
+	"github.com/go-errors/errors"
 )
 
 type ContainerFindIncludes struct{}
@@ -318,22 +320,33 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t
 		if library, ok := sourceFile.Origin.(*types.Library); ok && library.UtilityFolder != "" {
 			includes = append(includes, library.UtilityFolder)
 		}
+		var preproc_err error
+		var preproc_stderr []byte
 		if unchanged && cache.valid {
 			include = cache.Next().Include
 			if first && ctx.Verbose {
 				ctx.GetLogger().Println(constants.LOG_LEVEL_INFO, constants.MSG_USING_CACHED_INCLUDES, sourcePath)
 			}
 		} else {
-			stderr, err := GCCPreprocRunnerForDiscoveringIncludes(ctx, sourcePath, targetFilePath, includes)
+			preproc_stderr, preproc_err = GCCPreprocRunnerForDiscoveringIncludes(ctx, sourcePath, targetFilePath, includes)
 			// Unwrap error and see if it is an ExitError.
-			// Ignore ExitErrors (e.g. gcc returning
-			// non-zero status), but bail out on other
-			// errors
-			_, is_exit_error := i18n.UnwrapError(err).(*exec.ExitError)
-			if err != nil && !is_exit_error {
-				return i18n.WrapError(err)
+			_, is_exit_error := i18n.UnwrapError(preproc_err).(*exec.ExitError)
+			if preproc_err == nil {
+				// Preprocessor successful, done
+				include = ""
+			} else if (!is_exit_error || preproc_stderr == nil) {
+				// Ignore ExitErrors (e.g. gcc returning
+				// non-zero status), but bail out on
+				// other errors
+				return i18n.WrapError(preproc_err)
+			} else {
+				include = IncludesFinderWithRegExp(ctx, string(preproc_stderr))
+				if include == "" {
+					// No include found? Bail out.
+					os.Stderr.Write(preproc_stderr)
+					return i18n.WrapError(preproc_err)
+				}
 			}
-			include = IncludesFinderWithRegExp(ctx, string(stderr))
 		}
 
 		if include == "" {
@@ -345,8 +358,19 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t
 		library := ResolveLibrary(ctx, include)
 		if library == nil {
 			// Library could not be resolved, show error
-			err := GCCPreprocRunner(ctx, sourcePath, constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E, includes)
-			return i18n.WrapError(err)
+			if preproc_err == nil || preproc_stderr == nil {
+				// Filename came from cache, so run preprocessor to obtain error to show
+				preproc_stderr, preproc_err = GCCPreprocRunnerForDiscoveringIncludes(ctx, sourcePath, targetFilePath, includes)
+				if preproc_err == nil {
+					// If there is a missing #include in the cache, but running
+					// gcc does not reproduce that, there is something wrong.
+					// Returning an error here will cause the cache to be
+					// deleted, so hopefully the next compilation will succeed.
+					return errors.New("Internal error in cache")
+				}
+			}
+			os.Stderr.Write(preproc_stderr)
+			return i18n.WrapError(preproc_err)
 		}
 
 		// Add this library to the list of libraries, the