Limit recursive sketch compilation to the src directory

Since c7a18f8 (Sketch sources files were recursively copied BUT not
compiled), sketches were compiled recursively. However, this turned out
to have some side effects, such as including files that were not
intended to be compiled (included as documentation or otherwise not part
uof the sketch itself) and breaking the build if the build directory is
inside the sketch directory (arduino#89).

To prevent these side effects, but still allow users to apply more
structure to their sketches, recursive compilation is now limited to the
`src` directory inside the sketch folder. All .ino files must still
reside in the root folder, but any .c and .cpp files inside
(subdirectories of) the `src` directory are included in the build.

Using a `src` directory in this way is similar to how libraries are
compiled (except for libraries *all* source files live in the `src`
directory if it is present, while for sketches files can *also* live in
the root directory).

Sketches written to use this new `src` directory should also be backward
compatible, in the sense that arduino-builder 1.0.7 (Arduino IDE 1.6.7)
and later can succesfully compile it, not just the versions after this
commit. The reverse is not true: sketches that need recursive
compilation outside of the `src` directory, as supported by
arduino-builder 1.0.7 above, will not compile after this commit, but it
is trivial to convert them to do so (without breaking compilation before
this commit).

This also changes some testcases that used subfolders, to move these
inside the `src` directory instead of directly in the sketch root
directory.

This fixes arduino#89.

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

Author: matthijskooijman

src/arduino.cc/builder/constants/constants.go

@@ -204,6 +204,7 @@ const RECIPE_S_PATTERN = "recipe.S.o.pattern"
 const REWRITING_DISABLED = "disabled"
 const REWRITING = "rewriting"
 const SPACE = " "
+const SKETCH_FOLDER_SRC = "src"
 const TOOL_NAME = "name"
 const TOOL_URL = "url"
 const TOOL_VERSION = "version"

src/arduino.cc/builder/phases/sketch_builder.go

@@ -34,6 +34,9 @@ import (
 	"arduino.cc/builder/i18n"
 	"arduino.cc/builder/types"
 	"arduino.cc/builder/utils"
+	"arduino.cc/builder/constants"
+	"path/filepath"
+	"os"
 )
 
 type SketchBuilder struct{}
@@ -53,11 +56,20 @@ func (s *SketchBuilder) Run(ctx *types.Context) error {
 	}
 
 	var objectFiles []string
-	objectFiles, err = builder_utils.CompileFiles(objectFiles, sketchBuildPath, true, sketchBuildPath, buildProperties, includes, verbose, warningsLevel, logger)
+	objectFiles, err = builder_utils.CompileFiles(objectFiles, sketchBuildPath, false, sketchBuildPath, buildProperties, includes, verbose, warningsLevel, logger)
 	if err != nil {
 		return i18n.WrapError(err)
 	}
 
+	// 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)
+		if err != nil {
+			return i18n.WrapError(err)
+		}
+	}
+
 	ctx.SketchObjectFiles = objectFiles
 
 	return nil

src/arduino.cc/builder/sketch_loader.go

