Skip to content

Commit

Permalink
Let include detection generate .d files too
Browse files Browse the repository at this point in the history
Previously, the including detection process caching relied on the .d
files generated by compilation to know what .h files a given source
files depends on. This works correctly, but if a project does not
compile completely, not all .d files are generated, so not all cached
include detection results can be used.

In practice, this means that if a there is a compilation error in the
first file that is compiled, include detection will run again for all
files on the next run. If you have a few errors to solve and a big
project, this gets annoying quickly.

To fix this, the include detection process should generate .d files
itself. At first glance it appears that there is a problematic case
where the list of included header files changes, the include detection
overwrites the .d file and then compilation only sees the new list
(which was generated later than the .o file was generated). However,
since this implies that changes are made to an #include directive in the
source file itself or one of the files that are still included, this
should be detected normally. There is still a corner case when a file is
changed during the build, but that was already the case.

Since include detections uses `-o /dev/null`, the compiler generates a
slightly different .d file. During compilation, a file `foo.cpp.d` is
generated in the output directory starting with:

    /path/to/foo.cpp.o: \

But when just passing `-MMD` to the preproc recipe, it generates a
`foo.d` file in the source directory starting with:

  foo.o: \

To make these equal, `-MF` must be passed during include detection to
set the .d filename, and `-MT` must be passed to set the .o filename
inside the .d file.

To enable this feature, platform.txt should be modified by adding ` -MMD
-MF {dep_file} -MT {dep_file}` to `preproc.macros.flags` (or
`recipe.preproc.macros`). Without any changes to platform.txt, behaviour
is unchanged.

To allow this, this adds `{dep_file}` and `{object_file}` variables to
the build properties for the preproc macros recipe. For consistency,
`{dep_file}` is also added during normal compilation, though it is not
currently used.
  • Loading branch information
matthijskooijman committed Mar 2, 2017
1 parent 4e6ceff commit 33fde6d
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 9 deletions.
1 change: 1 addition & 0 deletions src/arduino.cc/builder/builder_utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ func compileFileWithRecipe(sourcePath string, source string, buildPath string, b
return "", i18n.WrapError(err)
}
properties[constants.BUILD_PROPERTIES_OBJECT_FILE] = filepath.Join(buildPath, relativeSource+".o")
properties[constants.BUILD_PROPERTIES_DEP_FILE] = filepath.Join(buildPath, relativeSource+".d")

err = utils.EnsureFolderExists(filepath.Dir(properties[constants.BUILD_PROPERTIES_OBJECT_FILE]))
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions src/arduino.cc/builder/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ const BUILD_PROPERTIES_EXTRA_TIME_UTC = "extra.time.utc"
const BUILD_PROPERTIES_EXTRA_TIME_ZONE = "extra.time.zone"
const BUILD_PROPERTIES_INCLUDES = "includes"
const BUILD_PROPERTIES_OBJECT_FILE = "object_file"
const BUILD_PROPERTIES_DEP_FILE = "dep_file"
const BUILD_PROPERTIES_OBJECT_FILES = "object_files"
const BUILD_PROPERTIES_PATTERN = "pattern"
const BUILD_PROPERTIES_PID = "pid"
Expand Down
4 changes: 3 additions & 1 deletion src/arduino.cc/builder/container_add_prototypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,10 @@ type ContainerAddPrototypes struct{}

