10000 Improve error handling in include detection · facchinm/arduino-builder@a2c8e5d · GitHub
[go: up one dir, main page]
Skip to content

Commit a2c8e5d

Browse files
matthijskooijmanfacchinm
matthijskooijman
authored and
facchinm
committed
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 arduino#230. Signed-off-by: Matthijs Kooijman <matthijs@stdin.nl>
1 parent 2d65d3e commit a2c8e5d

File tree

1 file changed

+34
-10
lines changed

1 file changed

+34
-10
lines changed

‎src/arduino.cc/builder/container_find_includes.go

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,8 @@ import (
119119
"arduino.cc/builder/i18n"
120120
"arduino.cc/builder/types"
121121
"arduino.cc/builder/utils"
122+
123+
"github.com/go-errors/errors"
122124
)
123125

124126
type ContainerFindIncludes struct{}
@@ -318,22 +320,33 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t
318320
if library, ok := sourceFile.Origin.(*types.Library); ok && library.UtilityFolder != "" {
319321
includes = append(includes, library.UtilityFolder)
320322
}
323+
var preproc_err error
324+
var preproc_stderr []byte
321325
if unchanged && cache.valid {
322326
include = cache.Next().Include
323327
if first && ctx.Verbose {
324328
ctx.GetLogger().Println(constants.LOG_LEVEL_INFO, constants.MSG_USING_CACHED_INCLUDES, sourcePath)
325329
}
326330
} else {
327-
stderr, err := GCCPreprocRunnerForDiscoveringIncludes(ctx, sourcePath, targetFilePath, includes)
331+
preproc_stderr, preproc_err = GCCPreprocRunnerForDiscoveringIncludes(ctx, sourcePath, targetFilePath, includes)
328332
// Unwrap error and see if it is an ExitError.
329-
// Ignore ExitErrors (e.g. gcc returning
330-
// non-zero status), but bail out on other
331-
// errors
332-
_, is_exit_error := i18n.UnwrapError(err).(*exec.ExitError)
333-
if err != nil && !is_exit_error {
334-
return i18n.WrapError(err)
333+
_, is_exit_error := i18n.UnwrapError(preproc_err).(*exec.ExitError)
334+
if preproc_err == nil {
335+
// Preprocessor successful, done
336+
include = ""
337+
} else if (!is_exit_error || preproc_stderr == nil) {
338+
// Ignore ExitErrors (e.g. gcc returning
339+
// non-zero status), but bail out on
340+
// other errors
341+
return i18n.WrapError(preproc_err)
342+
} else {
343+
include = IncludesFinderWithRegExp(ctx, string(preproc_stderr))
344+
if include == "" {
345+
// No include found? Bail out.
346+
os.Stderr.Write(preproc_stderr)
347+
return i18n.WrapError(preproc_err)
348+
}
335349
}
336-
include = IncludesFinderWithRegExp(ctx, string(stderr))
337350
}
338351

339352
if include == "" {
@@ -345,8 +358,19 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sour 8000 ceFile t
345358
library := ResolveLibrary(ctx, include)
346359
if library == nil {
347360
// Library could not be resolved, show error
348-
err := GCCPreprocRunner(ctx, sourcePath, constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E, includes)
349-
return i18n.WrapError(err)
361+
if preproc_err == nil || preproc_stderr == nil {
362+
// Filename came from cache, so run preprocessor to obtain error to show
363+
preproc_stderr, preproc_err = GCCPreprocRunnerForDiscoveringIncludes(ctx, sourcePath, targetFilePath, includes)
364+
if preproc_err == nil {
365+
// If there is a missing #include in the cache, but running
366+
// gcc does not reproduce that, there is something wrong.
367+
// Returning an error here will cause the cache to be
368+
// deleted, so hopefully the next compilation will succeed.
369+
return errors.New("Internal error in cache")
370+
}
371+
}
372+
os.Stderr.Write(preproc_stderr)
373+
return i18n.WrapError(preproc_err)
350374
}
351375

352376
// Add this library to the list of libraries, the

0 commit comments

Comments
 (0)
< 2F8E /div>
0