@@ -88,8 +88,21 @@ func (s *SketchLoader) Run(ctx *types.Context) error {
 
 func collectAllSketchFiles(from string) ([]string, error) {
 	filePaths := []string{}
-	extensions := func(ext string) bool { return MAIN_FILE_VALID_EXTENSIONS[ext] || ADDITIONAL_FILE_VALID_EXTENSIONS[ext] }
-	err := utils.FindFilesInFolder(&filePaths, from, extensions, /* recurse */ true)
+	// Source files in the root are compiled, non-recursively. This
+	// is the only place where .ino files can be present.
+	rootExtensions := func(ext string) bool { return MAIN_FILE_VALID_EXTENSIONS[ext] || ADDITIONAL_FILE_VALID_EXTENSIONS[ext] }
+	err := utils.FindFilesInFolder(&filePaths, from, rootExtensions, /* recurse */ false)
+	if err != nil {
+		return nil, i18n.WrapError(err)
+	}
+
+	// The "src/" subdirectory of a sketch is compiled recursively
+	// (but .ino files are *not* compiled)
+	srcPath := filepath.Join(from, constants.SKETCH_FOLDER_SRC)
+	if info, err := os.Stat(srcPath); err == nil && info.IsDir() {
+		srcExtensions := func(ext string) bool { return ADDITIONAL_FILE_VALID_EXTENSIONS[ext] }
+		err = utils.FindFilesInFolder(&filePaths, srcPath, srcExtensions, /* recurse */ true)
+	}
 	return filePaths, i18n.WrapError(err)
 }
 

src/arduino.cc/builder/test/additional_sketch_files_copier_test.go

@@ -83,9 +83,9 @@ func TestCopyOtherFiles(t *testing.T) {
 	sort.Sort(ByFileInfoName(files))
 	require.Equal(t, "header.h", files[0].Name())
 	require.Equal(t, "s_file.S", files[1].Name())
-	require.Equal(t, "subfolder", files[2].Name())
+	require.Equal(t, "src", files[2].Name())
 
-	files, err1 = gohasissues.ReadDir(filepath.Join(buildPath, constants.FOLDER_SKETCH, "subfolder"))
+	files, err1 = gohasissues.ReadDir(filepath.Join(buildPath, constants.FOLDER_SKETCH, "src"))
 	NoError(t, err1)
 	require.Equal(t, 1, len(files))
 	require.Equal(t, "helper.h", files[0].Name())

src/arduino.cc/builder/test/builder_test.go

@@ -373,3 +373,32 @@ func TestBuilderSketchBuildPathContainsUnusedPreviouslyCompiledLibrary(t *testin
 	_, err = os.Stat(filepath.Join(buildPath, constants.FOLDER_LIBRARIES, "Bridge"))
 	NoError(t, err)
 }
+
+func TestBuilderWithBuildPathInSketchDir(t *testing.T) {
+	DownloadCoresAndToolsAndLibraries(t)
+
+	ctx := &types.Context{
+		HardwareFolders:         []string{filepath.Join("..", "hardware"), "hardware", "downloaded_hardware"},
+		ToolsFolders:            []string{"downloaded_tools"},
+		BuiltInLibrariesFolders: []string{"downloaded_libraries"},
+		OtherLibrariesFolders:   []string{"libraries"},
+		SketchLocation:          filepath.Join("sketch1", "sketch.ino"),
+		FQBN:                    "arduino:avr:uno",
+		ArduinoAPIVersion:       "10600",
+		Verbose:                 true,
+	}
+
+	var err error
+	ctx.BuildPath, err = filepath.Abs(filepath.Join("sketch1", "build"))
+	NoError(t, err)
+	defer os.RemoveAll(ctx.BuildPath)
+
+	command := builder.Builder{}
+	err = command.Run(ctx)
+	NoError(t, err)
+
+	// Run build twice, to verify the build still works when the
+	// build directory is present at the start
+	err = command.Run(ctx)
+	NoError(t, err)
+}

src/arduino.cc/builder/test/collect_all_source_files_from_folders_with_sources_test.go

@@ -60,7 +60,7 @@ func TestCollectAllSourceFilesFromFoldersWithSources(t *testing.T) {
 	require.Equal(t, 0, len(*foldersWithSources))
 	sort.Strings(*sourceFiles)
 
-	require.Equal(t, Abs(t, filepath.Join("sketch_with_config", "includes", "de bug.cpp")), sourceFiles.Pop())
+	require.Equal(t, Abs(t, filepath.Join("sketch_with_config", "src", "includes", "de bug.cpp")), sourceFiles.Pop())
 	require.Equal(t, 0, len(*sourceFiles))
 }
 

src/arduino.cc/builder/test/sketch1/subfolder/helper.h renamed to src/arduino.cc/builder/test/sketch1/src/helper.h

@@ -1,7 +1,7 @@
 #include "config.h"
 
 #ifdef DEBUG
-#include "includes/de bug.h"
+#include "src/includes/de bug.h"
 #endif
 
 #ifdef UNDEF

src/arduino.cc/builder/test/sketch_with_config/sketch_with_config.ino

@@ -4,7 +4,7 @@
 #include "config.h"
 
 #ifdef DEBUG
-#include "includes/de bug.h"
+#include "src/includes/de bug.h"
 #endif
 
 #ifdef UNDEF

src/arduino.cc/builder/test/sketch_with_config/sketch_with_config.preprocessed.txt

@@ -1,4 +1,4 @@
-#include "subfolder/other.h"
+#include "src/subfolder/other.h"
 
 MyClass myClass;
 
@@ -7,4 +7,4 @@ void setup() {
 }
 
 void loop() {
-}
+}

src/arduino.cc/builder/test/sketch_with_config/includes/de bug.cpp renamed to src/arduino.cc/builder/test/sketch_with_config/src/includes/de bug.cpp

@@ -0,0 +1 @@
+#error "Whattya looking at?"

src/arduino.cc/builder/test/sketch_with_config/includes/de bug.h renamed to src/arduino.cc/builder/test/sketch_with_config/src/includes/de bug.h

@@ -0,0 +1 @@
+#error "Whattya looking at?"