Various fix and improvements to "upload"

commands/commands_test.go

@@ -26,11 +26,11 @@ import (
 	"testing"
 
 	"github.com/arduino/arduino-cli/commands/root"
-	"github.com/arduino/go-paths-helper"
+	paths "github.com/arduino/go-paths-helper"
 	"github.com/bouk/monkey"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"go.bug.st/relaxed-semver"
+	semver "go.bug.st/relaxed-semver"
 )
 
 // Redirecting stdOut so we can analyze output line by
@@ -110,11 +110,28 @@ func executeWithArgs(t *testing.T, args ...string) (int, []byte) {
 	return exitCode, output
 }
 
+var currDownloadDir *paths.Path
+
+func useSharedDownloadDir(t *testing.T) func() {
+	tmp := paths.TempDir().Join("arduino-cli-test-staging")
+	err := tmp.MkdirAll()
+	require.NoError(t, err, "making shared staging dir")
+	os.Setenv("ARDUINO_DOWNLOADS_DIR", tmp.String())
+	currDownloadDir = tmp
+	fmt.Printf("ARDUINO_DOWNLOADS_DIR = %s\n", os.Getenv("ARDUINO_DOWNLOADS_DIR"))
+
+	return func() {
+		os.Unsetenv("ARDUINO_DOWNLOADS_DIR")
+		currDownloadDir = nil
+		fmt.Printf("ARDUINO_DOWNLOADS_DIR = %s\n", os.Getenv("ARDUINO_DOWNLOADS_DIR"))
+	}
+}
+
 var currDataDir *paths.Path
 
 func makeTempDataDir(t *testing.T) func() {
 	tmp, err := paths.MkTempDir("", "test")
-	require.NoError(t, err, "making temporary staging dir")
+	require.NoError(t, err, "making temporary data dir")
 	os.Setenv("ARDUINO_DATA_DIR", tmp.String())
 	currDataDir = tmp
 	fmt.Printf("ARDUINO_DATA_DIR = %s\n", os.Getenv("ARDUINO_DATA_DIR"))
@@ -149,11 +166,98 @@ func makeTempSketchbookDir(t *testing.T) func() {
 	}
 }
 
+func setSketchbookDir(t *testing.T, tmp *paths.Path) func() {
+	os.Setenv("ARDUINO_SKETCHBOOK_DIR", tmp.String())
+	currSketchbookDir = tmp
+
+	fmt.Printf("ARDUINO_SKETCHBOOK_DIR = %s\n", os.Getenv("ARDUINO_SKETCHBOOK_DIR"))
+	return func() {
+		os.Unsetenv("ARDUINO_SKETCHBOOK_DIR")
+		currSketchbookDir = nil
+		fmt.Printf("ARDUINO_SKETCHBOOK_DIR = %s\n", os.Getenv("ARDUINO_SKETCHBOOK_DIR"))
+	}
+}
+
 // END -- Utility functions
 
