Compile extract all artifacts in "sketch/build" folder (#687)

* Deprecated exportFile/importFile in favor of exportDir/importDir

* Updated compile/upload cli commands

* Compile now saves artifacts in 'sketch/build/<FQBN>/...' folder

* Upload now uses export folder

* Test fix: use --output-dir option instead of deprecated --output

* Text fix: no more need to check if "extension won't be added if already present"

* Added Debug.ImportDir and deprecated Debug.ImportFile

* Upload now uses export folder

* Fixed GetCommandLine test

* Fixed test_core_install_esp32

Author: cmaglie

Diff for: cli/compile/compile.go

@@ -46,7 +46,7 @@ var (
 	uploadAfterCompile bool     // Upload the binary after the compilation.
 	port               string   // Upload port, e.g.: COM10 or /dev/ttyACM0.
 	verify             bool     // Upload, verify uploaded binary after the upload.
-	exportFile         string   // The compiled binary is written to this file
+	exportDir          string   // The compiled binary is written to this file
 	dryRun             bool     // Use this flag to now write the output file
 	libraries          []string // List of custom libraries paths separated by commas. Or can be used multiple times for multiple libraries paths.
 	optimizeForDebug   bool     // Optimize compile output for debug, not for release
@@ -67,7 +67,7 @@ func NewCommand() *cobra.Command {
 	command.Flags().BoolVar(&showProperties, "show-properties", false, "Show all build properties used instead of compiling.")
 	command.Flags().BoolVar(&preprocess, "preprocess", false, "Print preprocessed code to stdout instead of compiling.")
 	command.Flags().StringVar(&buildCachePath, "build-cache-path", "", "Builds of 'core.a' are saved into this path to be cached and reused.")
-	command.Flags().StringVarP(&exportFile, "output", "o", "", "Filename of the compile output.")
+	command.Flags().StringVarP(&exportDir, "output-dir", "", "", "Save build artifacts in this directory.")
 	command.Flags().BoolVarP(&dryRun, "dry-run", "n", false, "Perform the build but do not copy the compile output file.")
 	command.Flags().StringVar(&buildPath, "build-path", "",
 		"Path where to save compiled files. If omitted, a directory will be created in the default temporary path of your OS.")
@@ -115,7 +115,7 @@ func run(cmd *cobra.Command, args []string) {
 		Verbose:          verbose,
 		Quiet:            quiet,
 		VidPid:           vidPid,
-		ExportFile:       exportFile,
+		ExportDir:        exportDir,
 		DryRun:           dryRun,
 		Libraries:        libraries,
 		OptimizeForDebug: optimizeForDebug,
@@ -134,7 +134,7 @@ func run(cmd *cobra.Command, args []string) {
 			Port:       port,
 			Verbose:    verbose,
 			Verify:     verify,
-			ImportFile: exportFile,
+			ImportDir:  exportDir,
 		}, os.Stdout, os.Stderr)
 
 		if err != nil {

Diff for: cli/debug/debug.go

@@ -37,7 +37,7 @@ var (
 	verbose     bool
 	verify      bool
 	interpreter string
-	importFile  string
+	importDir   string
 )
 
 // NewCommand created a new `upload` command
@@ -54,7 +54,7 @@ func NewCommand() *cobra.Command {
 	debugCommand.Flags().StringVarP(&fqbn, "fqbn", "b", "", "Fully Qualified Board Name, e.g.: arduino:avr:uno")
 	debugCommand.Flags().StringVarP(&port, "port", "p", "", "Debug port, e.g.: COM10 or /dev/ttyACM0")
 	debugCommand.Flags().StringVar(&interpreter, "interpreter", "console", "Debug interpreter e.g.: console, mi, mi1, mi2, mi3")
-	debugCommand.Flags().StringVarP(&importFile, "input", "i", "", "Input file to be uploaded for debug.")
+	debugCommand.Flags().StringVarP(&importDir, "input-dir", "", "", "Direcory containing binaries for debug.")
 
 	return debugCommand
 }
@@ -82,7 +82,7 @@ func run(command *cobra.Command, args []string) {
 		SketchPath:  sketchPath.String(),
 		Port:        port,
 		Interpreter: interpreter,
-		ImportFile:  importFile,
+		ImportDir:   importDir,
 	}, os.Stdin, os.Stdout, ctrlc); err != nil {
 		feedback.Errorf("Error during Debug: %v", err)
 		os.Exit(errorcodes.ErrGeneric)

Diff for: cli/upload/upload.go

@@ -30,11 +30,11 @@ import (
 )
 
 var (
-	fqbn       string
-	port       string
-	verbose    bool
-	verify     bool
-	importFile string
+	fqbn      string
+	port      string
+	verbose   bool
+	verify    bool
+	importDir string
 )
 
 // NewCommand created a new `upload` command
@@ -50,7 +50,7 @@ func NewCommand() *cobra.Command {
 
 	uploadCommand.Flags().StringVarP(&fqbn, "fqbn", "b", "", "Fully Qualified Board Name, e.g.: arduino:avr:uno")
 	uploadCommand.Flags().StringVarP(&port, "port", "p", "", "Upload port, e.g.: COM10 or /dev/ttyACM0")
-	uploadCommand.Flags().StringVarP(&importFile, "input", "i", "", "Input file to be uploaded.")
+	uploadCommand.Flags().StringVarP(&importDir, "input-dir", "", "", "Direcory containing binaries to upload.")
 	uploadCommand.Flags().BoolVarP(&verify, "verify", "t", false, "Verify uploaded binary after the upload.")
 	uploadCommand.Flags().BoolVarP(&verbose, "verbose", "v", false, "Optional, turns on verbose mode.")
 
@@ -77,7 +77,7 @@ func run(command *cobra.Command, args []string) {
 		Port:       port,
 		Verbose:    verbose,
 		Verify:     verify,
-		ImportFile: importFile,
+		ImportDir:  importDir,
 	}, os.Stdout, os.Stderr); err != nil {
 		feedback.Errorf("Error during Upload: %v", err)
 		os.Exit(errorcodes.ErrGeneric)

Diff for: commands/compile/compile.go

@@ -17,7 +17,6 @@ package compile
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"io"
 	"path/filepath"
@@ -37,6 +36,7 @@ import (
 	"github.com/arduino/arduino-cli/telemetry"
 	paths "github.com/arduino/go-paths-helper"
 	properties "github.com/arduino/go-properties-orderedmap"
+	"github.com/pkg/errors"
 	"github.com/segmentio/stats/v4"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/viper"
@@ -55,11 +55,16 @@ func Compile(ctx context.Context, req *rpc.CompileReq, outStream, errStream io.W
 		"verbose":         strconv.FormatBool(req.Verbose),
 		"quiet":           strconv.FormatBool(req.Quiet),
 		"vidPid":          req.VidPid,
-		"exportFile":      telemetry.Sanitize(req.ExportFile),
+		"exportFile":      telemetry.Sanitize(req.ExportFile), // deprecated
+		"exportDir":       telemetry.Sanitize(req.GetExportDir()),
 		"jobs":            strconv.FormatInt(int64(req.Jobs), 10),
 		"libraries":       strings.Join(req.Libraries, ","),
 	}
 
+	if req.GetExportFile() != "" {
+		outStream.Write([]byte(fmt.Sprintln("Compile.ExportFile has been deprecated. The ExportFile parameter will be ignored, use ExportDir instead.")))
+	}
+
 	// Use defer func() to evaluate tags map when function returns
 	// and set success flag inspecting the error named return parameter
 	defer func() {
@@ -197,54 +202,38 @@ func Compile(ctx context.Context, req *rpc.CompileReq, outStream, errStream io.W
 	}
 
 	if !req.GetDryRun() {
-		// FIXME: Make a function to obtain these info...
-		outputPath := paths.New(
-			builderCtx.BuildProperties.ExpandPropsInString("{build.path}/{recipe.output.tmp_file}")) // "/build/path/sketch.ino.bin"
-		ext := outputPath.Ext()          // ".hex" | ".bin"
-		base := outputPath.Base()        // "sketch.ino.hex"
-		base = base[:len(base)-len(ext)] // "sketch.ino"
-
-		// FIXME: Make a function to produce a better name...
-		// Make the filename without the FQBN configs part
-		fqbnSuffix := strings.Replace(fqbn.StringWithoutConfig(), ":", ".", -1)
-
 		var exportPath *paths.Path
-		var exportFile string
-		if req.GetExportFile() == "" {
-			exportPath = sketch.FullPath
-			exportFile = sketch.Name + "." + fqbnSuffix // "sketch.arduino.avr.uno"
+		if exportDir := req.GetExportDir(); exportDir != "" {
+			exportPath = paths.New(exportDir)
 		} else {
-			exportPath = paths.New(req.GetExportFile()).Parent()
-			exportFile = paths.New(req.GetExportFile()).Base()
-			if strings.HasSuffix(exportFile, ext) {
-				exportFile = exportFile[:len(exportFile)-len(ext)]
-			}
+			exportPath = sketch.FullPath
+			// Add FQBN (without configs part) to export path
+			fqbnSuffix := strings.Replace(fqbn.StringWithoutConfig(), ":", ".", -1)
+			exportPath = exportPath.Join("build").Join(fqbnSuffix)
+		}
+		logrus.WithField("path", exportPath).Trace("Saving sketch to export path.")
+		if err := exportPath.MkdirAll(); err != nil {
+			return nil, errors.Wrap(err, "creating output dir")
 		}
 
-		// Copy "sketch.ino.*.hex" / "sketch.ino.*.bin" artifacts to sketch directory
-		srcDir, err := outputPath.Parent().ReadDir() // read "/build/path/*"
-		if err != nil {
-			return nil, fmt.Errorf("reading build directory: %s", err)
+		// Copy all "sketch.ino.*" artifacts to the export directory
+		baseName, ok := builderCtx.BuildProperties.GetOk("build.project_name") // == "sketch.ino"
+		if !ok {
+			return nil, errors.New("missing 'build.project_name' build property")
 		}
-		srcDir.FilterPrefix(base + ".")
-		srcDir.FilterSuffix(ext)
-		for _, srcOutput := range srcDir {
-			srcFilename := srcOutput.Base()       // "sketch.ino.*.bin"
-			srcFilename = srcFilename[len(base):] // ".*.bin"
-			dstOutput := exportPath.Join(exportFile + srcFilename)
-			logrus.WithField("from", srcOutput).WithField("to", dstOutput).Debug("copying sketch build output")
-			if err = srcOutput.CopyTo(dstOutput); err != nil {
-				return nil, fmt.Errorf("copying output file: %s", err)
-			}
+		buildFiles, err := builderCtx.BuildPath.ReadDir()
+		if err != nil {
+			return nil, errors.Errorf("reading build directory: %s", err)
 		}
-
-		// Copy .elf file to sketch directory
-		srcElf := outputPath.Parent().Join(base + ".elf")
-		if srcElf.Exist() {
-			dstElf := exportPath.Join(exportFile + ".elf")
-			logrus.WithField("from", srcElf).WithField("to", dstElf).Debug("copying sketch build output")
-			if err = srcElf.CopyTo(dstElf); err != nil {
-				return nil, fmt.Errorf("copying elf file: %s", err)
+		buildFiles.FilterPrefix(baseName)
+		for _, buildFile := range buildFiles {
+			exportedFile := exportPath.Join(buildFile.Base())
+			logrus.
+				WithField("src", buildFile).
+				WithField("dest", exportedFile).
+				Trace("Copying artifact.")
+			if err = buildFile.CopyTo(exportedFile); err != nil {
+				return nil, errors.Wrapf(err, "copying output file %s", buildFile)
 			}
 		}
 	}

Diff for: commands/debug/debug.go

@@ -115,6 +115,10 @@ func Debug(ctx context.Context, req *dbg.DebugConfigReq, inStream io.Reader, out
 
 // getCommandLine compose a debug command represented by a core recipe
 func getCommandLine(req *dbg.DebugConfigReq, pm *packagemanager.PackageManager) ([]string, error) {
+	if req.GetImportFile() != "" {
+		return nil, errors.New("the ImportFile parameter has been deprecated, use ImportDir instead")
+	}
+
 	// TODO: make a generic function to extract sketch from request
 	// and remove duplication in commands/compile.go
 	if req.GetSketchPath() == "" {
@@ -185,40 +189,24 @@ func getCommandLine(req *dbg.DebugConfigReq, pm *packagemanager.PackageManager)
 		}
 	}
 
-	// Set path to compiled binary
-	// Make the filename without the FQBN configs part
-	fqbn.Configs = properties.NewMap()
-	fqbnSuffix := strings.Replace(fqbn.String(), ":", ".", -1)
-
 	var importPath *paths.Path
-	var importFile string
-	if req.GetImportFile() == "" {
-		importPath = sketch.FullPath
-		importFile = sketch.Name + "." + fqbnSuffix
+	if importDir := req.GetImportDir(); importDir != "" {
+		importPath = paths.New(importDir)
 	} else {
-		importPath = paths.New(req.GetImportFile()).Parent()
-		importFile = paths.New(req.GetImportFile()).Base()
+		// TODO: Create a function to obtain importPath from sketch
+		importPath = sketch.FullPath
+		// Add FQBN (without configs part) to export path
+		fqbnSuffix := strings.Replace(fqbn.StringWithoutConfig(), ":", ".", -1)
+		importPath = importPath.Join("build").Join(fqbnSuffix)
 	}
-
-	outputTmpFile, ok := toolProperties.GetOk("recipe.output.tmp_file")
-	outputTmpFile = toolProperties.ExpandPropsInString(outputTmpFile)
-	if !ok {
-		return nil, fmt.Errorf("property 'recipe.output.tmp_file' not defined")
+	if !importPath.Exist() {
+		return nil, fmt.Errorf("compiled sketch not found in %s", importPath)
 	}
-	ext := filepath.Ext(outputTmpFile)
-	if strings.HasSuffix(importFile, ext) {
-		importFile = importFile[:len(importFile)-len(ext)]
+	if !importPath.IsDir() {
+		return nil, fmt.Errorf("expected compiled sketch in directory %s, but is a file instead", importPath)
 	}
-
 	toolProperties.SetPath("build.path", importPath)
-	toolProperties.Set("build.project_name", importFile)
-	uploadFile := importPath.Join(importFile + ext)
-	if _, err := uploadFile.Stat(); err != nil {
-		if os.IsNotExist(err) {
-			return nil, fmt.Errorf("compiled sketch %s not found", uploadFile.String())
-		}
-		return nil, errors.Wrap(err, "cannot open sketch")
-	}
+	toolProperties.Set("build.project_name", sketch.Name+".ino")
 
 	// Set debug port property
 	port := req.GetPort()

Diff for: commands/debug/debug_test.go

@@ -56,7 +56,7 @@ func TestGetCommandLine(t *testing.T) {
 		fmt.Sprintf(" %s/arduino-test/tools/openocd/0.10.0-arduino7/bin/openocd%s", dataDir, toolExtension) +
 		fmt.Sprintf(" -s \"%s/arduino-test/tools/openocd/0.10.0-arduino7/share/openocd/scripts/\"", dataDir) +
 		fmt.Sprintf(" --file \"%s/arduino-test/samd/variants/arduino_zero/openocd_scripts/arduino_zero.cfg\"", customHardware) +
-		fmt.Sprintf(" -c \"gdb_port pipe\" -c \"telnet_port 0\" -c init -c halt %s/hello.arduino-test.samd.arduino_zero_edbg.elf", sketchPath)
+		fmt.Sprintf(" -c \"gdb_port pipe\" -c \"telnet_port 0\" -c init -c halt %s/build/arduino-test.samd.arduino_zero_edbg/hello.ino.elf", sketchPath)
 
 	command, err := getCommandLine(req, pm)
 	assert.Nil(t, err)
@@ -77,7 +77,7 @@ func TestGetCommandLine(t *testing.T) {
 		fmt.Sprintf(" %s/arduino-test/tools/openocd/0.10.0-arduino7/bin/openocd%s", dataDir, toolExtension) +
 		fmt.Sprintf(" -s \"%s/arduino-test/tools/openocd/0.10.0-arduino7/share/openocd/scripts/\"", dataDir) +
 		fmt.Sprintf(" --file \"%s/arduino-test/samd/variants/mkr1000/openocd_scripts/arduino_zero.cfg\"", customHardware) +
-		fmt.Sprintf(" -c \"gdb_port pipe\" -c \"telnet_port 0\" -c init -c halt %s/hello.arduino-test.samd.mkr1000.elf", sketchPath)
+		fmt.Sprintf(" -c \"gdb_port pipe\" -c \"telnet_port 0\" -c init -c halt %s/build/arduino-test.samd.mkr1000/hello.ino.elf", sketchPath)
 
 	command2, err := getCommandLine(req2, pm)
 	assert.Nil(t, err)

Diff for: commands/debug/testdata/hello/hello.arduino-test.samd.arduino_zero_edbg.bin renamed to commands/debug/testdata/hello/build/arduino-test.samd.arduino_zero_edbg/hello.ino.bin

@@ -20,8 +20,6 @@ import (
 	"fmt"
 	"io"
 	"net/url"
-	"os"
-	"path/filepath"
 	"strings"
 	"time"
 
@@ -147,40 +145,25 @@ func Upload(ctx context.Context, req *rpc.UploadReq, outStream io.Writer, errStr
 		uploadProperties.Set("upload.verify", uploadProperties.Get("upload.params.noverify"))
 	}
 
-	// Set path to compiled binary
-	// Make the filename without the FQBN configs part
-	fqbn.Configs = properties.NewMap()
-	fqbnSuffix := strings.Replace(fqbn.String(), ":", ".", -1)
-
 	var importPath *paths.Path
-	var importFile string
-	if req.GetImportFile() == "" {
-		importPath = sketch.FullPath
-		importFile = sketch.Name + "." + fqbnSuffix
+	if importDir := req.GetImportDir(); importDir != "" {
+		importPath = paths.New(importDir)
 	} else {
-		importPath = paths.New(req.GetImportFile()).Parent()
-		importFile = paths.New(req.GetImportFile()).Base()
+		// TODO: Create a function to obtain importPath from sketch
+		importPath = sketch.FullPath
+		// Add FQBN (without configs part) to export path
+		fqbnSuffix := strings.Replace(fqbn.StringWithoutConfig(), ":", ".", -1)
+		importPath = importPath.Join("build").Join(fqbnSuffix)
 	}
 
-	outputTmpFile, ok := uploadProperties.GetOk("recipe.output.tmp_file")
-	outputTmpFile = uploadProperties.ExpandPropsInString(outputTmpFile)
-	if !ok {
-		return nil, fmt.Errorf("property 'recipe.output.tmp_file' not defined")
+	if !importPath.Exist() {
+		return nil, fmt.Errorf("compiled sketch not found in %s", importPath)
 	}
-	ext := filepath.Ext(outputTmpFile)
-	if strings.HasSuffix(importFile, ext) {
-		importFile = importFile[:len(importFile)-len(ext)]
+	if !importPath.IsDir() {
+		return nil, fmt.Errorf("expected compiled sketch in directory %s, but is a file instead", importPath)
 	}
-
 	uploadProperties.SetPath("build.path", importPath)
-	uploadProperties.Set("build.project_name", importFile)
-	uploadFile := importPath.Join(importFile + ext)
-	if _, err := uploadFile.Stat(); err != nil {
-		if os.IsNotExist(err) {
-			return nil, fmt.Errorf("compiled sketch %s not found", uploadFile.String())
-		}
-		return nil, fmt.Errorf("cannot open sketch: %s", err)
-	}
+	uploadProperties.Set("build.project_name", sketch.Name+".ino")
 
 	// Perform reset via 1200bps touch if requested
 	if uploadProperties.GetBoolean("upload.use_1200bps_touch") {