From 66cdcc00c7e0b5a9b02ca1461b9118643aaa094a Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 6 Jun 2017 12:55:56 +0200 Subject: [PATCH 01/21] Fix past-end-of-cache handling in includeCache.ExpectFile The comments state that if ExpectFile() is called, but there are no remaining items in the cache, it will invalidate the cache. However, the code would only invalidate the cache if at least one item was still present, but it didn't match the expected file. In practice, this wouldn't usually cause issues, since adding a new file would usually cause an invalid cache earlier on, and even if a new file was added at the end of the compilation, it would not be in the .d file, so it would be marked as "changed". However, in rare circumstances, such as when the include cache would not be properly generated due to other problems (see #230), this would cause a crash, when ExpectFile did not invalidate the cache and the file in question was unchanged, causing an out-of-bounds read from the cache. This commit fixes this by making ExpectFile behave like documented and invalidate the cache when there are no remaining entries. Signed-off-by: Matthijs Kooijman --- container_find_includes.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/container_find_includes.go b/container_find_includes.go index dbd31c71..a06cf059 100644 --- a/container_find_includes.go +++ b/container_find_includes.go @@ -213,7 +213,7 @@ func (cache *includeCache) Next() includeCacheEntry { // not, or no entry is available, the cache is invalidated. Does not // advance the cache. func (cache *includeCache) ExpectFile(sourcefile string) { - if cache.valid && cache.next < len(cache.entries) && cache.Next().Sourcefile != sourcefile { + if cache.valid && (cache.next >= len(cache.entries) || cache.Next().Sourcefile != sourcefile) { cache.valid = false cache.entries = cache.entries[:cache.next] } From f69a8ccfd58f7ae73c68aa2855473fb4499e3529 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 6 Jun 2017 18:10:26 +0200 Subject: [PATCH 02/21] Convert IncludesFinderWithRegExp to a normal function Signed-off-by: Matthijs Kooijman --- container_find_includes.go | 3 +- includes_finder_with_regexp.go | 14 ++----- test/includes_finder_with_regexp_test.go | 50 ++++++------------------ types/context.go | 1 - 4 files changed, 16 insertions(+), 52 deletions(-) diff --git a/container_find_includes.go b/container_find_includes.go index a06cf059..c5feed38 100644 --- a/container_find_includes.go +++ b/container_find_includes.go @@ -325,7 +325,6 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t } else { commands := []types.Command{ &GCCPreprocRunnerForDiscoveringIncludes{SourceFilePath: sourcePath, TargetFilePath: targetFilePath, Includes: includes}, - &IncludesFinderWithRegExp{Source: &ctx.SourceGccMinusE}, } for _, command := range commands { err := runCommand(ctx, command) @@ -333,7 +332,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t return i18n.WrapError(err) } } - include = ctx.IncludeJustFound + include = IncludesFinderWithRegExp(ctx, ctx.SourceGccMinusE) } if include == "" { diff --git a/includes_finder_with_regexp.go b/includes_finder_with_regexp.go index 56ac955e..abfe7635 100644 --- a/includes_finder_with_regexp.go +++ b/includes_finder_with_regexp.go @@ -37,21 +37,13 @@ import ( var INCLUDE_REGEXP = regexp.MustCompile("(?ms)^\\s*#[ \t]*include\\s*[<\"](\\S+)[\">]") -type IncludesFinderWithRegExp struct { - Source *string -} - -func (s *IncludesFinderWithRegExp) Run(ctx *types.Context) error { - source := *s.Source - +func IncludesFinderWithRegExp(ctx *types.Context, source string) string { match := INCLUDE_REGEXP.FindStringSubmatch(source) if match != nil { - ctx.IncludeJustFound = strings.TrimSpace(match[1]) + return strings.TrimSpace(match[1]) } else { - ctx.IncludeJustFound = findIncludeForOldCompilers(source) + return findIncludeForOldCompilers(source) } - - return nil } func findIncludeForOldCompilers(source string) string { diff --git a/test/includes_finder_with_regexp_test.go b/test/includes_finder_with_regexp_test.go index d95153a5..c3c048cc 100644 --- a/test/includes_finder_with_regexp_test.go +++ b/test/includes_finder_with_regexp_test.go @@ -43,27 +43,17 @@ func TestIncludesFinderWithRegExp(t *testing.T) { "#include \n" + "^\n" + "compilation terminated." - ctx.Source = output + include := builder.IncludesFinderWithRegExp(ctx, output) - parser := builder.IncludesFinderWithRegExp{Source: &ctx.Source} - err := parser.Run(ctx) - NoError(t, err) - - require.Equal(t, "SPI.h", ctx.IncludeJustFound) + require.Equal(t, "SPI.h", include) } func TestIncludesFinderWithRegExpEmptyOutput(t *testing.T) { ctx := &types.Context{} - output := "" - - ctx.Source = output + include := builder.IncludesFinderWithRegExp(ctx, "") - parser := builder.IncludesFinderWithRegExp{Source: &ctx.Source} - err := parser.Run(ctx) - NoError(t, err) - - require.Equal(t, "", ctx.IncludeJustFound) + require.Equal(t, "", include) } func TestIncludesFinderWithRegExpPaddedIncludes(t *testing.T) { @@ -73,13 +63,9 @@ func TestIncludesFinderWithRegExpPaddedIncludes(t *testing.T) { " # include \n" + " ^\n" + "compilation terminated.\n" - ctx.Source = output - - parser := builder.IncludesFinderWithRegExp{Source: &ctx.Source} - err := parser.Run(ctx) - NoError(t, err) + include := builder.IncludesFinderWithRegExp(ctx, output) - require.Equal(t, "Wire.h", ctx.IncludeJustFound) + require.Equal(t, "Wire.h", include) } func TestIncludesFinderWithRegExpPaddedIncludes2(t *testing.T) { @@ -89,13 +75,9 @@ func TestIncludesFinderWithRegExpPaddedIncludes2(t *testing.T) { " #\t\t\tinclude \n" + " ^\n" + "compilation terminated.\n" - ctx.Source = output + include := builder.IncludesFinderWithRegExp(ctx, output) - parser := builder.IncludesFinderWithRegExp{Source: &ctx.Source} - err := parser.Run(ctx) - NoError(t, err) - - require.Equal(t, "Wire.h", ctx.IncludeJustFound) + require.Equal(t, "Wire.h", include) } func TestIncludesFinderWithRegExpPaddedIncludes3(t *testing.T) { @@ -104,13 +86,9 @@ func TestIncludesFinderWithRegExpPaddedIncludes3(t *testing.T) { output := "/some/path/sketch.ino:1:33: fatal error: SPI.h: No such file or directory\n" + "compilation terminated.\n" - ctx.Source = output - - parser := builder.IncludesFinderWithRegExp{Source: &ctx.Source} - err := parser.Run(ctx) - NoError(t, err) + include := builder.IncludesFinderWithRegExp(ctx, output) - require.Equal(t, "SPI.h", ctx.IncludeJustFound) + require.Equal(t, "SPI.h", include) } func TestIncludesFinderWithRegExpPaddedIncludes4(t *testing.T) { @@ -119,11 +97,7 @@ func TestIncludesFinderWithRegExpPaddedIncludes4(t *testing.T) { output := "In file included from /tmp/arduino_modified_sketch_815412/binouts.ino:52:0:\n" + "/tmp/arduino_build_static/sketch/regtable.h:31:22: fatal error: register.h: No such file or directory\n" - ctx.Source = output - - parser := builder.IncludesFinderWithRegExp{Source: &ctx.Source} - err := parser.Run(ctx) - NoError(t, err) + include := builder.IncludesFinderWithRegExp(ctx, output) - require.Equal(t, "register.h", ctx.IncludeJustFound) + require.Equal(t, "register.h", include) } diff --git a/types/context.go b/types/context.go index bc9491dd..67c4b1bd 100644 --- a/types/context.go +++ b/types/context.go @@ -61,7 +61,6 @@ type Context struct { HeaderToLibraries map[string][]*Library ImportedLibraries []*Library LibrariesResolutionResults map[string]LibraryResolutionResult - IncludeJustFound string IncludeFolders []string OutputGccMinusM string From be0880995b27206a59e1acc51a70122fb1dec518 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 6 Jun 2017 19:07:40 +0200 Subject: [PATCH 03/21] Convert GCCPreprocRunner(ForDiscoveringIncludes) to a normal function Signed-off-by: Matthijs Kooijman --- container_add_prototypes.go | 7 ++++++- container_find_includes.go | 15 +++++---------- gcc_preproc_runner.go | 28 +++++++--------------------- 3 files changed, 18 insertions(+), 32 deletions(-) diff --git a/container_add_prototypes.go b/container_add_prototypes.go index cd0d56f7..14f30199 100644 --- a/container_add_prototypes.go +++ b/container_add_prototypes.go @@ -41,8 +41,13 @@ type ContainerAddPrototypes struct{} func (s *ContainerAddPrototypes) Run(ctx *types.Context) error { sourceFile := filepath.Join(ctx.SketchBuildPath, filepath.Base(ctx.Sketch.MainFile.Name)+".cpp") + + // Run preprocessor + err := GCCPreprocRunner(ctx, sourceFile, constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E, ctx.IncludeFolders) + if err != nil { + return i18n.WrapError(err) + } commands := []types.Command{ - &GCCPreprocRunner{SourceFilePath: sourceFile, 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}, diff --git a/container_find_includes.go b/container_find_includes.go index c5feed38..91bafd7c 100644 --- a/container_find_includes.go +++ b/container_find_includes.go @@ -323,16 +323,11 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t ctx.GetLogger().Println(constants.LOG_LEVEL_INFO, constants.MSG_USING_CACHED_INCLUDES, sourcePath) } } else { - commands := []types.Command{ - &GCCPreprocRunnerForDiscoveringIncludes{SourceFilePath: sourcePath, TargetFilePath: targetFilePath, Includes: includes}, + stderr, err := GCCPreprocRunnerForDiscoveringIncludes(ctx, sourcePath, targetFilePath, includes) + if err != nil { + return i18n.WrapError(err) } - for _, command := range commands { - err := runCommand(ctx, command) - if err != nil { - return i18n.WrapError(err) - } - } - include = IncludesFinderWithRegExp(ctx, ctx.SourceGccMinusE) + include = IncludesFinderWithRegExp(ctx, stderr) } if include == "" { @@ -344,7 +339,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 := GCCPreprocRunner(ctx, sourcePath, constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E, includes) return i18n.WrapError(err) } diff --git a/gcc_preproc_runner.go b/gcc_preproc_runner.go index b571cacc..9d3bd948 100644 --- a/gcc_preproc_runner.go +++ b/gcc_preproc_runner.go @@ -41,14 +41,8 @@ import ( "github.com/arduino/go-properties-map" ) -type GCCPreprocRunner struct { - SourceFilePath string - TargetFileName string - Includes []string -} - -func (s *GCCPreprocRunner) Run(ctx *types.Context) error { - properties, targetFilePath, err := prepareGCCPreprocRecipeProperties(ctx, s.SourceFilePath, s.TargetFileName, s.Includes) +func GCCPreprocRunner(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) error { + properties, targetFilePath, err := prepareGCCPreprocRecipeProperties(ctx, sourceFilePath, targetFilePath, includes) if err != nil { return i18n.WrapError(err) } @@ -70,16 +64,10 @@ func (s *GCCPreprocRunner) Run(ctx *types.Context) error { return nil } -type GCCPreprocRunnerForDiscoveringIncludes struct { - SourceFilePath string - TargetFilePath string - Includes []string -} - -func (s *GCCPreprocRunnerForDiscoveringIncludes) Run(ctx *types.Context) error { - properties, _, err := prepareGCCPreprocRecipeProperties(ctx, s.SourceFilePath, s.TargetFilePath, s.Includes) +func GCCPreprocRunnerForDiscoveringIncludes(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) (string, error) { + properties, _, err := prepareGCCPreprocRecipeProperties(ctx, sourceFilePath, targetFilePath, includes) if err != nil { - return i18n.WrapError(err) + return "", i18n.WrapError(err) } verbose := ctx.Verbose @@ -92,12 +80,10 @@ func (s *GCCPreprocRunnerForDiscoveringIncludes) Run(ctx *types.Context) error { stderr, err := builder_utils.ExecRecipeCollectStdErr(properties, constants.RECIPE_PREPROC_MACROS, true, verbose, false, logger) if err != nil { - return i18n.WrapError(err) + return "", i18n.WrapError(err) } - ctx.SourceGccMinusE = string(stderr) - - return nil + return string(stderr), nil } func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) (properties.Map, string, error) { From 85d3624f1e422c1fb5ae9500164967daee8032f9 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 6 Jun 2017 19:27:56 +0200 Subject: [PATCH 04/21] Refactor path generation for ctags_target_for_gcc_minus_e.cpp Previously, the relative filename passed to GCCPreprocRunner() was made absolute by prepareGCCPreprocRecipeProperties() and then returned, so it could be used later on. There was an exception to this when /dev/null was passed. Now, the only place that passed something other than /dev/null simply does this processing up front instead. This prepares the way for removing Context::FileToRead in the next commit. Signed-off-by: Matthijs Kooijman --- container_add_prototypes.go | 10 +++++++++- gcc_preproc_runner.go | 18 ++++-------------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/container_add_prototypes.go b/container_add_prototypes.go index 14f30199..fb1f04ec 100644 --- a/container_add_prototypes.go +++ b/container_add_prototypes.go @@ -35,6 +35,7 @@ import ( "github.com/arduino/arduino-builder/constants" "github.com/arduino/arduino-builder/i18n" "github.com/arduino/arduino-builder/types" + "github.com/arduino/arduino-builder/utils" ) type ContainerAddPrototypes struct{} @@ -42,8 +43,15 @@ type ContainerAddPrototypes struct{} func (s *ContainerAddPrototypes) Run(ctx *types.Context) error { sourceFile := filepath.Join(ctx.SketchBuildPath, filepath.Base(ctx.Sketch.MainFile.Name)+".cpp") + // Generate the full pathname for the preproc output file + err := utils.EnsureFolderExists(ctx.PreprocPath) + if err != nil { + return i18n.WrapError(err) + } + targetFilePath := filepath.Join(ctx.PreprocPath, constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E) + // Run preprocessor - err := GCCPreprocRunner(ctx, sourceFile, constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E, ctx.IncludeFolders) + err = GCCPreprocRunner(ctx, sourceFile, targetFilePath, ctx.IncludeFolders) if err != nil { return i18n.WrapError(err) } diff --git a/gcc_preproc_runner.go b/gcc_preproc_runner.go index 9d3bd948..6df8f421 100644 --- a/gcc_preproc_runner.go +++ b/gcc_preproc_runner.go @@ -30,7 +30,6 @@ package builder import ( - "path/filepath" "strings" "github.com/arduino/arduino-builder/builder_utils" @@ -42,7 +41,7 @@ import ( ) func GCCPreprocRunner(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) error { - properties, targetFilePath, err := prepareGCCPreprocRecipeProperties(ctx, sourceFilePath, targetFilePath, includes) + properties, err := prepareGCCPreprocRecipeProperties(ctx, sourceFilePath, targetFilePath, includes) if err != nil { return i18n.WrapError(err) } @@ -65,7 +64,7 @@ func GCCPreprocRunner(ctx *types.Context, sourceFilePath string, targetFilePath } func GCCPreprocRunnerForDiscoveringIncludes(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) (string, error) { - properties, _, err := prepareGCCPreprocRecipeProperties(ctx, sourceFilePath, targetFilePath, includes) + properties, err := prepareGCCPreprocRecipeProperties(ctx, sourceFilePath, targetFilePath, includes) if err != nil { return "", i18n.WrapError(err) } @@ -86,16 +85,7 @@ func GCCPreprocRunnerForDiscoveringIncludes(ctx *types.Context, sourceFilePath s return string(stderr), nil } -func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) (properties.Map, string, error) { - if targetFilePath != utils.NULLFile() { - preprocPath := ctx.PreprocPath - err := utils.EnsureFolderExists(preprocPath) - if err != nil { - return nil, "", i18n.WrapError(err) - } - targetFilePath = filepath.Join(preprocPath, targetFilePath) - } - +func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) (properties.Map, error) { properties := ctx.BuildProperties.Clone() properties[constants.BUILD_PROPERTIES_SOURCE_FILE] = sourceFilePath properties[constants.BUILD_PROPERTIES_PREPROCESSED_FILE_PATH] = targetFilePath @@ -104,7 +94,7 @@ func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath string properties[constants.BUILD_PROPERTIES_INCLUDES] = strings.Join(includes, constants.SPACE) builder_utils.RemoveHyphenMDDFlagFromGCCCommandLine(properties) - return properties, targetFilePath, nil + return properties, nil } func GeneratePreprocPatternFromCompile(compilePattern string) string { From a14e0ef8730a7cd60a1b83a71061bb5ae433bf6b Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 6 Jun 2017 19:33:44 +0200 Subject: [PATCH 05/21] Pass FileToRead to ReadFileAndStoreInContext explicitly Previously, this filename was set by GCCPreprocRunner into the context, because the full filename was not known until then. Since the previous commit, this filename is known by the ContainerAddPrototypes, which can just pass it to ReadFileAndStoreInContext explicitly. This allows Context::FileToRead to be removed. Signed-off-by: Matthijs Kooijman --- container_add_prototypes.go | 2 +- gcc_preproc_runner.go | 2 -- read_file_and_store_in_context.go | 3 ++- test/read_file_and_store_in_context_test.go | 3 +-- types/context.go | 3 --- 5 files changed, 4 insertions(+), 9 deletions(-) diff --git a/container_add_prototypes.go b/container_add_prototypes.go index fb1f04ec..13b777d3 100644 --- a/container_add_prototypes.go +++ b/container_add_prototypes.go @@ -56,7 +56,7 @@ func (s *ContainerAddPrototypes) Run(ctx *types.Context) error { return i18n.WrapError(err) } commands := []types.Command{ - &ReadFileAndStoreInContext{Target: &ctx.SourceGccMinusE}, + &ReadFileAndStoreInContext{FileToRead: targetFilePath, Target: &ctx.SourceGccMinusE}, &FilterSketchSource{Source: &ctx.SourceGccMinusE}, &CTagsTargetFileSaver{Source: &ctx.SourceGccMinusE, TargetFileName: constants.FILE_CTAGS_TARGET_FOR_GCC_MINUS_E}, &CTagsRunner{}, diff --git a/gcc_preproc_runner.go b/gcc_preproc_runner.go index 6df8f421..0f95917f 100644 --- a/gcc_preproc_runner.go +++ b/gcc_preproc_runner.go @@ -58,8 +58,6 @@ func GCCPreprocRunner(ctx *types.Context, sourceFilePath string, targetFilePath return i18n.WrapError(err) } - ctx.FileToRead = targetFilePath - return nil } diff --git a/read_file_and_store_in_context.go b/read_file_and_store_in_context.go index a2533929..bd165617 100644 --- a/read_file_and_store_in_context.go +++ b/read_file_and_store_in_context.go @@ -36,11 +36,12 @@ import ( ) type ReadFileAndStoreInContext struct { + FileToRead string Target *string } func (s *ReadFileAndStoreInContext) Run(ctx *types.Context) error { - bytes, err := ioutil.ReadFile(ctx.FileToRead) + bytes, err := ioutil.ReadFile(s.FileToRead) if err != nil { return i18n.WrapError(err) } diff --git a/test/read_file_and_store_in_context_test.go b/test/read_file_and_store_in_context_test.go index a83cfa0c..b33ff47c 100644 --- a/test/read_file_and_store_in_context_test.go +++ b/test/read_file_and_store_in_context_test.go @@ -47,9 +47,8 @@ func TestReadFileAndStoreInContext(t *testing.T) { utils.WriteFile(file.Name(), "test test\nciao") ctx := &types.Context{} - ctx.FileToRead = file.Name() - command := &builder.ReadFileAndStoreInContext{Target: &ctx.SourceGccMinusE} + command := &builder.ReadFileAndStoreInContext{FileToRead: file.Name(), Target: &ctx.SourceGccMinusE} err = command.Run(ctx) NoError(t, err) diff --git a/types/context.go b/types/context.go index 67c4b1bd..2e7ae92c 100644 --- a/types/context.go +++ b/types/context.go @@ -84,9 +84,6 @@ type Context struct { // Logging logger i18n.Logger DebugLevel int - - // ReadFileAndStoreInContext command - FileToRead string } func (ctx *Context) ExtractBuildOptions() properties.Map { From 73bae1128640ee152e8b05a0124f6b1f0518ca18 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 6 Jun 2017 19:35:35 +0200 Subject: [PATCH 06/21] Remove GCCPreprocSourceSaver This pass was unused. Signed-off-by: Matthijs Kooijman --- gcc_preproc_source_saver.go | 53 ------------------------------------- 1 file changed, 53 deletions(-) delete mode 100644 gcc_preproc_source_saver.go diff --git a/gcc_preproc_source_saver.go b/gcc_preproc_source_saver.go deleted file mode 100644 index 556cd276..00000000 --- a/gcc_preproc_source_saver.go +++ /dev/null @@ -1,53 +0,0 @@ -/* - * This file is part of Arduino Builder. - * - * Arduino Builder is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA - * - * As a special exception, you may use this file as part of a free software - * library without restriction. Specifically, if other files instantiate - * templates or use macros or inline functions from this file, or you compile - * this file and link it with other files to produce an executable, this - * file does not by itself cause the resulting executable to be covered by - * the GNU General Public License. This exception does not however - * invalidate any other reasons why the executable file might be covered by - * the GNU General Public License. - * - * Copyright 2015 Arduino LLC (http://www.arduino.cc/) - */ - -package builder - -// XXX: OBSOLETE? - -import ( - "github.com/arduino/arduino-builder/constants" - "github.com/arduino/arduino-builder/i18n" - "github.com/arduino/arduino-builder/types" - "github.com/arduino/arduino-builder/utils" - "path/filepath" -) - -type GCCPreprocSourceSaver struct{} - -func (s *GCCPreprocSourceSaver) Run(ctx *types.Context) error { - preprocPath := ctx.PreprocPath - err := utils.EnsureFolderExists(preprocPath) - if err != nil { - return i18n.WrapError(err) - } - - err = utils.WriteFile(filepath.Join(preprocPath, constants.FILE_GCC_PREPROC_TARGET), ctx.Source) - return i18n.WrapError(err) -} From b861a844fb4210bb49ba83c979f921cd223ae7d0 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 16 Jun 2017 13:00:13 +0200 Subject: [PATCH 07/21] execSizeRecipe: Fix typo in method name Signed-off-by: Matthijs Kooijman --- phases/sizer.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/phases/sizer.go b/phases/sizer.go index 7ea86997..b1bedf75 100644 --- a/phases/sizer.go +++ b/phases/sizer.go @@ -89,7 +89,7 @@ func checkSize(buildProperties properties.Map, verbose bool, warningsLevel strin } } - textSize, dataSize, _, err := execSizeReceipe(properties, logger) + textSize, dataSize, _, err := execSizeRecipe(properties, logger) if err != nil { logger.Println(constants.LOG_LEVEL_WARN, constants.MSG_SIZER_ERROR_NO_RULE) return nil @@ -127,7 +127,7 @@ func checkSize(buildProperties properties.Map, verbose bool, warningsLevel strin return nil } -func execSizeReceipe(properties properties.Map, logger i18n.Logger) (textSize int, dataSize int, eepromSize int, resErr error) { +func execSizeRecipe(properties properties.Map, logger i18n.Logger) (textSize int, dataSize int, eepromSize int, resErr error) { out, err := builder_utils.ExecRecipe(properties, constants.RECIPE_SIZE_PATTERN, false, false, false, logger) if err != nil { resErr = errors.New("Error while determining sketch size: " + err.Error()) From 108e65f0fe62de39ae4f05561a45d76352002a34 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 16 Jun 2017 13:18:55 +0200 Subject: [PATCH 08/21] Pass types.Context down into compilation helpers Previously, the verbose, logger and sometimes warningFlags were extracted from the Context by the top-level runner and passed down separately. Since this causes a lot of variable-passing of what is essentially global state, it is clearer to just pass a types.Context down and let the helpers get the data they need from that state. This prepared for a next commit where ExecRecipe will be refactored and needs access to the Context. Since the next commit will heavily change ExecRecipe anyway, this commit does not actually change ExecRecipe to accept the Context. This commit should not change any behaviour. Signed-off-by: Matthijs Kooijman --- builder_utils/utils.go | 39 ++++++++++++++++++++----------------- phases/core_builder.go | 18 ++++++++--------- phases/libraries_builder.go | 22 ++++++++++----------- phases/linker.go | 11 ++++------- phases/sizer.go | 16 +++++++-------- phases/sketch_builder.go | 7 ++----- 6 files changed, 52 insertions(+), 61 deletions(-) diff --git a/builder_utils/utils.go b/builder_utils/utils.go index 139ad770..f3d7aa81 100644 --- a/builder_utils/utils.go +++ b/builder_utils/utils.go @@ -40,12 +40,13 @@ import ( "github.com/arduino/arduino-builder/constants" "github.com/arduino/arduino-builder/i18n" + "github.com/arduino/arduino-builder/types" "github.com/arduino/arduino-builder/utils" "github.com/arduino/go-properties-map" ) -func CompileFilesRecursive(objectFiles []string, sourcePath string, buildPath string, buildProperties properties.Map, includes []string, verbose bool, warningsLevel string, logger i18n.Logger) ([]string, error) { - objectFiles, err := CompileFiles(objectFiles, sourcePath, false, buildPath, buildProperties, includes, verbose, warningsLevel, logger) +func CompileFilesRecursive(ctx *types.Context, objectFiles []string, sourcePath string, buildPath string, buildProperties properties.Map, includes []string) ([]string, error) { + objectFiles, err := CompileFiles(ctx, objectFiles, sourcePath, false, buildPath, buildProperties, includes) if err != nil { return nil, i18n.WrapError(err) } @@ -56,7 +57,7 @@ func CompileFilesRecursive(objectFiles []string, sourcePath string, buildPath st } for _, folder := range folders { - objectFiles, err = CompileFilesRecursive(objectFiles, filepath.Join(sourcePath, folder.Name()), filepath.Join(buildPath, folder.Name()), buildProperties, includes, verbose, warningsLevel, logger) + objectFiles, err = CompileFilesRecursive(ctx, objectFiles, filepath.Join(sourcePath, folder.Name()), filepath.Join(buildPath, folder.Name()), buildProperties, includes) if err != nil { return nil, i18n.WrapError(err) } @@ -65,28 +66,28 @@ func CompileFilesRecursive(objectFiles []string, sourcePath string, buildPath st return objectFiles, nil } -func CompileFiles(objectFiles []string, sourcePath string, recurse bool, buildPath string, buildProperties properties.Map, includes []string, verbose bool, warningsLevel string, logger i18n.Logger) ([]string, error) { - objectFiles, err := compileFilesWithExtensionWithRecipe(objectFiles, sourcePath, recurse, buildPath, buildProperties, includes, ".S", constants.RECIPE_S_PATTERN, verbose, warningsLevel, logger) +func CompileFiles(ctx *types.Context, objectFiles []string, sourcePath string, recurse bool, buildPath string, buildProperties properties.Map, includes []string) ([]string, error) { + objectFiles, err := compileFilesWithExtensionWithRecipe(ctx, objectFiles, sourcePath, recurse, buildPath, buildProperties, includes, ".S", constants.RECIPE_S_PATTERN) if err != nil { return nil, i18n.WrapError(err) } - objectFiles, err = compileFilesWithExtensionWithRecipe(objectFiles, sourcePath, recurse, buildPath, buildProperties, includes, ".c", constants.RECIPE_C_PATTERN, verbose, warningsLevel, logger) + objectFiles, err = compileFilesWithExtensionWithRecipe(ctx, objectFiles, sourcePath, recurse, buildPath, buildProperties, includes, ".c", constants.RECIPE_C_PATTERN) if err != nil { return nil, i18n.WrapError(err) } - objectFiles, err = compileFilesWithExtensionWithRecipe(objectFiles, sourcePath, recurse, buildPath, buildProperties, includes, ".cpp", constants.RECIPE_CPP_PATTERN, verbose, warningsLevel, logger) + objectFiles, err = compileFilesWithExtensionWithRecipe(ctx, objectFiles, sourcePath, recurse, buildPath, buildProperties, includes, ".cpp", constants.RECIPE_CPP_PATTERN) if err != nil { return nil, i18n.WrapError(err) } return objectFiles, nil } -func compileFilesWithExtensionWithRecipe(objectFiles []string, sourcePath string, recurse bool, buildPath string, buildProperties properties.Map, includes []string, extension string, recipe string, verbose bool, warningsLevel string, logger i18n.Logger) ([]string, error) { +func compileFilesWithExtensionWithRecipe(ctx *types.Context, objectFiles []string, sourcePath string, recurse bool, buildPath string, buildProperties properties.Map, includes []string, extension string, recipe string) ([]string, error) { sources, err := findFilesInFolder(sourcePath, extension, recurse) if err != nil { return nil, i18n.WrapError(err) } - return compileFilesWithRecipe(objectFiles, sourcePath, sources, buildPath, buildProperties, includes, recipe, verbose, warningsLevel, logger) + return compileFilesWithRecipe(ctx, objectFiles, sourcePath, sources, buildPath, buildProperties, includes, recipe) } func findFilesInFolder(sourcePath string, extension string, recurse bool) ([]string, error) { @@ -145,9 +146,9 @@ func findAllFilesInFolder(sourcePath string, recurse bool) ([]string, error) { return sources, nil } -func compileFilesWithRecipe(objectFiles []string, sourcePath string, sources []string, buildPath string, buildProperties properties.Map, includes []string, recipe string, verbose bool, warningsLevel string, logger i18n.Logger) ([]string, error) { +func compileFilesWithRecipe(ctx *types.Context, objectFiles []string, sourcePath string, sources []string, buildPath string, buildProperties properties.Map, includes []string, recipe string) ([]string, error) { for _, source := range sources { - objectFile, err := compileFileWithRecipe(sourcePath, source, buildPath, buildProperties, includes, recipe, verbose, warningsLevel, logger) + objectFile, err := compileFileWithRecipe(ctx, sourcePath, source, buildPath, buildProperties, includes, recipe) if err != nil { return nil, i18n.WrapError(err) } @@ -157,9 +158,10 @@ func compileFilesWithRecipe(objectFiles []string, sourcePath string, sources []s return objectFiles, nil } -func compileFileWithRecipe(sourcePath string, source string, buildPath string, buildProperties properties.Map, includes []string, recipe string, verbose bool, warningsLevel string, logger i18n.Logger) (string, error) { +func compileFileWithRecipe(ctx *types.Context, sourcePath string, source string, buildPath string, buildProperties properties.Map, includes []string, recipe string) (string, error) { + logger := ctx.GetLogger() properties := buildProperties.Clone() - properties[constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS] = properties[constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS+"."+warningsLevel] + properties[constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS] = properties[constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS+"."+ctx.WarningsLevel] properties[constants.BUILD_PROPERTIES_INCLUDES] = strings.Join(includes, constants.SPACE) properties[constants.BUILD_PROPERTIES_SOURCE_FILE] = source relativeSource, err := filepath.Rel(sourcePath, source) @@ -179,11 +181,11 @@ func compileFileWithRecipe(sourcePath string, source string, buildPath string, b } if !objIsUpToDate { - _, err = ExecRecipe(properties, recipe, false, verbose, verbose, logger) + _, err = ExecRecipe(properties, recipe, false, ctx.Verbose, ctx.Verbose, logger) if err != nil { return "", i18n.WrapError(err) } - } else if verbose { + } else if ctx.Verbose { logger.Println(constants.LOG_LEVEL_INFO, constants.MSG_USING_PREVIOUS_COMPILED_FILE, properties[constants.BUILD_PROPERTIES_OBJECT_FILE]) } @@ -309,7 +311,8 @@ func CoreOrReferencedCoreHasChanged(corePath, targetCorePath, targetFile string) return true } -func ArchiveCompiledFiles(buildPath string, archiveFile string, objectFiles []string, buildProperties properties.Map, verbose bool, logger i18n.Logger) (string, error) { +func ArchiveCompiledFiles(ctx *types.Context, buildPath string, archiveFile string, objectFiles []string, buildProperties properties.Map) (string, error) { + logger := ctx.GetLogger() archiveFilePath := filepath.Join(buildPath, archiveFile) rebuildArchive := false @@ -332,7 +335,7 @@ func ArchiveCompiledFiles(buildPath string, archiveFile string, objectFiles []st return "", i18n.WrapError(err) } } else { - if verbose { + if ctx.Verbose { logger.Println(constants.LOG_LEVEL_INFO, constants.MSG_USING_PREVIOUS_COMPILED_FILE, archiveFilePath) } return archiveFilePath, nil @@ -345,7 +348,7 @@ func ArchiveCompiledFiles(buildPath string, archiveFile string, objectFiles []st properties[constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH] = archiveFilePath properties[constants.BUILD_PROPERTIES_OBJECT_FILE] = objectFile - _, err := ExecRecipe(properties, constants.RECIPE_AR_PATTERN, false, verbose, verbose, logger) + _, err := ExecRecipe(properties, constants.RECIPE_AR_PATTERN, false, ctx.Verbose, ctx.Verbose, logger) if err != nil { return "", i18n.WrapError(err) } diff --git a/phases/core_builder.go b/phases/core_builder.go index 75b6535d..cecea3de 100644 --- a/phases/core_builder.go +++ b/phases/core_builder.go @@ -46,9 +46,6 @@ func (s *CoreBuilder) Run(ctx *types.Context) error { coreBuildPath := ctx.CoreBuildPath coreBuildCachePath := ctx.CoreBuildCachePath buildProperties := ctx.BuildProperties - verbose := ctx.Verbose - warningsLevel := ctx.WarningsLevel - logger := ctx.GetLogger() err := utils.EnsureFolderExists(coreBuildPath) if err != nil { @@ -62,7 +59,7 @@ func (s *CoreBuilder) Run(ctx *types.Context) error { } } - archiveFile, objectFiles, err := compileCore(coreBuildPath, coreBuildCachePath, buildProperties, verbose, warningsLevel, logger) + archiveFile, objectFiles, err := compileCore(ctx, coreBuildPath, coreBuildCachePath, buildProperties) if err != nil { return i18n.WrapError(err) } @@ -73,7 +70,8 @@ func (s *CoreBuilder) Run(ctx *types.Context) error { return nil } -func compileCore(buildPath string, buildCachePath string, buildProperties properties.Map, verbose bool, warningsLevel string, logger i18n.Logger) (string, []string, error) { +func compileCore(ctx *types.Context, buildPath string, buildCachePath string, buildProperties properties.Map) (string, []string, error) { + logger := ctx.GetLogger() coreFolder := buildProperties[constants.BUILD_PROPERTIES_BUILD_CORE_PATH] variantFolder := buildProperties[constants.BUILD_PROPERTIES_BUILD_VARIANT_PATH] @@ -90,7 +88,7 @@ func compileCore(buildPath string, buildCachePath string, buildProperties proper variantObjectFiles := []string{} if variantFolder != constants.EMPTY_STRING { - variantObjectFiles, err = builder_utils.CompileFiles(variantObjectFiles, variantFolder, true, buildPath, buildProperties, includes, verbose, warningsLevel, logger) + variantObjectFiles, err = builder_utils.CompileFiles(ctx, variantObjectFiles, variantFolder, true, buildPath, buildProperties, includes) if err != nil { return "", nil, i18n.WrapError(err) } @@ -107,26 +105,26 @@ func compileCore(buildPath string, buildCachePath string, buildProperties proper if canUseArchivedCore { // use archived core - if verbose { + if ctx.Verbose { logger.Println(constants.LOG_LEVEL_INFO, "Using precompiled core: {0}", targetArchivedCore) } return targetArchivedCore, variantObjectFiles, nil } } - coreObjectFiles, err := builder_utils.CompileFiles([]string{}, coreFolder, true, buildPath, buildProperties, includes, verbose, warningsLevel, logger) + coreObjectFiles, err := builder_utils.CompileFiles(ctx, []string{}, coreFolder, true, buildPath, buildProperties, includes) if err != nil { return "", nil, i18n.WrapError(err) } - archiveFile, err := builder_utils.ArchiveCompiledFiles(buildPath, "core.a", coreObjectFiles, buildProperties, verbose, logger) + archiveFile, err := builder_utils.ArchiveCompiledFiles(ctx, buildPath, "core.a", coreObjectFiles, buildProperties) if err != nil { return "", nil, i18n.WrapError(err) } // archive core.a if targetArchivedCore != "" { - if verbose { + if ctx.Verbose { logger.Println(constants.LOG_LEVEL_INFO, constants.MSG_ARCHIVING_CORE_CACHE, targetArchivedCore) } builder_utils.CopyFile(archiveFile, targetArchivedCore) diff --git a/phases/libraries_builder.go b/phases/libraries_builder.go index fda56b80..dcbcaa4b 100644 --- a/phases/libraries_builder.go +++ b/phases/libraries_builder.go @@ -52,16 +52,13 @@ func (s *LibrariesBuilder) Run(ctx *types.Context) error { includes := ctx.IncludeFolders includes = utils.Map(includes, utils.WrapWithHyphenI) libraries := ctx.ImportedLibraries - verbose := ctx.Verbose - warningsLevel := ctx.WarningsLevel - logger := ctx.GetLogger() err := utils.EnsureFolderExists(librariesBuildPath) if err != nil { return i18n.WrapError(err) } - objectFiles, err := compileLibraries(libraries, librariesBuildPath, buildProperties, includes, verbose, warningsLevel, logger) + objectFiles, err := compileLibraries(ctx, libraries, librariesBuildPath, buildProperties, includes) if err != nil { return i18n.WrapError(err) } @@ -99,10 +96,10 @@ func fixLDFLAGforPrecompiledLibraries(ctx *types.Context, libraries []*types.Lib return nil } -func compileLibraries(libraries []*types.Library, buildPath string, buildProperties properties.Map, includes []string, verbose bool, warningsLevel string, logger i18n.Logger) ([]string, error) { +func compileLibraries(ctx *types.Context, libraries []*types.Library, buildPath string, buildProperties properties.Map, includes []string) ([]string, error) { objectFiles := []string{} for _, library := range libraries { - libraryObjectFiles, err := compileLibrary(library, buildPath, buildProperties, includes, verbose, warningsLevel, logger) + libraryObjectFiles, err := compileLibrary(ctx, library, buildPath, buildProperties, includes) if err != nil { return nil, i18n.WrapError(err) } @@ -113,8 +110,9 @@ func compileLibraries(libraries []*types.Library, buildPath string, buildPropert } -func compileLibrary(library *types.Library, buildPath string, buildProperties properties.Map, includes []string, verbose bool, warningsLevel string, logger i18n.Logger) ([]string, error) { - if verbose { +func compileLibrary(ctx *types.Context, library *types.Library, buildPath string, buildProperties properties.Map, includes []string) ([]string, error) { + logger := ctx.GetLogger() + if ctx.Verbose { logger.Println(constants.LOG_LEVEL_INFO, "Compiling library \"{0}\"", library.Name) } libraryBuildPath := filepath.Join(buildPath, library.Name) @@ -144,12 +142,12 @@ func compileLibrary(library *types.Library, buildPath string, buildProperties pr } if library.Layout == types.LIBRARY_RECURSIVE { - objectFiles, err = builder_utils.CompileFilesRecursive(objectFiles, library.SrcFolder, libraryBuildPath, buildProperties, includes, verbose, warningsLevel, logger) + objectFiles, err = builder_utils.CompileFilesRecursive(ctx, objectFiles, library.SrcFolder, libraryBuildPath, buildProperties, includes) if err != nil { return nil, i18n.WrapError(err) } if library.DotALinkage { - archiveFile, err := builder_utils.ArchiveCompiledFiles(libraryBuildPath, library.Name+".a", objectFiles, buildProperties, verbose, logger) + archiveFile, err := builder_utils.ArchiveCompiledFiles(ctx, libraryBuildPath, library.Name+".a", objectFiles, buildProperties) if err != nil { return nil, i18n.WrapError(err) } @@ -159,14 +157,14 @@ func compileLibrary(library *types.Library, buildPath string, buildProperties pr if library.UtilityFolder != "" { includes = append(includes, utils.WrapWithHyphenI(library.UtilityFolder)) } - objectFiles, err = builder_utils.CompileFiles(objectFiles, library.SrcFolder, false, libraryBuildPath, buildProperties, includes, verbose, warningsLevel, logger) + objectFiles, err = builder_utils.CompileFiles(ctx, objectFiles, library.SrcFolder, false, libraryBuildPath, buildProperties, includes) if err != nil { return nil, i18n.WrapError(err) } if library.UtilityFolder != "" { utilityBuildPath := filepath.Join(libraryBuildPath, constants.LIBRARY_FOLDER_UTILITY) - objectFiles, err = builder_utils.CompileFiles(objectFiles, library.UtilityFolder, false, utilityBuildPath, buildProperties, includes, verbose, warningsLevel, logger) + objectFiles, err = builder_utils.CompileFiles(ctx, objectFiles, library.UtilityFolder, false, utilityBuildPath, buildProperties, includes) if err != nil { return nil, i18n.WrapError(err) } diff --git a/phases/linker.go b/phases/linker.go index d35f4993..9b3eb5c2 100644 --- a/phases/linker.go +++ b/phases/linker.go @@ -61,11 +61,8 @@ func (s *Linker) Run(ctx *types.Context) error { } buildProperties := ctx.BuildProperties - verbose := ctx.Verbose - warningsLevel := ctx.WarningsLevel - logger := ctx.GetLogger() - err = link(objectFiles, coreDotARelPath, coreArchiveFilePath, buildProperties, verbose, warningsLevel, logger) + err = link(ctx, objectFiles, coreDotARelPath, coreArchiveFilePath, buildProperties) if err != nil { return i18n.WrapError(err) } @@ -73,7 +70,7 @@ func (s *Linker) Run(ctx *types.Context) error { return nil } -func link(objectFiles []string, coreDotARelPath string, coreArchiveFilePath string, buildProperties properties.Map, verbose bool, warningsLevel string, logger i18n.Logger) error { +func link(ctx *types.Context, objectFiles []string, coreDotARelPath string, coreArchiveFilePath string, buildProperties properties.Map) error { optRelax := addRelaxTrickIfATMEGA2560(buildProperties) objectFiles = utils.Map(objectFiles, wrapWithDoubleQuotes) @@ -81,12 +78,12 @@ func link(objectFiles []string, coreDotARelPath string, coreArchiveFilePath stri properties := buildProperties.Clone() properties[constants.BUILD_PROPERTIES_COMPILER_C_ELF_FLAGS] = properties[constants.BUILD_PROPERTIES_COMPILER_C_ELF_FLAGS] + optRelax - properties[constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS] = properties[constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS+"."+warningsLevel] + properties[constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS] = properties[constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS+"."+ctx.WarningsLevel] properties[constants.BUILD_PROPERTIES_ARCHIVE_FILE] = coreDotARelPath properties[constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH] = coreArchiveFilePath properties[constants.BUILD_PROPERTIES_OBJECT_FILES] = objectFileList - _, err := builder_utils.ExecRecipe(properties, constants.RECIPE_C_COMBINE_PATTERN, false, verbose, verbose, logger) + _, err := builder_utils.ExecRecipe(properties, constants.RECIPE_C_COMBINE_PATTERN, false, ctx.Verbose, ctx.Verbose, ctx.GetLogger()) return err } diff --git a/phases/sizer.go b/phases/sizer.go index b1bedf75..94d771dc 100644 --- a/phases/sizer.go +++ b/phases/sizer.go @@ -52,11 +52,8 @@ func (s *Sizer) Run(ctx *types.Context) error { } buildProperties := ctx.BuildProperties - verbose := ctx.Verbose - warningsLevel := ctx.WarningsLevel - logger := ctx.GetLogger() - err := checkSize(buildProperties, verbose, warningsLevel, logger) + err := checkSize(ctx, buildProperties) if err != nil { return i18n.WrapError(err) } @@ -64,10 +61,11 @@ func (s *Sizer) Run(ctx *types.Context) error { return nil } -func checkSize(buildProperties properties.Map, verbose bool, warningsLevel string, logger i18n.Logger) error { +func checkSize(ctx *types.Context, buildProperties properties.Map) error { + logger := ctx.GetLogger() properties := buildProperties.Clone() - properties[constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS] = properties[constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS+"."+warningsLevel] + properties[constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS] = properties[constants.BUILD_PROPERTIES_COMPILER_WARNING_FLAGS+"."+ctx.WarningsLevel] maxTextSizeString := properties[constants.PROPERTY_UPLOAD_MAX_SIZE] maxDataSizeString := properties[constants.PROPERTY_UPLOAD_MAX_DATA_SIZE] @@ -89,7 +87,7 @@ func checkSize(buildProperties properties.Map, verbose bool, warningsLevel strin } } - textSize, dataSize, _, err := execSizeRecipe(properties, logger) + textSize, dataSize, _, err := execSizeRecipe(ctx, properties) if err != nil { logger.Println(constants.LOG_LEVEL_WARN, constants.MSG_SIZER_ERROR_NO_RULE) return nil @@ -127,8 +125,8 @@ func checkSize(buildProperties properties.Map, verbose bool, warningsLevel strin return nil } -func execSizeRecipe(properties properties.Map, logger i18n.Logger) (textSize int, dataSize int, eepromSize int, resErr error) { - out, err := builder_utils.ExecRecipe(properties, constants.RECIPE_SIZE_PATTERN, false, false, false, logger) +func execSizeRecipe(ctx *types.Context, properties properties.Map) (textSize int, dataSize int, eepromSize int, resErr error) { + out, err := builder_utils.ExecRecipe(properties, constants.RECIPE_SIZE_PATTERN, false, false, false, ctx.GetLogger()) if err != nil { resErr = errors.New("Error while determining sketch size: " + err.Error()) return diff --git a/phases/sketch_builder.go b/phases/sketch_builder.go index 04c7ded7..8a3158d9 100644 --- a/phases/sketch_builder.go +++ b/phases/sketch_builder.go @@ -46,9 +46,6 @@ func (s *SketchBuilder) Run(ctx *types.Context) error { buildProperties := ctx.BuildProperties includes := ctx.IncludeFolders includes = utils.Map(includes, utils.WrapWithHyphenI) - verbose := ctx.Verbose - warningsLevel := ctx.WarningsLevel - logger := ctx.GetLogger() err := utils.EnsureFolderExists(sketchBuildPath) if err != nil { @@ -56,7 +53,7 @@ func (s *SketchBuilder) Run(ctx *types.Context) error { } var objectFiles []string - objectFiles, err = builder_utils.CompileFiles(objectFiles, sketchBuildPath, false, sketchBuildPath, buildProperties, includes, verbose, warningsLevel, logger) + objectFiles, err = builder_utils.CompileFiles(ctx, objectFiles, sketchBuildPath, false, sketchBuildPath, buildProperties, includes) if err != nil { return i18n.WrapError(err) } @@ -64,7 +61,7 @@ func (s *SketchBuilder) Run(ctx *types.Context) error { // The "src/" subdirectory of a sketch is compiled recursively sketchSrcPath := filepath.Join(sketchBuildPath, constants.SKETCH_FOLDER_SRC) if info, err := os.Stat(sketchSrcPath); err == nil && info.IsDir() { - objectFiles, err = builder_utils.CompileFiles(objectFiles, sketchSrcPath, true, sketchSrcPath, buildProperties, includes, verbose, warningsLevel, logger) + objectFiles, err = builder_utils.CompileFiles(ctx, objectFiles, sketchSrcPath, true, sketchSrcPath, buildProperties, includes) if err != nil { return i18n.WrapError(err) } From 0f1baa5abe43f204c96364a4c5b30664a6df1424 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 16 Jun 2017 13:39:01 +0200 Subject: [PATCH 09/21] Show the sizer commandline in verbose mode For some reason the sizer commandline was never shown. For consistency, it is now shown in verbose mode, just like the other commands. Signed-off-by: Matthijs Kooijman --- phases/sizer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phases/sizer.go b/phases/sizer.go index 94d771dc..90d4e705 100644 --- a/phases/sizer.go +++ b/phases/sizer.go @@ -126,7 +126,7 @@ func checkSize(ctx *types.Context, buildProperties properties.Map) error { } func execSizeRecipe(ctx *types.Context, properties properties.Map) (textSize int, dataSize int, eepromSize int, resErr error) { - out, err := builder_utils.ExecRecipe(properties, constants.RECIPE_SIZE_PATTERN, false, false, false, ctx.GetLogger()) + out, err := builder_utils.ExecRecipe(properties, constants.RECIPE_SIZE_PATTERN, false, ctx.Verbose, false, ctx.GetLogger()) if err != nil { resErr = errors.New("Error while determining sketch size: " + err.Error()) return From 2254d74e74ea87518e531595711bea803ada218b Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 16 Jun 2017 13:47:47 +0200 Subject: [PATCH 10/21] Show stdout of preproc commands in verbose mode This also happens with the normal compilation commands, so why not with these? Normally these commands should not output to stdout, so this doesn't make any difference, but it makes things more consistent. Signed-off-by: Matthijs Kooijman --- gcc_preproc_runner.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc_preproc_runner.go b/gcc_preproc_runner.go index 0f95917f..8156e06e 100644 --- a/gcc_preproc_runner.go +++ b/gcc_preproc_runner.go @@ -53,7 +53,7 @@ func GCCPreprocRunner(ctx *types.Context, sourceFilePath string, targetFilePath verbose := ctx.Verbose logger := ctx.GetLogger() - _, err = builder_utils.ExecRecipe(properties, constants.RECIPE_PREPROC_MACROS, true, verbose, false, logger) + _, err = builder_utils.ExecRecipe(properties, constants.RECIPE_PREPROC_MACROS, true, verbose, verbose, logger) if err != nil { return i18n.WrapError(err) } @@ -75,7 +75,7 @@ func GCCPreprocRunnerForDiscoveringIncludes(ctx *types.Context, sourceFilePath s properties[constants.RECIPE_PREPROC_MACROS] = GeneratePreprocPatternFromCompile(properties[constants.RECIPE_CPP_PATTERN]) } - stderr, err := builder_utils.ExecRecipeCollectStdErr(properties, constants.RECIPE_PREPROC_MACROS, true, verbose, false, logger) + stderr, err := builder_utils.ExecRecipeCollectStdErr(properties, constants.RECIPE_PREPROC_MACROS, true, verbose, verbose, logger) if err != nil { return "", i18n.WrapError(err) } From 611f07a5df0efc8b5086a9635cc36d6f41b53678 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 16 Jun 2017 14:56:55 +0200 Subject: [PATCH 11/21] Do not ignore command errors in ExecRecipeCollectStdErr Previously, this function would ignore any errors returned by `Run()` since the command is expected to fail in most cases. However, in addition to ignoring a non-zero exit code from the command, it would also ignore errors in running the command itself. With this commit, `ExecRecipeCollectStdErr()` simply returns all errors, but its caller checks the type of the error. If it is `ExitError`, this indicates a non-zero exit status, which is ignored. Otherwise, the error is reported as normal. Signed-off-by: Matthijs Kooijman --- builder_utils/utils.go | 4 ++-- container_find_includes.go | 8 +++++++- gcc_preproc_runner.go | 2 +- i18n/errors.go | 11 +++++++++++ 4 files changed, 21 insertions(+), 4 deletions(-) diff --git a/builder_utils/utils.go b/builder_utils/utils.go index f3d7aa81..d6997c24 100644 --- a/builder_utils/utils.go +++ b/builder_utils/utils.go @@ -410,8 +410,8 @@ func ExecRecipeCollectStdErr(buildProperties properties.Map, recipe string, remo buffer := &bytes.Buffer{} command.Stderr = buffer - command.Run() - return string(buffer.Bytes()), nil + err = command.Run() + return string(buffer.Bytes()), err } func RemoveHyphenMDDFlagFromGCCCommandLine(buildProperties properties.Map) { diff --git a/container_find_includes.go b/container_find_includes.go index 91bafd7c..eb49d3ef 100644 --- a/container_find_includes.go +++ b/container_find_includes.go @@ -110,6 +110,7 @@ import ( "encoding/json" "io/ioutil" "os" + "os/exec" "path/filepath" "time" @@ -324,7 +325,12 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t } } else { stderr, err := GCCPreprocRunnerForDiscoveringIncludes(ctx, sourcePath, targetFilePath, includes) - if err != nil { + // 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) } include = IncludesFinderWithRegExp(ctx, stderr) diff --git a/gcc_preproc_runner.go b/gcc_preproc_runner.go index 8156e06e..2e3dbe1c 100644 --- a/gcc_preproc_runner.go +++ b/gcc_preproc_runner.go @@ -77,7 +77,7 @@ func GCCPreprocRunnerForDiscoveringIncludes(ctx *types.Context, sourceFilePath s stderr, err := builder_utils.ExecRecipeCollectStdErr(properties, constants.RECIPE_PREPROC_MACROS, true, verbose, verbose, logger) if err != nil { - return "", i18n.WrapError(err) + return string(stderr), i18n.WrapError(err) } return string(stderr), nil diff --git a/i18n/errors.go b/i18n/errors.go index ba2a86ea..b3617800 100644 --- a/i18n/errors.go +++ b/i18n/errors.go @@ -18,3 +18,14 @@ func WrapError(err error) error { } return errors.Wrap(err, 0) } + +func UnwrapError(err error) error { + // Perhaps go-errors can do this already in later versions? + // See https://github.com/go-errors/errors/issues/14 + switch e := err.(type) { + case *errors.Error: + return e.Err + default: + return err + } +} From 5eb37bc8a8b2a0463e750773af54b923ced51717 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 16 Jun 2017 15:25:15 +0200 Subject: [PATCH 12/21] Let ExecRecipeCollectStdErr return []byte for stderr Previously, the return value was converted to string. Letting callers convert to string makes it easier to write the sterr contents to os.Stderr again in a future commit. Signed-off-by: Matthijs Kooijman --- builder_utils/utils.go | 6 +++--- container_find_includes.go | 2 +- gcc_preproc_runner.go | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/builder_utils/utils.go b/builder_utils/utils.go index d6997c24..c18eb04b 100644 --- a/builder_utils/utils.go +++ b/builder_utils/utils.go @@ -402,16 +402,16 @@ func PrepareCommandForRecipe(buildProperties properties.Map, recipe string, remo return command, nil } -func ExecRecipeCollectStdErr(buildProperties properties.Map, recipe string, removeUnsetProperties bool, echoCommandLine bool, echoOutput bool, logger i18n.Logger) (string, error) { +func ExecRecipeCollectStdErr(buildProperties properties.Map, recipe string, removeUnsetProperties bool, echoCommandLine bool, echoOutput bool, logger i18n.Logger) ([]byte, error) { command, err := PrepareCommandForRecipe(buildProperties, recipe, removeUnsetProperties, echoCommandLine, echoOutput, logger) if err != nil { - return "", i18n.WrapError(err) + return nil, i18n.WrapError(err) } buffer := &bytes.Buffer{} command.Stderr = buffer err = command.Run() - return string(buffer.Bytes()), err + return buffer.Bytes(), err } func RemoveHyphenMDDFlagFromGCCCommandLine(buildProperties properties.Map) { diff --git a/container_find_includes.go b/container_find_includes.go index eb49d3ef..912e3711 100644 --- a/container_find_includes.go +++ b/container_find_includes.go @@ -333,7 +333,7 @@ func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile t if err != nil && !is_exit_error { return i18n.WrapError(err) } - include = IncludesFinderWithRegExp(ctx, stderr) + include = IncludesFinderWithRegExp(ctx, string(stderr)) } if include == "" { diff --git a/gcc_preproc_runner.go b/gcc_preproc_runner.go index 2e3dbe1c..65fc91a5 100644 --- a/gcc_preproc_runner.go +++ b/gcc_preproc_runner.go @@ -61,10 +61,10 @@ func GCCPreprocRunner(ctx *types.Context, sourceFilePath string, targetFilePath return nil } -func GCCPreprocRunnerForDiscoveringIncludes(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) (string, error) { +func GCCPreprocRunnerForDiscoveringIncludes(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) ([]byte, error) { properties, err := prepareGCCPreprocRecipeProperties(ctx, sourceFilePath, targetFilePath, includes) if err != nil { - return "", i18n.WrapError(err) + return nil, i18n.WrapError(err) } verbose := ctx.Verbose @@ -77,10 +77,10 @@ func GCCPreprocRunnerForDiscoveringIncludes(ctx *types.Context, sourceFilePath s stderr, err := builder_utils.ExecRecipeCollectStdErr(properties, constants.RECIPE_PREPROC_MACROS, true, verbose, verbose, logger) if err != nil { - return string(stderr), i18n.WrapError(err) + return stderr, i18n.WrapError(err) } - return string(stderr), nil + return stderr, nil } func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) (properties.Map, error) { From ca9008e767770ba493e89b2fb0455fcc31553bd1 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 16 Jun 2017 15:46:04 +0200 Subject: [PATCH 13/21] 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 --- container_find_includes.go | 44 +++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/container_find_includes.go b/container_find_includes.go index 912e3711..427e0177 100644 --- a/container_find_includes.go +++ b/container_find_includes.go @@ -119,6 +119,8 @@ import ( "github.com/arduino/arduino-builder/i18n" "github.com/arduino/arduino-builder/types" "github.com/arduino/arduino-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 From 60265b5ad37f2aa907fd7347c6c70fecb80d22b1 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 16 Jun 2017 17:01:22 +0200 Subject: [PATCH 14/21] Merge ExecRecipeCollectStdErr into ExecRecipe This unifies these similar methods into a single method. The interface is additionally changed to: - Accepts a `Context` argument. - Allow for defaults and named arguments, using an `ExecOptions` struct that is passed as an argument. - Allow configuring command output handling in a flexible way. Instead of passing bools for some specific configurations, you can now pass either `Ignore`, `Show` or `Capture` for both stdout and stderr independently. By default, stdout is is shown when verbose is true, or ignored when verbose is false. Stderr is shown by default. - Actually redirect stdout to `/dev/null` when it is not needed (by leaving `command.Stdout` at nil). Previously, `ExecRecipe` would either show or capture stdout, and the captured output was usually just thrown away. To allow for even more reuse, the biggest part of `ExecRecipe` is extracted into a new `utils.ExecCommand()` function which executes an arbitrary `exec.Cmd` object, with configurable output redirection. Signed-off-by: Matthijs Kooijman --- builder_utils/utils.go | 44 ++++++++++-------------------------------- gcc_preproc_runner.go | 9 ++------- phases/linker.go | 2 +- phases/sizer.go | 3 ++- recipe_runner.go | 5 ++--- utils/utils.go | 41 +++++++++++++++++++++++++++++++++++++++ 6 files changed, 58 insertions(+), 46 deletions(-) diff --git a/builder_utils/utils.go b/builder_utils/utils.go index c18eb04b..0a86d282 100644 --- a/builder_utils/utils.go +++ b/builder_utils/utils.go @@ -30,7 +30,6 @@ package builder_utils import ( - "bytes" "fmt" "io" "os" @@ -181,7 +180,7 @@ func compileFileWithRecipe(ctx *types.Context, sourcePath string, source string, } if !objIsUpToDate { - _, err = ExecRecipe(properties, recipe, false, ctx.Verbose, ctx.Verbose, logger) + _, _, err = ExecRecipe(ctx, properties, recipe, false, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Show) if err != nil { return "", i18n.WrapError(err) } @@ -348,7 +347,7 @@ func ArchiveCompiledFiles(ctx *types.Context, buildPath string, archiveFile stri properties[constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH] = archiveFilePath properties[constants.BUILD_PROPERTIES_OBJECT_FILE] = objectFile - _, err := ExecRecipe(properties, constants.RECIPE_AR_PATTERN, false, ctx.Verbose, ctx.Verbose, logger) + _, _, err := ExecRecipe(ctx, properties, constants.RECIPE_AR_PATTERN, false, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Show) if err != nil { return "", i18n.WrapError(err) } @@ -357,28 +356,17 @@ func ArchiveCompiledFiles(ctx *types.Context, buildPath string, archiveFile stri return archiveFilePath, nil } -func ExecRecipe(properties properties.Map, recipe string, removeUnsetProperties bool, echoCommandLine bool, echoOutput bool, logger i18n.Logger) ([]byte, error) { - command, err := PrepareCommandForRecipe(properties, recipe, removeUnsetProperties, echoCommandLine, echoOutput, logger) +// See util.ExecCommand for stdout/stderr arguments +func ExecRecipe(ctx *types.Context, buildProperties properties.Map, recipe string, removeUnsetProperties bool, stdout int, stderr int) ([]byte, []byte, error) { + command, err := PrepareCommandForRecipe(ctx, buildProperties, recipe, removeUnsetProperties) if err != nil { - return nil, i18n.WrapError(err) - } - - if echoOutput { - command.Stdout = os.Stdout - } - - command.Stderr = os.Stderr - - if echoOutput { - err := command.Run() - return nil, i18n.WrapError(err) + return nil, nil, i18n.WrapError(err) } - - bytes, err := command.Output() - return bytes, i18n.WrapError(err) + return utils.ExecCommand(ctx, command, stdout, stderr) } -func PrepareCommandForRecipe(buildProperties properties.Map, recipe string, removeUnsetProperties bool, echoCommandLine bool, echoOutput bool, logger i18n.Logger) (*exec.Cmd, error) { +func PrepareCommandForRecipe(ctx *types.Context, buildProperties properties.Map, recipe string, removeUnsetProperties bool) (*exec.Cmd, error) { + logger := ctx.GetLogger() pattern := buildProperties[recipe] if pattern == constants.EMPTY_STRING { return nil, i18n.ErrorfWithLogger(logger, constants.MSG_PATTERN_MISSING, recipe) @@ -395,25 +383,13 @@ func PrepareCommandForRecipe(buildProperties properties.Map, recipe string, remo return nil, i18n.WrapError(err) } - if echoCommandLine { + if ctx.Verbose { fmt.Println(commandLine) } return command, nil } -func ExecRecipeCollectStdErr(buildProperties properties.Map, recipe string, removeUnsetProperties bool, echoCommandLine bool, echoOutput bool, logger i18n.Logger) ([]byte, error) { - command, err := PrepareCommandForRecipe(buildProperties, recipe, removeUnsetProperties, echoCommandLine, echoOutput, logger) - if err != nil { - return nil, i18n.WrapError(err) - } - - buffer := &bytes.Buffer{} - command.Stderr = buffer - err = command.Run() - return buffer.Bytes(), err -} - func RemoveHyphenMDDFlagFromGCCCommandLine(buildProperties properties.Map) { buildProperties[constants.BUILD_PROPERTIES_COMPILER_CPP_FLAGS] = strings.Replace(buildProperties[constants.BUILD_PROPERTIES_COMPILER_CPP_FLAGS], "-MMD", "", -1) } diff --git a/gcc_preproc_runner.go b/gcc_preproc_runner.go index 65fc91a5..821a496a 100644 --- a/gcc_preproc_runner.go +++ b/gcc_preproc_runner.go @@ -51,9 +51,7 @@ func GCCPreprocRunner(ctx *types.Context, sourceFilePath string, targetFilePath properties[constants.RECIPE_PREPROC_MACROS] = GeneratePreprocPatternFromCompile(properties[constants.RECIPE_CPP_PATTERN]) } - verbose := ctx.Verbose - logger := ctx.GetLogger() - _, err = builder_utils.ExecRecipe(properties, constants.RECIPE_PREPROC_MACROS, true, verbose, verbose, logger) + _, _, err = builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_PREPROC_MACROS, true, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Show) if err != nil { return i18n.WrapError(err) } @@ -67,15 +65,12 @@ func GCCPreprocRunnerForDiscoveringIncludes(ctx *types.Context, sourceFilePath s return nil, i18n.WrapError(err) } - verbose := ctx.Verbose - logger := ctx.GetLogger() - if properties[constants.RECIPE_PREPROC_MACROS] == constants.EMPTY_STRING { //generate PREPROC_MACROS from RECIPE_CPP_PATTERN properties[constants.RECIPE_PREPROC_MACROS] = GeneratePreprocPatternFromCompile(properties[constants.RECIPE_CPP_PATTERN]) } - stderr, err := builder_utils.ExecRecipeCollectStdErr(properties, constants.RECIPE_PREPROC_MACROS, true, verbose, verbose, logger) + _, stderr, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_PREPROC_MACROS, true, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Capture) if err != nil { return stderr, i18n.WrapError(err) } diff --git a/phases/linker.go b/phases/linker.go index 9b3eb5c2..2514b406 100644 --- a/phases/linker.go +++ b/phases/linker.go @@ -83,7 +83,7 @@ func link(ctx *types.Context, objectFiles []string, coreDotARelPath string, core properties[constants.BUILD_PROPERTIES_ARCHIVE_FILE_PATH] = coreArchiveFilePath properties[constants.BUILD_PROPERTIES_OBJECT_FILES] = objectFileList - _, err := builder_utils.ExecRecipe(properties, constants.RECIPE_C_COMBINE_PATTERN, false, ctx.Verbose, ctx.Verbose, ctx.GetLogger()) + _, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_C_COMBINE_PATTERN, false, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Show) return err } diff --git a/phases/sizer.go b/phases/sizer.go index 90d4e705..fa719e18 100644 --- a/phases/sizer.go +++ b/phases/sizer.go @@ -38,6 +38,7 @@ import ( "github.com/arduino/arduino-builder/constants" "github.com/arduino/arduino-builder/i18n" "github.com/arduino/arduino-builder/types" + "github.com/arduino/arduino-builder/utils" "github.com/arduino/go-properties-map" ) @@ -126,7 +127,7 @@ func checkSize(ctx *types.Context, buildProperties properties.Map) error { } func execSizeRecipe(ctx *types.Context, properties properties.Map) (textSize int, dataSize int, eepromSize int, resErr error) { - out, err := builder_utils.ExecRecipe(properties, constants.RECIPE_SIZE_PATTERN, false, ctx.Verbose, false, ctx.GetLogger()) + out, _, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_SIZE_PATTERN, false /* stdout */, utils.Capture /* stderr */, utils.Show) if err != nil { resErr = errors.New("Error while determining sketch size: " + err.Error()) return diff --git a/recipe_runner.go b/recipe_runner.go index bec23f90..ace82ade 100644 --- a/recipe_runner.go +++ b/recipe_runner.go @@ -34,6 +34,7 @@ import ( "github.com/arduino/arduino-builder/constants" "github.com/arduino/arduino-builder/i18n" "github.com/arduino/arduino-builder/types" + "github.com/arduino/arduino-builder/utils" "os" "sort" "strings" @@ -51,8 +52,6 @@ func (s *RecipeByPrefixSuffixRunner) Run(ctx *types.Context) error { } buildProperties := ctx.BuildProperties.Clone() - verbose := ctx.Verbose - recipes := findRecipes(buildProperties, s.Prefix, s.Suffix) properties := buildProperties.Clone() @@ -60,7 +59,7 @@ func (s *RecipeByPrefixSuffixRunner) Run(ctx *types.Context) error { if ctx.DebugLevel >= 10 { logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, constants.MSG_RUNNING_RECIPE, recipe) } - _, err := builder_utils.ExecRecipe(properties, recipe, false, verbose, verbose, logger) + _, _, err := builder_utils.ExecRecipe(ctx, properties, recipe, false /* stdout */, utils.ShowIfVerbose /* stderr */, utils.Show) if err != nil { return i18n.WrapError(err) } diff --git a/utils/utils.go b/utils/utils.go index 318b3984..726d344e 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -30,6 +30,7 @@ package utils import ( + "bytes" "crypto/md5" "encoding/hex" "io/ioutil" @@ -254,6 +255,46 @@ func PrepareCommand(pattern string, logger i18n.Logger) (*exec.Cmd, error) { return PrepareCommandFilteredArgs(pattern, filterEmptyArg, logger) } +const ( + Ignore = 0 // Redirect to null + Show = 1 // Show on stdout/stderr as normal + ShowIfVerbose = 2 // Show if verbose is set, Ignore otherwise + Capture = 3 // Capture into buffer +) + +func ExecCommand(ctx *types.Context, command *exec.Cmd, stdout int, stderr int) ([]byte, []byte, error) { + if stdout == Capture { + buffer := &bytes.Buffer{} + command.Stdout = buffer + } else if stdout == Show || stdout == ShowIfVerbose && ctx.Verbose { + command.Stdout = os.Stdout + } + + if stderr == Capture { + buffer := &bytes.Buffer{} + command.Stderr = buffer + } else if stderr == Show || stderr == ShowIfVerbose && ctx.Verbose { + command.Stderr = os.Stderr + } + + err := command.Start() + if err != nil { + return nil, nil, i18n.WrapError(err) + } + + err = command.Wait() + + var outbytes, errbytes []byte + if buf, ok := command.Stdout.(*bytes.Buffer); ok { + outbytes = buf.Bytes() + } + if buf, ok := command.Stderr.(*bytes.Buffer); ok { + errbytes = buf.Bytes() + } + + return outbytes, errbytes, i18n.WrapError(err) +} + func MapHas(aMap map[string]interface{}, key string) bool { _, ok := aMap[key] return ok From 31e42b7f977fe07e666572d49b24516fb72c6cf4 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 16 Jun 2017 17:50:13 +0200 Subject: [PATCH 15/21] Merge some duplicate code into prepareGCCPreprocRecipeProperties Signed-off-by: Matthijs Kooijman --- gcc_preproc_runner.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/gcc_preproc_runner.go b/gcc_preproc_runner.go index 821a496a..5ba285fe 100644 --- a/gcc_preproc_runner.go +++ b/gcc_preproc_runner.go @@ -46,11 +46,6 @@ func GCCPreprocRunner(ctx *types.Context, sourceFilePath string, targetFilePath return i18n.WrapError(err) } - if properties[constants.RECIPE_PREPROC_MACROS] == constants.EMPTY_STRING { - //generate PREPROC_MACROS from RECIPE_CPP_PATTERN - properties[constants.RECIPE_PREPROC_MACROS] = GeneratePreprocPatternFromCompile(properties[constants.RECIPE_CPP_PATTERN]) - } - _, _, err = builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_PREPROC_MACROS, true, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Show) if err != nil { return i18n.WrapError(err) @@ -65,11 +60,6 @@ func GCCPreprocRunnerForDiscoveringIncludes(ctx *types.Context, sourceFilePath s return nil, i18n.WrapError(err) } - if properties[constants.RECIPE_PREPROC_MACROS] == constants.EMPTY_STRING { - //generate PREPROC_MACROS from RECIPE_CPP_PATTERN - properties[constants.RECIPE_PREPROC_MACROS] = GeneratePreprocPatternFromCompile(properties[constants.RECIPE_CPP_PATTERN]) - } - _, stderr, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_PREPROC_MACROS, true, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Capture) if err != nil { return stderr, i18n.WrapError(err) @@ -87,6 +77,11 @@ func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath string properties[constants.BUILD_PROPERTIES_INCLUDES] = strings.Join(includes, constants.SPACE) builder_utils.RemoveHyphenMDDFlagFromGCCCommandLine(properties) + if properties[constants.RECIPE_PREPROC_MACROS] == constants.EMPTY_STRING { + //generate PREPROC_MACROS from RECIPE_CPP_PATTERN + properties[constants.RECIPE_PREPROC_MACROS] = GeneratePreprocPatternFromCompile(properties[constants.RECIPE_CPP_PATTERN]) + } + return properties, nil } From b03ecabd68c8e0289ae606528fac81c854d5d764 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 16 Jun 2017 20:10:31 +0200 Subject: [PATCH 16/21] Let utils.ExecCommand print the command in verbose mode Previously, the command was printed by PrepareCommandForRecipe. Letting ExecCommand print seems more accurate, since it is only printed when it is actually run (though this already happened in practice). Additionally, the command can now be modified between PrepareCommandForRecipe and ExecCommand while preserving correct output. Since ExecCommand deals with a slice of arguments instead of a single command string, this requires merging them together into a proper commandline. Some care is taken to quote arguments containing spaces, quotes or backslashes, though this is mostly intended for display purposes. Arguments are only quoted when needed, regardless of whether they were quoted in the original pattern. Signed-off-by: Matthijs Kooijman --- builder_utils/utils.go | 5 ----- test/utils_test.go | 19 +++++++++++++++++++ utils/utils.go | 23 +++++++++++++++++++++++ 3 files changed, 42 insertions(+), 5 deletions(-) diff --git a/builder_utils/utils.go b/builder_utils/utils.go index 0a86d282..a51b0c97 100644 --- a/builder_utils/utils.go +++ b/builder_utils/utils.go @@ -30,7 +30,6 @@ package builder_utils import ( - "fmt" "io" "os" "os/exec" @@ -383,10 +382,6 @@ func PrepareCommandForRecipe(ctx *types.Context, buildProperties properties.Map, return nil, i18n.WrapError(err) } - if ctx.Verbose { - fmt.Println(commandLine) - } - return command, nil } diff --git a/test/utils_test.go b/test/utils_test.go index da8395fa..6ada0c29 100644 --- a/test/utils_test.go +++ b/test/utils_test.go @@ -70,6 +70,25 @@ func TestCommandLineParser(t *testing.T) { require.Equal(t, "/tmp/sketch321469072.cpp", parts[22]) } +func TestPrintableCommand(t *testing.T) { + parts := []string{ + "/path/to/dir with spaces/cmd", + "arg1", + "arg-\"with\"-quotes", + "specialchar-`~!@#$%^&*()-_=+[{]}\\|;:'\",<.>/?-argument", + "arg with spaces", + "arg\twith\t\ttabs", + "lastarg", + } + correct := "\"/path/to/dir with spaces/cmd\"" + + " arg1 \"arg-\\\"with\\\"-quotes\"" + + " \"specialchar-`~!@#$%^&*()-_=+[{]}\\\\|;:'\\\",<.>/?-argument\"" + + " \"arg with spaces\" \"arg\twith\t\ttabs\"" + + " lastarg" + result := utils.PrintableCommand(parts) + require.Equal(t, correct, result) +} + func TestCommandLineParserError(t *testing.T) { command := "\"command missing quote" diff --git a/utils/utils.go b/utils/utils.go index 726d344e..21e6241e 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -30,6 +30,7 @@ package utils import ( + "fmt" "bytes" "crypto/md5" "encoding/hex" @@ -255,6 +256,24 @@ func PrepareCommand(pattern string, logger i18n.Logger) (*exec.Cmd, error) { return PrepareCommandFilteredArgs(pattern, filterEmptyArg, logger) } +func printableArgument(arg string) string { + if strings.ContainsAny(arg, "\"\\ \t") { + arg = strings.Replace(arg, "\\", "\\\\", -1) + arg = strings.Replace(arg, "\"", "\\\"", -1) + return "\"" + arg + "\"" + } else { + return arg + } +} + +// Convert a command and argument slice back to a printable string. +// This adds basic escaping which is sufficient for debug output, but +// probably not for shell interpretation. This essentially reverses +// ParseCommandLine. +func PrintableCommand(parts []string) string { + return strings.Join(Map(parts, printableArgument), " ") +} + const ( Ignore = 0 // Redirect to null Show = 1 // Show on stdout/stderr as normal @@ -263,6 +282,10 @@ const ( ) func ExecCommand(ctx *types.Context, command *exec.Cmd, stdout int, stderr int) ([]byte, []byte, error) { + if ctx.Verbose { + fmt.Println(PrintableCommand(command.Args)) + } + if stdout == Capture { buffer := &bytes.Buffer{} command.Stdout = buffer From 9abc875b72e3e1438d140d9406f11f0fc96afa26 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 16 Jun 2017 20:18:16 +0200 Subject: [PATCH 17/21] Fix removal of -MMD option when running the preprocessor Usually, the `preproc.macros` recipe includes the C/C++ flags, and through that the `-MMD` flag to generate dependency files. However, since include detection passed an output file of `/dev/null` (or the equivalent on other operating systems), this causes gcc to try and generate a `/dev/null.d` file and fail. To prevent this, the `-MMD` flag was filtered out, but this filtering was applied to the `compiler.cpp.flags` variable, where it *usually* comes from. However, this is not necessarily true for all platforms. For example, the PIC32 platform used to have this flag in the `compiler.c.flags` variable and have `compiler.cpp.flags` include that. This prevented the flag from being filtered away and caused a failure. Due to previous changes, it is now possible for this filtering to happen after all variables have been replaced and the command to run was generated, but before actually running it. An extra advantage is that the filtering is more robust than the previous substring-based filtering. This fixes #230. Signed-off-by: Matthijs Kooijman --- builder_utils/utils.go | 4 ---- gcc_preproc_runner.go | 24 ++++++++++++++++-------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/builder_utils/utils.go b/builder_utils/utils.go index a51b0c97..82dd24b6 100644 --- a/builder_utils/utils.go +++ b/builder_utils/utils.go @@ -385,10 +385,6 @@ func PrepareCommandForRecipe(ctx *types.Context, buildProperties properties.Map, return command, nil } -func RemoveHyphenMDDFlagFromGCCCommandLine(buildProperties properties.Map) { - buildProperties[constants.BUILD_PROPERTIES_COMPILER_CPP_FLAGS] = strings.Replace(buildProperties[constants.BUILD_PROPERTIES_COMPILER_CPP_FLAGS], "-MMD", "", -1) -} - // CopyFile copies the contents of the file named src to the file named // by dst. The file will be created if it does not already exist. If the // destination file exists, all it's contents will be replaced by the contents diff --git a/gcc_preproc_runner.go b/gcc_preproc_runner.go index 5ba285fe..3c801f89 100644 --- a/gcc_preproc_runner.go +++ b/gcc_preproc_runner.go @@ -30,6 +30,7 @@ package builder import ( + "os/exec" "strings" "github.com/arduino/arduino-builder/builder_utils" @@ -37,16 +38,15 @@ import ( "github.com/arduino/arduino-builder/i18n" "github.com/arduino/arduino-builder/types" "github.com/arduino/arduino-builder/utils" - "github.com/arduino/go-properties-map" ) func GCCPreprocRunner(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) error { - properties, err := prepareGCCPreprocRecipeProperties(ctx, sourceFilePath, targetFilePath, includes) + cmd, err := prepareGCCPreprocRecipeProperties(ctx, sourceFilePath, targetFilePath, includes) if err != nil { return i18n.WrapError(err) } - _, _, err = builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_PREPROC_MACROS, true, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Show) + _, _, err = utils.ExecCommand(ctx, cmd /* stdout */, utils.ShowIfVerbose /* stderr */, utils.Show) if err != nil { return i18n.WrapError(err) } @@ -55,12 +55,12 @@ func GCCPreprocRunner(ctx *types.Context, sourceFilePath string, targetFilePath } func GCCPreprocRunnerForDiscoveringIncludes(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) ([]byte, error) { - properties, err := prepareGCCPreprocRecipeProperties(ctx, sourceFilePath, targetFilePath, includes) + cmd, err := prepareGCCPreprocRecipeProperties(ctx, sourceFilePath, targetFilePath, includes) if err != nil { return nil, i18n.WrapError(err) } - _, stderr, err := builder_utils.ExecRecipe(ctx, properties, constants.RECIPE_PREPROC_MACROS, true, /* stdout */ utils.ShowIfVerbose, /* stderr */ utils.Capture) + _, stderr, err := utils.ExecCommand(ctx, cmd /* stdout */, utils.ShowIfVerbose /* stderr */, utils.Capture) if err != nil { return stderr, i18n.WrapError(err) } @@ -68,21 +68,29 @@ func GCCPreprocRunnerForDiscoveringIncludes(ctx *types.Context, sourceFilePath s return stderr, nil } -func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) (properties.Map, error) { +func prepareGCCPreprocRecipeProperties(ctx *types.Context, sourceFilePath string, targetFilePath string, includes []string) (*exec.Cmd, error) { properties := ctx.BuildProperties.Clone() properties[constants.BUILD_PROPERTIES_SOURCE_FILE] = sourceFilePath properties[constants.BUILD_PROPERTIES_PREPROCESSED_FILE_PATH] = targetFilePath includes = utils.Map(includes, utils.WrapWithHyphenI) properties[constants.BUILD_PROPERTIES_INCLUDES] = strings.Join(includes, constants.SPACE) - builder_utils.RemoveHyphenMDDFlagFromGCCCommandLine(properties) if properties[constants.RECIPE_PREPROC_MACROS] == constants.EMPTY_STRING { //generate PREPROC_MACROS from RECIPE_CPP_PATTERN properties[constants.RECIPE_PREPROC_MACROS] = GeneratePreprocPatternFromCompile(properties[constants.RECIPE_CPP_PATTERN]) } - return properties, nil + cmd, err := builder_utils.PrepareCommandForRecipe(ctx, properties, constants.RECIPE_PREPROC_MACROS, true) + if err != nil { + return nil, i18n.WrapError(err) + } + + // Remove -MMD argument if present. Leaving it will make gcc try + // to create a /dev/null.d dependency file, which won't work. + cmd.Args = utils.Filter(cmd.Args, func(a string) bool { return a != "-MMD" }) + + return cmd, nil } func GeneratePreprocPatternFromCompile(compilePattern string) string { From 788cecdeba8b131c57a64acb9cc3a6cdb217060a Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 16 Jun 2017 21:12:30 +0200 Subject: [PATCH 18/21] Use utils.ExecCommand for running ctags This slightly simplifies the code. Signed-off-by: Matthijs Kooijman --- ctags_runner.go | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/ctags_runner.go b/ctags_runner.go index 82e4165c..18850ca1 100644 --- a/ctags_runner.go +++ b/ctags_runner.go @@ -30,8 +30,6 @@ package builder import ( - "fmt" - "github.com/arduino/arduino-builder/constants" "github.com/arduino/arduino-builder/ctags" "github.com/arduino/arduino-builder/i18n" @@ -61,12 +59,7 @@ func (s *CTagsRunner) Run(ctx *types.Context) error { return i18n.WrapError(err) } - verbose := ctx.Verbose - if verbose { - fmt.Println(commandLine) - } - - sourceBytes, err := command.Output() + sourceBytes, _, err := utils.ExecCommand(ctx, command /* stdout */, utils.Capture /* stderr */, utils.Ignore) if err != nil { return i18n.WrapError(err) } From cc978ec99313fa556e9d4c39c84a77c175efa95c Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 30 Nov 2017 15:23:04 +0100 Subject: [PATCH 19/21] Pass Context to ObjFileIsUpToDate This does not use the passed variable yet, but prepares for a future commit. Signed-off-by: Matthijs Kooijman --- builder_utils/utils.go | 4 ++-- container_find_includes.go | 2 +- test/builder_utils_test.go | 29 ++++++++++++++++++++++------- 3 files changed, 25 insertions(+), 10 deletions(-) diff --git a/builder_utils/utils.go b/builder_utils/utils.go index 82dd24b6..834364e7 100644 --- a/builder_utils/utils.go +++ b/builder_utils/utils.go @@ -173,7 +173,7 @@ func compileFileWithRecipe(ctx *types.Context, sourcePath string, source string, return "", i18n.WrapError(err) } - objIsUpToDate, err := ObjFileIsUpToDate(properties[constants.BUILD_PROPERTIES_SOURCE_FILE], properties[constants.BUILD_PROPERTIES_OBJECT_FILE], filepath.Join(buildPath, relativeSource+".d")) + objIsUpToDate, err := ObjFileIsUpToDate(ctx, properties[constants.BUILD_PROPERTIES_SOURCE_FILE], properties[constants.BUILD_PROPERTIES_OBJECT_FILE], filepath.Join(buildPath, relativeSource+".d")) if err != nil { return "", i18n.WrapError(err) } @@ -190,7 +190,7 @@ func compileFileWithRecipe(ctx *types.Context, sourcePath string, source string, return properties[constants.BUILD_PROPERTIES_OBJECT_FILE], nil } -func ObjFileIsUpToDate(sourceFile, objectFile, dependencyFile string) (bool, error) { +func ObjFileIsUpToDate(ctx *types.Context, sourceFile, objectFile, dependencyFile string) (bool, error) { sourceFile = filepath.Clean(sourceFile) objectFile = filepath.Clean(objectFile) dependencyFile = filepath.Clean(dependencyFile) diff --git a/container_find_includes.go b/container_find_includes.go index 427e0177..15cdc3b9 100644 --- a/container_find_includes.go +++ b/container_find_includes.go @@ -306,7 +306,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.ObjFileIsUpToDate(sourcePath, sourceFile.ObjectPath(ctx), sourceFile.DepfilePath(ctx)) + unchanged, err := builder_utils.ObjFileIsUpToDate(ctx, sourcePath, sourceFile.ObjectPath(ctx), sourceFile.DepfilePath(ctx)) if err != nil { return i18n.WrapError(err) } diff --git a/test/builder_utils_test.go b/test/builder_utils_test.go index c6fc941c..32ff14dc 100644 --- a/test/builder_utils_test.go +++ b/test/builder_utils_test.go @@ -31,6 +31,7 @@ package test import ( "github.com/arduino/arduino-builder/builder_utils" + "github.com/arduino/arduino-builder/types" "github.com/arduino/arduino-builder/utils" "github.com/stretchr/testify/require" "io/ioutil" @@ -52,27 +53,33 @@ func tempFile(t *testing.T, prefix string) string { } func TestObjFileIsUpToDateObjMissing(t *testing.T) { + ctx := &types.Context{} + sourceFile := tempFile(t, "source") defer os.RemoveAll(sourceFile) - upToDate, err := builder_utils.ObjFileIsUpToDate(sourceFile, "", "") + upToDate, err := builder_utils.ObjFileIsUpToDate(ctx, sourceFile, "", "") NoError(t, err) require.False(t, upToDate) } func TestObjFileIsUpToDateDepMissing(t *testing.T) { + ctx := &types.Context{} + sourceFile := tempFile(t, "source") defer os.RemoveAll(sourceFile) objFile := tempFile(t, "obj") defer os.RemoveAll(objFile) - upToDate, err := builder_utils.ObjFileIsUpToDate(sourceFile, objFile, "") + upToDate, err := builder_utils.ObjFileIsUpToDate(ctx, sourceFile, objFile, "") NoError(t, err) require.False(t, upToDate) } func TestObjFileIsUpToDateObjOlder(t *testing.T) { + ctx := &types.Context{} + objFile := tempFile(t, "obj") defer os.RemoveAll(objFile) depFile := tempFile(t, "dep") @@ -83,12 +90,14 @@ func TestObjFileIsUpToDateObjOlder(t *testing.T) { sourceFile := tempFile(t, "source") defer os.RemoveAll(sourceFile) - upToDate, err := builder_utils.ObjFileIsUpToDate(sourceFile, objFile, depFile) + upToDate, err := builder_utils.ObjFileIsUpToDate(ctx, sourceFile, objFile, depFile) NoError(t, err) require.False(t, upToDate) } func TestObjFileIsUpToDateObjNewer(t *testing.T) { + ctx := &types.Context{} + sourceFile := tempFile(t, "source") defer os.RemoveAll(sourceFile) @@ -99,12 +108,14 @@ func TestObjFileIsUpToDateObjNewer(t *testing.T) { depFile := tempFile(t, "dep") defer os.RemoveAll(depFile) - upToDate, err := builder_utils.ObjFileIsUpToDate(sourceFile, objFile, depFile) + upToDate, err := builder_utils.ObjFileIsUpToDate(ctx, sourceFile, objFile, depFile) NoError(t, err) require.True(t, upToDate) } func TestObjFileIsUpToDateDepIsNewer(t *testing.T) { + ctx := &types.Context{} + sourceFile := tempFile(t, "source") defer os.RemoveAll(sourceFile) @@ -122,12 +133,14 @@ func TestObjFileIsUpToDateDepIsNewer(t *testing.T) { utils.WriteFile(depFile, objFile+": \\\n\t"+sourceFile+" \\\n\t"+headerFile) - upToDate, err := builder_utils.ObjFileIsUpToDate(sourceFile, objFile, depFile) + upToDate, err := builder_utils.ObjFileIsUpToDate(ctx, sourceFile, objFile, depFile) NoError(t, err) require.False(t, upToDate) } func TestObjFileIsUpToDateDepIsOlder(t *testing.T) { + ctx := &types.Context{} + sourceFile := tempFile(t, "source") defer os.RemoveAll(sourceFile) @@ -143,12 +156,14 @@ func TestObjFileIsUpToDateDepIsOlder(t *testing.T) { utils.WriteFile(depFile, objFile+": \\\n\t"+sourceFile+" \\\n\t"+headerFile) - upToDate, err := builder_utils.ObjFileIsUpToDate(sourceFile, objFile, depFile) + upToDate, err := builder_utils.ObjFileIsUpToDate(ctx, sourceFile, objFile, depFile) NoError(t, err) require.True(t, upToDate) } func TestObjFileIsUpToDateDepIsWrong(t *testing.T) { + ctx := &types.Context{} + sourceFile := tempFile(t, "source") defer os.RemoveAll(sourceFile) @@ -166,7 +181,7 @@ func TestObjFileIsUpToDateDepIsWrong(t *testing.T) { utils.WriteFile(depFile, sourceFile+": \\\n\t"+sourceFile+" \\\n\t"+headerFile) - upToDate, err := builder_utils.ObjFileIsUpToDate(sourceFile, objFile, depFile) + upToDate, err := builder_utils.ObjFileIsUpToDate(ctx, sourceFile, objFile, depFile) NoError(t, err) require.False(t, upToDate) } From e92bf655baf8f04ca2a5929ba17816a26f319eff Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 2 Mar 2017 13:53:54 +0100 Subject: [PATCH 20/21] Let ObjFileIsUpToDate output verbose debug output If -debug-level=20 is passed, whenever the cached file is not usable for whatever reason, a message is displayed. This should help debug caching problems. The messages are hardcoded in the source and not put into `constants`, since they are only debug messages. Signed-off-by: Matthijs Kooijman --- builder_utils/utils.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/builder_utils/utils.go b/builder_utils/utils.go index 834364e7..5a8c1103 100644 --- a/builder_utils/utils.go +++ b/builder_utils/utils.go @@ -194,6 +194,12 @@ func ObjFileIsUpToDate(ctx *types.Context, sourceFile, objectFile, dependencyFil sourceFile = filepath.Clean(sourceFile) objectFile = filepath.Clean(objectFile) dependencyFile = filepath.Clean(dependencyFile) + logger := ctx.GetLogger() + debugLevel := ctx.DebugLevel + + if debugLevel >= 20 { + logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, "Checking previous results for {0} (result = {1}, dep = {2})", sourceFile, objectFile, dependencyFile) + } sourceFileStat, err := os.Stat(sourceFile) if err != nil { @@ -203,6 +209,9 @@ func ObjFileIsUpToDate(ctx *types.Context, sourceFile, objectFile, dependencyFil objectFileStat, err := os.Stat(objectFile) if err != nil { if os.IsNotExist(err) { + if debugLevel >= 20 { + logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, "Not found: {0}", objectFile) + } return false, nil } else { return false, i18n.WrapError(err) @@ -212,6 +221,9 @@ func ObjFileIsUpToDate(ctx *types.Context, sourceFile, objectFile, dependencyFil dependencyFileStat, err := os.Stat(dependencyFile) if err != nil { if os.IsNotExist(err) { + if debugLevel >= 20 { + logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, "Not found: {0}", dependencyFile) + } return false, nil } else { return false, i18n.WrapError(err) @@ -219,9 +231,15 @@ func ObjFileIsUpToDate(ctx *types.Context, sourceFile, objectFile, dependencyFil } if sourceFileStat.ModTime().After(objectFileStat.ModTime()) { + if debugLevel >= 20 { + logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, "{0} newer than {1}", sourceFile, objectFile) + } return false, nil } if sourceFileStat.ModTime().After(dependencyFileStat.ModTime()) { + if debugLevel >= 20 { + logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, "{0} newer than {1}", sourceFile, dependencyFile) + } return false, nil } @@ -241,10 +259,16 @@ func ObjFileIsUpToDate(ctx *types.Context, sourceFile, objectFile, dependencyFil firstRow := rows[0] if !strings.HasSuffix(firstRow, ":") { + if debugLevel >= 20 { + logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, "No colon in first line of depfile") + } return false, nil } objFileInDepFile := firstRow[:len(firstRow)-1] if objFileInDepFile != objectFile { + if debugLevel >= 20 { + logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, "Depfile is about different file: {0}", objFileInDepFile) + } return false, nil } @@ -254,12 +278,22 @@ func ObjFileIsUpToDate(ctx *types.Context, sourceFile, objectFile, dependencyFil if err != nil && !os.IsNotExist(err) { // There is probably a parsing error of the dep file // Ignore the error and trigger a full rebuild anyway + if debugLevel >= 20 { + logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, "Failed to read: {0}", row) + logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, i18n.WrapError(err).Error()) + } return false, nil } if os.IsNotExist(err) { + if debugLevel >= 20 { + logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, "Not found: {0}", row) + } return false, nil } if depStat.ModTime().After(objectFileStat.ModTime()) { + if debugLevel >= 20 { + logger.Fprintln(os.Stdout, constants.LOG_LEVEL_DEBUG, "{0} newer than {1}", row, objectFile) + } return false, nil } } From d2d49e625805853e9a531fec95788fbdc39243e0 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Thu, 2 Mar 2017 16:24:02 +0100 Subject: [PATCH 21/21] ContainerFindIncludes: Add some temporary variables This slightly cleans up a function call. Signed-off-by: Matthijs Kooijman --- container_find_includes.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/container_find_includes.go b/container_find_includes.go index 15cdc3b9..df0fca81 100644 --- a/container_find_includes.go +++ b/container_find_includes.go @@ -292,6 +292,8 @@ func writeCache(cache *includeCache, path string) error { func findIncludesUntilDone(ctx *types.Context, cache *includeCache, sourceFile types.SourceFile) error { sourcePath := sourceFile.SourcePath(ctx) + depPath := sourceFile.DepfilePath(ctx) + objPath := sourceFile.ObjectPath(ctx) targetFilePath := utils.NULLFile() // TODO: This should perhaps also compare against the @@ -306,7 +308,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.ObjFileIsUpToDate(ctx, sourcePath, sourceFile.ObjectPath(ctx), sourceFile.DepfilePath(ctx)) + unchanged, err := builder_utils.ObjFileIsUpToDate(ctx, sourcePath, objPath, depPath) if err != nil { return i18n.WrapError(err) }