+func TestUploadCommands(t *testing.T) {
+	defer makeTempDataDir(t)()
+	defer useSharedDownloadDir(t)()
+	defer setSketchbookDir(t, paths.New("testdata", "sketchbook_with_custom_hardware"))()
+
+	updateCoreIndex(t)
+
+	exitCode, _ := executeWithArgs(t, "core", "install", "arduino:avr")
+	require.Zero(t, exitCode, "exit code")
+
+	// -i flag
+	exitCode, d := executeWithArgs(t, "upload", "-i", currSketchbookDir.Join("test.hex").String(), "-b", "test:avr:testboard", "-p", "/dev/ttyACM0")
+	require.Zero(t, exitCode, "exit code")
+	require.Contains(t, string(d), "QUIET")
+	require.Contains(t, string(d), "NOVERIFY")
+	require.Contains(t, string(d), "testdata/sketchbook_with_custom_hardware/test.hex")
+
+	// -i flag with implicit extension
+	exitCode, d = executeWithArgs(t, "upload", "-i", currSketchbookDir.Join("test").String(), "-b", "test:avr:testboard", "-p", "/dev/ttyACM0")
+	require.Zero(t, exitCode, "exit code")
+	require.Contains(t, string(d), "QUIET")
+	require.Contains(t, string(d), "NOVERIFY")
+	require.Contains(t, string(d), "testdata/sketchbook_with_custom_hardware/test.hex")
+
+	// -i with absolute path
+	fullPath, err := currSketchbookDir.Join("test.hex").Abs()
+	require.NoError(t, err, "absolute path of test.hex")
+	exitCode, d = executeWithArgs(t, "upload", "-i", fullPath.String(), "-b", "test:avr:testboard", "-p", "/dev/ttyACM0")
+	require.Zero(t, exitCode, "exit code")
+	require.Contains(t, string(d), "QUIET")
+	require.Contains(t, string(d), "NOVERIFY")
+	require.Contains(t, string(d), "testdata/sketchbook_with_custom_hardware/test.hex")
+
+	// -v verbose
+	exitCode, d = executeWithArgs(t, "upload", "-v", "-i", currSketchbookDir.Join("test.hex").String(), "-b", "test:avr:testboard", "-p", "/dev/ttyACM0")
+	require.Zero(t, exitCode, "exit code")
+	require.Contains(t, string(d), "VERBOSE")
+	require.Contains(t, string(d), "NOVERIFY")
+	require.Contains(t, string(d), "testdata/sketchbook_with_custom_hardware/test.hex")
+
+	// -t verify
+	exitCode, d = executeWithArgs(t, "upload", "-t", "-i", currSketchbookDir.Join("test.hex").String(), "-b", "test:avr:testboard", "-p", "/dev/ttyACM0")
+	require.Zero(t, exitCode, "exit code")
+	require.Contains(t, string(d), "QUIET")
+	require.Contains(t, string(d), "VERIFY")
+	require.Contains(t, string(d), "testdata/sketchbook_with_custom_hardware/test.hex")
+
+	// -v -t verbose verify
+	exitCode, d = executeWithArgs(t, "upload", "-v", "-t", "-i", currSketchbookDir.Join("test.hex").String(), "-b", "test:avr:testboard", "-p", "/dev/ttyACM0")
+	require.Zero(t, exitCode, "exit code")
+	require.Contains(t, string(d), "VERBOSE")
+	require.Contains(t, string(d), "VERIFY")
+	require.Contains(t, string(d), "testdata/sketchbook_with_custom_hardware/test.hex")
+
+	// non-existent file
+	exitCode, _ = executeWithArgs(t, "upload", "-i", currSketchbookDir.Join("test123.hex").String(), "-b", "test:avr:testboard", "-p", "/dev/ttyACM0")
+	require.NotZero(t, exitCode, "exit code")
+
+	// sketch
+	exitCode, d = executeWithArgs(t, "upload", currSketchbookDir.Join("TestSketch").String(), "-b", "test:avr:testboard", "-p", "/dev/ttyACM0")
+	require.Zero(t, exitCode, "exit code")
+	require.Contains(t, string(d), "QUIET")
+	require.Contains(t, string(d), "NOVERIFY")
+	require.Contains(t, string(d), "testdata/sketchbook_with_custom_hardware/TestSketch/TestSketch.test.avr.testboard.hex")
+
+	// sketch without build
+	exitCode, _ = executeWithArgs(t, "upload", currSketchbookDir.Join("TestSketch2").String(), "-b", "test:avr:testboard", "-p", "/dev/ttyACM0")
+	require.NotZero(t, exitCode, "exit code")
+
+	// platform without 'recipe.output.tmp_file' property
+	exitCode, _ = executeWithArgs(t, "upload", "-i", currSketchbookDir.Join("test.hex").String(), "-b", "test2:avr:testboard", "-p", "/dev/ttyACM0")
+	require.NotZero(t, exitCode, "exit code")
+}
+
 func TestLibSearch(t *testing.T) {
 	defer makeTempDataDir(t)()
 	defer makeTempSketchbookDir(t)()
+	defer useSharedDownloadDir(t)()
 
 	exitCode, output := executeWithArgs(t, "lib", "search", "audiozer", "--format", "json")
 	require.Zero(t, exitCode, "process exit code")
@@ -184,6 +288,7 @@ func TestLibSearch(t *testing.T) {
 func TestUserLibs(t *testing.T) {
 	defer makeTempDataDir(t)()
 	defer makeTempSketchbookDir(t)()
+	defer useSharedDownloadDir(t)()
 	libDir := currSketchbookDir.Join("libraries")
 	err := libDir.MkdirAll()
 	require.NoError(t, err, "creating 'sketchbook/libraries' dir")
@@ -218,24 +323,22 @@ func TestUserLibs(t *testing.T) {
 func TestSketchCommands(t *testing.T) {
 	defer makeTempDataDir(t)()
 	defer makeTempSketchbookDir(t)()
-	//var d []byte
-	var exitCode int
+	defer useSharedDownloadDir(t)()
 
-	exitCode, _ = executeWithArgs(t, "sketch", "new", "Test")
+	exitCode, _ := executeWithArgs(t, "sketch", "new", "Test")
 	require.Zero(t, exitCode, "exit code")
 }
 
 func TestLibDownloadAndInstall(t *testing.T) {
 	defer makeTempDataDir(t)()
 	defer makeTempSketchbookDir(t)()
-	var d []byte
-	var exitCode int
+	defer useSharedDownloadDir(t)()
 
-	exitCode, _ = executeWithArgs(t, "core", "update-index")
+	exitCode, _ := executeWithArgs(t, "core", "update-index")
 	require.Zero(t, exitCode, "exit code")
 
 	// Download inexistent
-	exitCode, d = executeWithArgs(t, "lib", "download", "inexistentLibrary", "--format", "json")
+	exitCode, d := executeWithArgs(t, "lib", "download", "inexistentLibrary", "--format", "json")
 	require.NotZero(t, exitCode, "exit code")
 	require.Contains(t, string(d), "library inexistentLibrary not found")
 
@@ -372,6 +475,7 @@ func detectLatestAVRCore(t *testing.T) string {
 func TestCompileCommands(t *testing.T) {
 	defer makeTempDataDir(t)()
 	defer makeTempSketchbookDir(t)()
+	defer useSharedDownloadDir(t)()
 
 	// Set staging dir to a temporary dir
 	tmp, err := ioutil.TempDir(os.TempDir(), "test")
@@ -429,6 +533,7 @@ func TestCompileCommands(t *testing.T) {
 func TestInvalidCoreURL(t *testing.T) {
 	defer makeTempDataDir(t)()
 	defer makeTempSketchbookDir(t)()
+	defer useSharedDownloadDir(t)()
 
 	tmp, err := paths.MkTempDir("", "")
 	require.NoError(t, err, "making temporary dir")
@@ -465,27 +570,23 @@ board_manager:
 func TestCoreCommands(t *testing.T) {
 	defer makeTempDataDir(t)()
 	defer makeTempSketchbookDir(t)()
-
-	// Set staging dir to a temporary dir
-	tmp, err := ioutil.TempDir(os.TempDir(), "test")
-	require.NoError(t, err, "making temporary staging dir")
-	defer os.RemoveAll(tmp)
+	defer useSharedDownloadDir(t)()
 
 	updateCoreIndex(t)
 	AVR := "arduino:avr@" + detectLatestAVRCore(t)
 
 	// Download a specific core version
 	exitCode, d := executeWithArgs(t, "core", "download", "arduino:[email protected]")
 	require.Zero(t, exitCode, "exit code")
-	require.Contains(t, string(d), "arduino:[email protected] downloaded")
-	require.Contains(t, string(d), "arduino:[email protected] downloaded")
-	require.Contains(t, string(d), "arduino:[email protected] downloaded")
-	require.Contains(t, string(d), "arduino:[email protected] downloaded")
+	require.Regexp(t, "arduino:[email protected] (already )?downloaded", string(d))
+	require.Regexp(t, "arduino:[email protected] (already )?downloaded", string(d))
+	require.Regexp(t, "arduino:[email protected] (already )?downloaded", string(d))
+	require.Regexp(t, "arduino:[email protected] (already )?downloaded", string(d))
 
 	// Download latest
 	exitCode, d = executeWithArgs(t, "core", "download", "arduino:avr")
 	require.Zero(t, exitCode, "exit code")
-	require.Contains(t, string(d), AVR+" downloaded")
+	require.Regexp(t, AVR+" (already )?downloaded", string(d))
 
 	// Wrong downloads
 	exitCode, d = executeWithArgs(t, "core", "download", "arduino:[email protected]")

commands/testdata/sketchbook_with_custom_hardware/hardware/test/avr/boards.txt

@@ -0,0 +1,25 @@
+testboard.name=Test Board
+
+testboard.vid.0=0x2341
+testboard.pid.0=0x8888
+
+testboard.upload.tool=avrdude-none
+testboard.upload.protocol=arduino
+testboard.upload.maximum_size=32256
+testboard.upload.maximum_data_size=2048
+testboard.upload.speed=115200
+
+testboard.bootloader.tool=avrdude
+testboard.bootloader.low_fuses=0xFF
+testboard.bootloader.high_fuses=0xDE
+testboard.bootloader.extended_fuses=0xFD
+testboard.bootloader.unlock_bits=0x3F
+testboard.bootloader.lock_bits=0x0F
+testboard.bootloader.file=optiboot/optiboot_atmega328.hex
+
+testboard.build.mcu=atmega328p
+testboard.build.f_cpu=16000000L
+testboard.build.board=AVR_UNO
+testboard.build.core=arduino:arduino
+testboard.build.variant=standard
+

commands/testdata/sketchbook_with_custom_hardware/hardware/test/avr/platform.txt

@@ -0,0 +1,13 @@
+
+name=Test AVR Boards
+version=1.0.0
+
+recipe.output.tmp_file={build.project_name}.hex
+
+# Fake AVR programmer tool for testing
+# ------------------------------------
+tools.avrdude-none.upload.params.verbose=VERBOSE
+tools.avrdude-none.upload.params.quiet=QUIET
+tools.avrdude-none.upload.params.verify=VERIFY
+tools.avrdude-none.upload.params.noverify=NOVERIFY
+tools.avrdude-none.upload.pattern=echo {upload.verbose} {upload.verify} "{build.path}/{build.project_name}.hex"

commands/testdata/sketchbook_with_custom_hardware/hardware/test2/avr/boards.txt

@@ -0,0 +1,25 @@
+testboard.name=Test Board
+
+testboard.vid.0=0x2341
+testboard.pid.0=0x8888
+
+testboard.upload.tool=avrdude-none
+testboard.upload.protocol=arduino
+testboard.upload.maximum_size=32256
+testboard.upload.maximum_data_size=2048
+testboard.upload.speed=115200
+
+testboard.bootloader.tool=avrdude
+testboard.bootloader.low_fuses=0xFF
+testboard.bootloader.high_fuses=0xDE
+testboard.bootloader.extended_fuses=0xFD
+testboard.bootloader.unlock_bits=0x3F
+testboard.bootloader.lock_bits=0x0F
+testboard.bootloader.file=optiboot/optiboot_atmega328.hex
+
+testboard.build.mcu=atmega328p
+testboard.build.f_cpu=16000000L
+testboard.build.board=AVR_UNO
+testboard.build.core=arduino:arduino
+testboard.build.variant=standard
+

commands/testdata/sketchbook_with_custom_hardware/hardware/test2/avr/platform.txt

@@ -0,0 +1,14 @@
+
+name=Test AVR Boards
+version=1.0.0
+
+# Removed for testing purposes
+#recipe.output.tmp_file={build.project_name}.hex
+
+# Fake AVR programmer tool for testing
+# ------------------------------------
+tools.avrdude-none.upload.params.verbose=VERBOSE
+tools.avrdude-none.upload.params.quiet=QUIET
+tools.avrdude-none.upload.params.verify=VERIFY
+tools.avrdude-none.upload.params.noverify=NOVERIFY
+tools.avrdude-none.upload.pattern=echo {upload.verbose} {upload.verify} "{build.path}/{build.project_name}.hex"

commands/testdata/sketchbook_with_custom_hardware/test.hex

@@ -0,0 +1 @@
+This is a fake .hex file for testing purposes

commands/upload/upload.go

@@ -112,43 +112,30 @@ func run(command *cobra.Command, args []string) {
 	}
 
 	// Load programmer tool
-	uploadToolID, have := boardProperties.GetOk("upload.tool")
-	if !have || uploadToolID == "" {
-		formatter.PrintErrorMessage("The board defines an invalid 'upload.tool': " + uploadToolID)
+	uploadToolPattern, have := boardProperties.GetOk("upload.tool")
+	if !have || uploadToolPattern == "" {
+		formatter.PrintErrorMessage("The board does not define an 'upload.tool' property.")
 		os.Exit(commands.ErrGeneric)
 	}
 
-	var referencedPackage *cores.Package
-	var referencedPlatform *cores.Platform
 	var referencedPlatformRelease *cores.PlatformRelease
-	var uploadTool *cores.Tool
-	if split := strings.Split(uploadToolID, ":"); len(split) == 1 {
-		uploadTool = board.PlatformRelease.Platform.Package.Tools[uploadToolID]
+	if split := strings.Split(uploadToolPattern, ":"); len(split) > 2 {
+		formatter.PrintErrorMessage("The board defines an invalid 'upload.tool' property: " + uploadToolPattern)
+		os.Exit(commands.ErrGeneric)
 	} else if len(split) == 2 {
-		referencedPackage = pm.GetPackages().Packages[split[0]]
-		if referencedPackage == nil {
-			formatter.PrintErrorMessage("The board requires a tool from package '" + split[0] + "' that is not installed: " + uploadToolID)
-			os.Exit(commands.ErrGeneric)
-		}
-		uploadTool = referencedPackage.Tools[split[1]]
+		referencedPackageName := split[0]
+		uploadToolPattern = split[1]
+		architecture := board.PlatformRelease.Platform.Architecture
 
-		referencedPlatform = referencedPackage.Platforms[board.PlatformRelease.Platform.Architecture]
-		if referencedPlatform != nil {
+		if referencedPackage := pm.GetPackages().Packages[referencedPackageName]; referencedPackage == nil {
+			formatter.PrintErrorMessage("The board requires platform '" + referencedPackageName + ":" + architecture + "' that is not installed.")
+			os.Exit(commands.ErrGeneric)
+		} else if referencedPlatform := referencedPackage.Platforms[architecture]; referencedPlatform == nil {
+			formatter.PrintErrorMessage("The board requires platform '" + referencedPackageName + ":" + architecture + "' that is not installed.")
+			os.Exit(commands.ErrGeneric)
+		} else {
 			referencedPlatformRelease = pm.GetInstalledPlatformRelease(referencedPlatform)
 		}
-	} else {
-		formatter.PrintErrorMessage("The board defines an invalid 'upload.tool': " + uploadToolID)
-		os.Exit(commands.ErrGeneric)
-	}
-	if uploadTool == nil {
-		formatter.PrintErrorMessage("Upload tool '" + uploadToolID + "' not found.")
-		os.Exit(commands.ErrGeneric)
-	}
-	// FIXME: Look into index if the platform requires a specific version
-	uploadToolRelease := uploadTool.GetLatestInstalled()
-	if uploadToolRelease == nil {
-		formatter.PrintErrorMessage("Upload tool '" + uploadToolID + "' not installed.")
-		os.Exit(commands.ErrGeneric)
 	}
 
 	// Build configuration for upload
@@ -160,7 +147,7 @@ func run(command *cobra.Command, args []string) {
 	uploadProperties.Merge(board.PlatformRelease.RuntimeProperties())
 	uploadProperties.Merge(boardProperties)
 
-	uploadToolProperties := uploadProperties.SubTree("tools." + uploadTool.Name)
+	uploadToolProperties := uploadProperties.SubTree("tools." + uploadToolPattern)
 	uploadProperties.Merge(uploadToolProperties)
 
 	if requiredTools, err := pm.FindToolsRequiredForBoard(board); err == nil {
@@ -191,7 +178,12 @@ func run(command *cobra.Command, args []string) {
 	// Make the filename without the FQBN configs part
 	fqbn.Configs = properties.NewMap()
 	fqbnSuffix := strings.Replace(fqbn.String(), ":", ".", -1)
-	ext := filepath.Ext(uploadProperties.ExpandPropsInString("{recipe.output.tmp_file}"))
+	outputTmpFile, ok := uploadProperties.GetOk("recipe.output.tmp_file")
+	if !ok {
+		formatter.PrintErrorMessage("The platform does not define the required property 'recipe.output.tmp_file'.")
+		os.Exit(commands.ErrGeneric)
+	}
+	ext := filepath.Ext(outputTmpFile)
 
 	var importPath *paths.Path
 	var importFile string