func (s *ContainerAddPrototypes) Run(ctx *types.Context) error {
sourceFile := filepath.Join(ctx.SketchBuildPath, filepath.Base(ctx.Sketch.MainFile.Name)+".cpp")
depFile := sourceFile+".d"
objFile := sourceFile+".o"
commands := []types.Command{
&GCCPreprocRunner{SourceFilePath: sourceFile, TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E, Includes: ctx.IncludeFolders},
&GCCPreprocRunner{SourceFilePath: sourceFile, ObjFilePath: objFile, DepFilePath: depFile, TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E, Includes: ctx.IncludeFolders},
&ReadFileAndStoreInContext{Target: &ctx.SourceGccMinusE},
&FilterSketchSource{Source: &ctx.SourceGccMinusE},
&CTagsTargetFileSaver{Source: &ctx.SourceGccMinusE, TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E},
Expand Down
10 changes: 5 additions & 5 deletions src/arduino.cc/builder/container_find_includes.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,8 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t

// TODO: This should perhaps also compare against the
// include.cache file timestamp. Now, it only checks if the file
// changed after the object file was generated, but if it
// changed between generating the cache and the object file,
// changed after the dependency file was generated, but if it
// changed between generating the cache and the dependency file,
// this could show the file as unchanged when it really is
// changed. Changing files during a build isn't really
// supported, but any problems from it should at least be
Expand All @@ -305,7 +305,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t
// TODO: This reads the dependency file, but the actual building
// does it again. Should the result be somehow cached? Perhaps
// remove the object file if it is found to be stale?
unchanged, err := builder_utils.BuildResultIsUpToDate(sourcePath, objPath, objPath, depPath, ctx.DebugLevel, ctx.GetLogger())
unchanged, err := builder_utils.BuildResultIsUpToDate(sourcePath, depPath, objPath, depPath, ctx.DebugLevel, ctx.GetLogger())
if err != nil {
return i18n.WrapError(err)
}
Expand All @@ -326,7 +326,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t
}
} else {
commands := []types.Command{
&GCCPreprocRunnerForDiscoveringIncludes{SourceFilePath: sourcePath, TargetFilePath: targetFilePath, Includes: includes},
&GCCPreprocRunnerForDiscoveringIncludes{SourceFilePath: sourcePath, ObjFilePath: objPath, DepFilePath: depPath, TargetFilePath: targetFilePath, Includes: includes},
&IncludesFinderWithRegExp{Source: &ctx.SourceGccMinusE},
}
for _, command := range commands {
Expand All @@ -347,7 +347,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t
library := ResolveLibrary(ctx, include)
if library == nil {
// Library could not be resolved, show error
err := runCommand(ctx, &GCCPreprocRunner{SourceFilePath: sourcePath, TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E, Includes: includes})
err := runCommand(ctx, &GCCPreprocRunner{SourceFilePath: sourcePath, ObjFilePath: objPath, DepFilePath: depPath, TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E, Includes: includes})
return i18n.WrapError(err)
}

Expand Down
16 changes: 13 additions & 3 deletions src/arduino.cc/builder/gcc_preproc_runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,14 @@ import (

type GCCPreprocRunner struct {
SourceFilePath string
ObjFilePath string
DepFilePath string
TargetFileName string
Includes []string
}

func (s *GCCPreprocRunner) Run(ctx *types.Context) error {
properties, targetFilePath, err := prepareGCCPreprocRecipeProperties(ctx, s.SourceFilePath, s.TargetFileName, s.Includes)
properties, targetFilePath, err := prepareGCCPreprocRecipeProperties(ctx, s.SourceFilePath, s.ObjFilePath, s.DepFilePath, s.TargetFileName, s.Includes)
if err != nil {
return i18n.WrapError(err)
}
Expand All @@ -72,12 +74,14 @@ func (s *GCCPreprocRunner) Run(ctx *types.Context) error {

type GCCPreprocRunnerForDiscoveringIncludes struct {
SourceFilePath string
ObjFilePath string
DepFilePath string
TargetFilePath string
Includes []string
}

func (s *GCCPreprocRunnerForDiscoveringIncludes) Run(ctx *types.Context) error {
properties, _, err := prepareGCCPreprocRecipeProperties(ctx, s.SourceFilePath, s.TargetFilePath, s.Includes)
properties, _, err := prepareGCCPreprocRecipeProperties(ctx, s.SourceFilePath, s.ObjFilePath, s.DepFilePath, s.TargetFilePath, s.Includes)
if err != nil {
return i18n.WrapError(err)
}
Expand All @@ -100,7 +104,7 @@ func (s *GCCPreprocRunnerForDiscoveringIncludes) Run(ctx *types.Context) error {
return nil
}

func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) (properties.Map, string, error) {
func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath string, objFilePath string, depFilePath string, targetFilePath string, includes []string) (properties.Map, string, error) {
if targetFilePath != utils.NULLFile() {
preprocPath := ctx.PreprocPath
err := utils.EnsureFolderExists(preprocPath)
Expand All @@ -113,6 +117,12 @@ func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath string
properties := ctx.BuildProperties.Clone()
properties[constants.BUILD_PROPERTIES_SOURCE_FILE] = sourceFilePath
properties[constants.BUILD_PROPERTIES_PREPROCESSED_FILE_PATH] = targetFilePath
properties[constants.BUILD_PROPERTIES_DEP_FILE] = depFilePath
properties[constants.BUILD_PROPERTIES_OBJECT_FILE] = objFilePath
err := utils.EnsureFolderExists(filepath.Dir(depFilePath))
if err != nil {
return nil, "", i18n.WrapError(err)
}

includes = utils.Map(includes, utils.WrapWithHyphenI)
properties[constants.BUILD_PROPERTIES_INCLUDES] = strings.Join(includes, constants.SPACE)
Expand Down

0 comments on commit 33fde6d

Please sign in to comment.