Fix mixed code precompiled libraries (again!) (#611)

* Fix mixed code precompiled libraries

This patch restores some functionalities broken by #512 .
It introduces a BREAKING CHANGE to "precompiled" field

If merged, "precompiled" means that the library ONLY contains a precompiled library and its headers.
No file containing implementation will be compiled if the library is specified as such.

Fixes arduino/arduino-builder#353

Tested with
* #512 (comment) (Nano33BLE and generic MKR board)
* https://github.com/BoschSensortec/BSEC-Arduino-library (works after removing precompiled=true directive in library.properties)
* https://github.com/vidor-libraries/USBBlaster (MKRVidor, fully precompiled)

* Added test-build for Arduino_TensorFlowLite

* Added another test for Bosch Sensor lib

* Fallback search for precompiled libraries in non-fpu specific directories

* variable renamed

* Moved fixLDFLAG inside compileLibraries

* Inlined FixLDflags

* Using more paths helpers to simplify code

* Added support for "precompiled=full" in library.properties

This flag allow the deployment of a pure precompiled library that ships
also the source code of the precompiled part.
This allows to possibly compile-from-source as a fallback if the library
does not provide a precompiled build for the current architecture.

The "precompiled=true" instead has been ported back to the old behaviour
(for libraries that ships a precompiled binary and support source code
that wraps/uses the precompiled part).

* Updated tests

* Fixed test compile-build on very long paths on Windows

* Removed constants.BUILD_PROPERTIES_COMPILER_LIBRARIES_LDFLAGS

* Add space before "-L" gcc flag to allow multiple precompiled libs to be included

Co-authored-by: Cristian Maglie <[email protected]>

Author: facchinm

Diff for: arduino/libraries/libraries.go

@@ -55,20 +55,21 @@ type Library struct {
 
 	Types []string `json:"types,omitempty"`
 
-	InstallDir        *paths.Path
-	SourceDir         *paths.Path
-	UtilityDir        *paths.Path
-	Location          LibraryLocation
-	ContainerPlatform *cores.PlatformRelease `json:""`
-	Layout            LibraryLayout
-	RealName          string
-	DotALinkage       bool
-	Precompiled       bool
-	LDflags           string
-	IsLegacy          bool
-	Version           *semver.Version
-	License           string
-	Properties        *properties.Map
+	InstallDir             *paths.Path
+	SourceDir              *paths.Path
+	UtilityDir             *paths.Path
+	Location               LibraryLocation
+	ContainerPlatform      *cores.PlatformRelease `json:""`
+	Layout                 LibraryLayout
+	RealName               string
+	DotALinkage            bool
+	Precompiled            bool
+	PrecompiledWithSources bool
+	LDflags                string
+	IsLegacy               bool
+	Version                *semver.Version
+	License                string
+	Properties             *properties.Map
 }
 
 func (library *Library) String() string {

Diff for: arduino/libraries/loader.go

@@ -103,7 +103,8 @@ func makeNewLibrary(libraryDir *paths.Path, location LibraryLocation) (*Library,
 	library.Website = strings.TrimSpace(libProperties.Get("url"))
 	library.IsLegacy = false
 	library.DotALinkage = libProperties.GetBoolean("dot_a_linkage")
-	library.Precompiled = libProperties.GetBoolean("precompiled")
+	library.PrecompiledWithSources = libProperties.Get("precompiled") == "full"
+	library.Precompiled = libProperties.Get("precompiled") == "true" || library.PrecompiledWithSources
 	library.LDflags = strings.TrimSpace(libProperties.Get("ldflags"))
 	library.Properties = libProperties
 

Diff for: legacy/builder/constants/constants.go

@@ -26,7 +26,6 @@ const BUILD_PROPERTIES_BUILD_BOARD = "build.board"
 const BUILD_PROPERTIES_BUILD_MCU = "build.mcu"
 const BUILD_PROPERTIES_COMPILER_C_ELF_FLAGS = "compiler.c.elf.flags"
 const BUILD_PROPERTIES_COMPILER_LDFLAGS = "compiler.ldflags"
-const BUILD_PROPERTIES_COMPILER_LIBRARIES_LDFLAGS = "compiler.libraries.ldflags"
 const BUILD_PROPERTIES_COMPILER_CPP_FLAGS = "compiler.cpp.flags"
 const BUILD_PROPERTIES_COMPILER_WARNING_FLAGS = "compiler.warning_flags"
 const BUILD_PROPERTIES_FQBN = "build.fqbn"
@@ -95,7 +94,6 @@ const MSG_FIND_INCLUDES_FAILED = "Error while detecting libraries included by {0
 const MSG_INVALID_QUOTING = "Invalid quoting: no closing [{0}] char found."
 const MSG_LIB_LEGACY = "(legacy)"
 const MSG_LIBRARIES_MULTIPLE_LIBS_FOUND_FOR = "Multiple libraries were found for \"{0}\""
-const MSG_PRECOMPILED_LIBRARY_NOT_FOUND_FOR = "Library \"{0}\" declared precompiled but folder \"{1}\" does not exist"
 const MSG_LIBRARIES_NOT_USED = " Not used: {0}"
 const MSG_LIBRARIES_USED = " Used: {0}"
 const MSG_LIBRARY_CAN_USE_SRC_AND_UTILITY_FOLDERS = "Library can't use both 'src' and 'utility' folders. Double check {0}"

Diff for: legacy/builder/phases/libraries_builder.go

@@ -17,7 +17,6 @@ package phases
 
 import (
 	"os"
-	"path/filepath"
 	"strings"
 
 	"github.com/arduino/arduino-cli/arduino/libraries"
@@ -30,8 +29,6 @@ import (
 	"github.com/pkg/errors"
 )
 
-var PRECOMPILED_LIBRARIES_VALID_EXTENSIONS_STATIC = map[string]bool{".a": true}
-var PRECOMPILED_LIBRARIES_VALID_EXTENSIONS_DYNAMIC = map[string]bool{".so": true}
 var FLOAT_ABI_CFLAG = "float-abi"
 var FPU_CFLAG = "fpu"
 
@@ -53,10 +50,6 @@ func (s *LibrariesBuilder) Run(ctx *types.Context) error {
 	}
 
 	ctx.LibrariesObjectFiles = objectFiles
-
-	// Search for precompiled libraries
-	fixLDFLAGforPrecompiledLibraries(ctx, libs)
-
 	return nil
 }
 
@@ -87,58 +80,25 @@ func findExpectedPrecompiledLibFolder(ctx *types.Context, library *libraries.Lib
 	}
 
 	logger := ctx.GetLogger()
+	logger.Fprintln(os.Stdout, constants.LOG_LEVEL_INFO, "Library {0} has been declared precompiled:", library.Name)
+
+	// Try directory with full fpuSpecs first, if available
 	if len(fpuSpecs) > 0 {
 		fpuSpecs = strings.TrimRight(fpuSpecs, "-")
-		if library.SourceDir.Join(mcu).Join(fpuSpecs).Exist() {
-			return library.SourceDir.Join(mcu).Join(fpuSpecs)
-		} else {
-			// we are unsure, compile from sources
-			logger.Fprintln(os.Stdout, constants.LOG_LEVEL_INFO,
-				constants.MSG_PRECOMPILED_LIBRARY_NOT_FOUND_FOR, library.Name, library.SourceDir.Join(mcu).Join(fpuSpecs))
-			return nil
+		fullPrecompDir := library.SourceDir.Join(mcu).Join(fpuSpecs)
+		if fullPrecompDir.Exist() {
+			logger.Fprintln(os.Stdout, constants.LOG_LEVEL_INFO, "Using precompiled library in {0}", fullPrecompDir)
+			return fullPrecompDir
 		}
+		logger.Fprintln(os.Stdout, constants.LOG_LEVEL_INFO, "Precompiled library in \"{0}\" not found", fullPrecompDir)
 	}
 
-	if library.SourceDir.Join(mcu).Exist() {
-		return library.SourceDir.Join(mcu)
-	}
-
-	logger.Fprintln(os.Stdout, constants.LOG_LEVEL_INFO,
-		constants.MSG_PRECOMPILED_LIBRARY_NOT_FOUND_FOR, library.Name, library.SourceDir.Join(mcu))
-
-	return nil
-}
-
-func fixLDFLAGforPrecompiledLibraries(ctx *types.Context, libs libraries.List) error {
-
-	for _, library := range libs {
-		if library.Precompiled {
-			// add library src path to compiler.c.elf.extra_flags
-			// use library.Name as lib name and srcPath/{mcpu} as location
-			path := findExpectedPrecompiledLibFolder(ctx, library)
-			if path == nil {
-				break
-			}
-			// find all library names in the folder and prepend -l
-			filePaths := []string{}
-			libs_cmd := library.LDflags + " "
-			extensions := func(ext string) bool {
-				return PRECOMPILED_LIBRARIES_VALID_EXTENSIONS_DYNAMIC[ext] || PRECOMPILED_LIBRARIES_VALID_EXTENSIONS_STATIC[ext]
-			}
-			utils.FindFilesInFolder(&filePaths, path.String(), extensions, false)
-			for _, lib := range filePaths {
-				name := strings.TrimSuffix(filepath.Base(lib), filepath.Ext(lib))
-				// strip "lib" first occurrence
-				if strings.HasPrefix(name, "lib") {
-					name = strings.Replace(name, "lib", "", 1)
-					libs_cmd += "-l" + name + " "
-				}
-			}
-
-			currLDFlags := ctx.BuildProperties.Get(constants.BUILD_PROPERTIES_COMPILER_LIBRARIES_LDFLAGS)
-			ctx.BuildProperties.Set(constants.BUILD_PROPERTIES_COMPILER_LIBRARIES_LDFLAGS, currLDFlags+"\"-L"+path.String()+"\" "+libs_cmd+" ")
-		}
+	precompDir := library.SourceDir.Join(mcu)
+	if precompDir.Exist() {
+		logger.Fprintln(os.Stdout, constants.LOG_LEVEL_INFO, "Using precompiled library in {0}", precompDir)
+		return precompDir
 	}
+	logger.Fprintln(os.Stdout, constants.LOG_LEVEL_INFO, "Precompiled library in \"{0}\" not found", precompDir)
 	return nil
 }
 
@@ -175,25 +135,48 @@ func compileLibrary(ctx *types.Context, library *libraries.Library, buildPath *p
 	objectFiles := paths.NewPathList()
 
 	if library.Precompiled {
-		// search for files with PRECOMPILED_LIBRARIES_VALID_EXTENSIONS
-		extensions := func(ext string) bool { return PRECOMPILED_LIBRARIES_VALID_EXTENSIONS_STATIC[ext] }
-
-		filePaths := []string{}
+		coreSupportPrecompiled := ctx.BuildProperties.ContainsKey("compiler.libraries.ldflags")
 		precompiledPath := findExpectedPrecompiledLibFolder(ctx, library)
-		if precompiledPath != nil {
-			// TODO: This codepath is just taken for .a with unusual names that would
-			// be ignored by -L / -l methods.
-			// Should we force precompiled libraries to start with "lib" ?
-			err := utils.FindFilesInFolder(&filePaths, precompiledPath.String(), extensions, false)
+
+		if !coreSupportPrecompiled {
+			logger := ctx.GetLogger()
+			logger.Fprintln(os.Stdout, constants.LOG_LEVEL_INFO, "The plaform does not support 'compiler.libraries.ldflags' for precompiled libraries.")
+
+		} else if precompiledPath != nil {
+			// Find all libraries in precompiledPath
+			libs, err := precompiledPath.ReadDir()
 			if err != nil {
 				return nil, errors.WithStack(err)
 			}
-			for _, path := range filePaths {
-				if !strings.HasPrefix(filepath.Base(path), "lib") {
-					objectFiles.Add(paths.New(path))
+
+			// Add required LD flags
+			libsCmd := library.LDflags + " "
+			dynAndStaticLibs := libs.Clone()
+			dynAndStaticLibs.FilterSuffix(".a", ".so")
+			for _, lib := range dynAndStaticLibs {
+				name := strings.TrimSuffix(lib.Base(), lib.Ext())
+				if strings.HasPrefix(name, "lib") {
+					libsCmd += "-l" + name[3:] + " "
+				}
+			}
+
+			currLDFlags := ctx.BuildProperties.Get("compiler.libraries.ldflags")
+			ctx.BuildProperties.Set("compiler.libraries.ldflags", currLDFlags+" \"-L"+precompiledPath.String()+"\" "+libsCmd+" ")
+
+			// TODO: This codepath is just taken for .a with unusual names that would
+			// be ignored by -L / -l methods.
+			// Should we force precompiled libraries to start with "lib" ?
+			staticLibs := libs.Clone()
+			staticLibs.FilterSuffix(".a")
+			for _, lib := range staticLibs {
+				if !strings.HasPrefix(lib.Base(), "lib") {
+					objectFiles.Add(lib)
 				}
 			}
-			return objectFiles, nil
+
+			if library.PrecompiledWithSources {
+				return objectFiles, nil
+			}
 		}
 	}
 

Diff for: test/conftest.py

@@ -20,6 +20,7 @@
 import simplejson as json
 from invoke import Local
 from invoke.context import Context
+import tempfile
 
 from .common import Board
 
@@ -31,7 +32,28 @@ def data_dir(tmpdir_factory):
     each test and deleted at the end, this way all the
     tests work in isolation.
     """
-    return str(tmpdir_factory.mktemp("ArduinoTest"))
+
+    # it seems that paths generated by pytest's tmpdir_factory are too
+    # long and may lead to arduino-cli compile failures due to the
+    # combination of (some or all of) the following reasons:
+    # 1) Windows not liking path >260 chars in len
+    # 2) arm-gcc not fully optimizing long paths
+    # 3) libraries requiring headers deep down the include path
+    # for example:
+    #
+    #             from C:\Users\runneradmin\AppData\Local\Temp\pytest-of-runneradmin\pytest-0\A7\packages\arduino\hardware\mbed\1.1.4\cores\arduino/mbed/rtos/Thread.h:29,
+    #             from C:\Users\runneradmin\AppData\Local\Temp\pytest-of-runneradmin\pytest-0\A7\packages\arduino\hardware\mbed\1.1.4\cores\arduino/mbed/rtos/rtos.h:28,
+    #             from C:\Users\runneradmin\AppData\Local\Temp\pytest-of-runneradmin\pytest-0\A7\packages\arduino\hardware\mbed\1.1.4\cores\arduino/mbed/mbed.h:23,
+    #             from C:\Users\runneradmin\AppData\Local\Temp\pytest-of-runneradmin\pytest-0\A7\packages\arduino\hardware\mbed\1.1.4\cores\arduino/Arduino.h:32,
+    #             from C:\Users\RUNNER~1\AppData\Local\Temp\arduino-sketch-739B2B6DD21EB014317DA2A46062811B\sketch\magic_wand.ino.cpp:1:
+    ##[error]c:\users\runneradmin\appdata\local\temp\pytest-of-runneradmin\pytest-0\a7\packages\arduino\tools\arm-none-eabi-gcc\7-2017q4\arm-none-eabi\include\c++\7.2.1\new:39:10: fatal error: bits/c++config.h: No such file or directory
+    #
+    # due to the above on Windows we cut the tmp path straight to /tmp/xxxxxxxx
+    if platform.system() == "Windows":
+        with tempfile.TemporaryDirectory() as tmp:
+            yield tmp
+    else:
+        yield str(tmpdir_factory.mktemp("ArduinoTest"))
 
 
 @pytest.fixture(scope="session")

Diff for: test/test_compile.py

@@ -281,3 +281,44 @@ def test_compile_blacklisted_sketchname(run_command, data_dir):
         "compile -b {fqbn} {sketch_path}".format(fqbn=fqbn, sketch_path=sketch_path)
     )
     assert result.ok
+
+
+def test_compile_without_precompiled_libraries(run_command, data_dir):
+    # Init the environment explicitly
+    url = "https://adafruit.github.io/arduino-board-index/package_adafruit_index.json"
+    result = run_command("core update-index --additional-urls={}".format(url))
+    assert result.ok
+    result = run_command("core install arduino:mbed --additional-urls={}".format(url))
+    assert result.ok
+    result = run_command("core install arduino:samd --additional-urls={}".format(url))
+    assert result.ok
+    result = run_command("core install adafruit:samd --additional-urls={}".format(url))
+    assert result.ok
+
+    # Install pre-release version of Arduino_TensorFlowLite (will be officially released
+    # via lib manager after https://github.com/arduino/arduino-builder/issues/353 is in)
+    import zipfile
+    with zipfile.ZipFile("test/testdata/Arduino_TensorFlowLite.zip", 'r') as zip_ref:
+        zip_ref.extractall("{}/libraries/".format(data_dir))
+    result = run_command("lib install [email protected]")
+    assert result.ok
+    result = run_command("compile -b arduino:mbed:nano33ble {}/libraries/Arduino_TensorFlowLite/examples/magic_wand/".format(data_dir))
+    assert result.ok
+    result = run_command("compile -b adafruit:samd:adafruit_feather_m4 {}/libraries/Arduino_TensorFlowLite/examples/magic_wand/".format(data_dir))
+    assert result.ok
+
+    # Non-precompiled version of Arduino_TensorflowLite
+    result = run_command("lib install [email protected]")
+    assert result.ok
+    result = run_command("compile -b arduino:mbed:nano33ble {}/libraries/Arduino_TensorFlowLite/examples/magic_wand/".format(data_dir))
+    assert result.ok
+    result = run_command("compile -b adafruit:samd:adafruit_feather_m4 {}/libraries/Arduino_TensorFlowLite/examples/magic_wand/".format(data_dir))
+    assert result.ok
+
+    # Bosch sensor library
+    result = run_command("lib install \"BSEC Software [email protected]\"")
+    assert result.ok
+    result = run_command("compile -b arduino:samd:mkr1000 {}/libraries/BSEC_Software_Library/examples/basic/".format(data_dir))
+    assert result.ok
+    result = run_command("compile -b arduino:mbed:nano33ble {}/libraries/BSEC_Software_Library/examples/basic/".format(data_dir))
+    assert result.ok