Skip to content

Commit

Permalink
Improved build path and project name auto-detection
Browse files Browse the repository at this point in the history
This should make the upload command compatibile with all the reasonable
usages.

See #764
See #641
  • Loading branch information
cmaglie committed Sep 7, 2020
1 parent 3df5b6b commit de6c443
Show file tree
Hide file tree
Showing 23 changed files with 225 additions and 35 deletions.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
154 changes: 119 additions & 35 deletions commands/upload/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"fmt"
"io"
"net/url"
"path/filepath"
"strings"
"time"

Expand All @@ -32,6 +33,7 @@ import (
rpc "github.com/arduino/arduino-cli/rpc/commands"
paths "github.com/arduino/go-paths-helper"
properties "github.com/arduino/go-properties-orderedmap"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"go.bug.st/serial"
)
Expand Down Expand Up @@ -240,43 +242,19 @@ func runProgramAction(pm *packagemanager.PackageManager,
uploadProperties.Set("program.verify", uploadProperties.Get("program.params.noverify"))
}

var importPath *paths.Path
if !burnBootloader {
if importFile != "" {
importFilePath := paths.New(importFile)
if !importFilePath.Exist() {
return fmt.Errorf("binary file not found in %s", importFilePath)
}
importPath = importFilePath.Parent()
// In general, the binary file extension (like .bin or .hex or even .zip) are already written explicitly in
// the core recipes inside platform.txt. This why the CLI removes it before setting the build.project_name
// property.
importFileName := strings.TrimSuffix(importFilePath.Base(), importFilePath.Ext())
uploadProperties.SetPath("build.path", importPath)
uploadProperties.Set("build.project_name", importFileName)
} else {
if sketch == nil {
return fmt.Errorf(("no sketch specified"))
}
if importDir != "" {
importPath = paths.New(importDir)
} else {
// 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)
}

if !importPath.Exist() {
return fmt.Errorf("compiled sketch not found in %s", importPath)
}
if !importPath.IsDir() {
return fmt.Errorf("expected compiled sketch in directory %s, but is a file instead", importPath)
}
uploadProperties.SetPath("build.path", importPath)
uploadProperties.Set("build.project_name", sketch.Name+".ino")
importPath, sketchName, err := determineBuildPathAndSketchName(importFile, importDir, sketch, fqbn)
if err != nil {
return errors.Errorf("retrieving build artifacts: %s", err)
}
if !importPath.Exist() {
return fmt.Errorf("compiled sketch not found in %s", importPath)
}
if !importPath.IsDir() {
return fmt.Errorf("expected compiled sketch in directory %s, but is a file instead", importPath)
}
uploadProperties.SetPath("build.path", importPath)
uploadProperties.Set("build.project_name", sketchName)
}

// If not using programmer perform some action required
Expand Down Expand Up @@ -463,3 +441,109 @@ func waitForNewSerialPort() (string, error) {

return "", nil
}

func determineBuildPathAndSketchName(importFile, importDir string, sketch *sketches.Sketch, fqbn *cores.FQBN) (*paths.Path, string, error) {
// In general, compiling a sketch will produce a set of files that are
// named as the sketch but have different extensions, for example Sketch.ino
// may produce: Sketch.ino.bin; Sketch.ino.hex; Sketch.ino.zip; etc...
// These files are created together in the build directory and anyone of
// them may be required for upload.

// The upload recipes are already written using the 'build.project_name' property
// concatenated with an explicit extension. To perform the upload we should now
// determine the project name (e.g. 'sketch.ino) and set the 'build.project_name'
// property accordingly, together with the 'build.path' property to point to the
// directory containing the build artifacts.

// Case 1: importFile flag has been specified
if importFile != "" {
if importDir != "" {
return nil, "", fmt.Errorf("importFile and importDir cannot be used together")
}

// We have a path like "path/to/my/build/SketchName.ino.bin". We are going to
// ignore the extension and set:
// - "build.path" as "path/to/my/build"
// - "build.project_name" as "SketchName.ino"

importFilePath := paths.New(importFile)
if !importFilePath.Exist() {
return nil, "", fmt.Errorf("binary file not found in %s", importFilePath)
}
return importFilePath.Parent(), strings.TrimSuffix(importFilePath.Base(), importFilePath.Ext()), nil
}

if importDir != "" {
// Case 2: importDir flag with a sketch
if sketch != nil {
// In this case we have both the build path and the sketch name given,
// so we just return them as-is
return paths.New(importDir), sketch.Name + ".ino", nil
}

// Case 3: importDir flag without a sketch

// In this case we have a build path but the sketch name is not given, we may
// try to determine the sketch name by applying some euristics to the build folder.
// - "build.path" as importDir
// - "build.project_name" after trying to autodetect it from the build folder.
buildPath := paths.New(importDir)
sketchName, err := detectSketchNameFromBuildPath(buildPath)
if err != nil {
return nil, "", errors.Errorf("autodetect build artifact: %s", err)
}
return buildPath, sketchName, nil
}

// Case 4: nothing given...
if sketch == nil {
return nil, "", fmt.Errorf("no sketch or build directory/file specified")
}

// Case 5: only sketch specified. In this case we use the default sketch build path
// and the given sketch name.

// TODO: Create a function to obtain importPath from sketch
// Add FQBN (without configs part) to export path
if fqbn == nil {
return nil, "", fmt.Errorf("missing FQBN")
}
fqbnSuffix := strings.Replace(fqbn.StringWithoutConfig(), ":", ".", -1)
return sketch.FullPath.Join("build").Join(fqbnSuffix), sketch.Name + ".ino", nil
}

func detectSketchNameFromBuildPath(buildPath *paths.Path) (string, error) {
files, err := buildPath.ReadDir()
if err != nil {
return "", err
}

candidateName := ""
var candidateFile *paths.Path
for _, file := range files {
// Build artifacts are usually names as "Blink.ino.hex" or "Blink.ino.bin".
// Extract the "Blink.ino" part
name := strings.TrimSuffix(file.Base(), file.Ext())

// Sometimes we may have particular files like:
// Blink.ino.with_bootloader.bin
if filepath.Ext(name) != ".ino" {
// just ignore those files
continue
}

if candidateName == "" {
candidateName = name
candidateFile = file
}

if candidateName != name {
return "", errors.Errorf("multiple build artifacts found: '%s' and '%s'", candidateFile, file)
}
}

if candidateName == "" {
return "", errors.New("could not find a valid build artifact")
}
return candidateName, nil
}
106 changes: 106 additions & 0 deletions commands/upload/upload_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// This file is part of arduino-cli.
//
// Copyright 2020 ARDUINO SA (http://www.arduino.cc/)
//
// This software is released under the GNU General Public License version 3,
// which covers the main part of arduino-cli.
// The terms of this license can be found at:
// https://www.gnu.org/licenses/gpl-3.0.en.html
//
// You can be released from the requirements of the above licenses by purchasing
// a commercial license. Buying such a license is mandatory if you want to
// modify or otherwise use the software for commercial activities involving the
// Arduino software without disclosing the source code of your own applications.
// To purchase a commercial license, send an email to license@arduino.cc.

package upload

import (
"fmt"
"testing"

"github.com/arduino/arduino-cli/arduino/cores"
"github.com/arduino/arduino-cli/arduino/sketches"
paths "github.com/arduino/go-paths-helper"
"github.com/stretchr/testify/require"
)

func TestDetectSketchNameFromBuildPath(t *testing.T) {
sk1, err1 := detectSketchNameFromBuildPath(paths.New("testdata/build_path_1"))
require.NoError(t, err1)
require.Equal(t, "sketch.ino", sk1)

sk2, err2 := detectSketchNameFromBuildPath(paths.New("testdata/build_path_2"))
require.NoError(t, err2)
require.Equal(t, "Blink.ino", sk2)

sk3, err3 := detectSketchNameFromBuildPath(paths.New("testdata/build_path_3"))
require.Error(t, err3)
require.Equal(t, "", sk3)
}

func TestDetermineBuildPathAndSketchName(t *testing.T) {
type test struct {
importFile string
importDir string
sketch *sketches.Sketch
fqbn *cores.FQBN
resBuildPath string
resSketchName string
hasError bool
}

blonk, err := sketches.NewSketchFromPath(paths.New("testdata/Blonk"))
require.NoError(t, err)

fqbn, err := cores.ParseFQBN("arduino:samd:mkr1000")
require.NoError(t, err)

tests := []test{
// 00: error: no data passed in
{"", "", nil, nil, "<nil>", "", true},
// 01: use importFile to detect build.path and project_name
{"testdata/build_path_2/Blink.ino.hex", "", nil, nil, "testdata/build_path_2", "Blink.ino", false},
// 02: use importPath as build.path and project_name
{"", "testdata/build_path_2", nil, nil, "testdata/build_path_2", "Blink.ino", false},
// 03: error: used both importPath and importFile
{"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", nil, nil, "<nil>", "", true},
// 04: error: only sketch without FQBN
{"", "", blonk, nil, "<nil>", "", true},
// 05: use importFile to detect build.path and project_name, sketch is ignored.
{"testdata/build_path_2/Blink.ino.hex", "", blonk, nil, "testdata/build_path_2", "Blink.ino", false},
// 06: use importPath as build.path and Blonk as project name (forced by the sketch)
{"", "testdata/build_path_2", blonk, nil, "testdata/build_path_2", "Blonk.ino", false},
// 07: error: used both importPath and importFile
{"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", blonk, nil, "<nil>", "", true},

// 08: error: no data passed in
{"", "", nil, fqbn, "<nil>", "", true},
// 09: use importFile to detect build.path and project_name, fqbn ignored
{"testdata/build_path_2/Blink.ino.hex", "", nil, fqbn, "testdata/build_path_2", "Blink.ino", false},
// 10: use importPath as build.path and project_name, fqbn ignored
{"", "testdata/build_path_2", nil, fqbn, "testdata/build_path_2", "Blink.ino", false},
// 11: error: used both importPath and importFile
{"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", nil, fqbn, "<nil>", "", true},
// 12: use sketch to determine project name and sketch+fqbn to determine build path
{"", "", blonk, fqbn, "testdata/Blonk/build/arduino.samd.mkr1000", "Blonk.ino", false},
// 13: use importFile to detect build.path and project_name, sketch+fqbn is ignored.
{"testdata/build_path_2/Blink.ino.hex", "", blonk, fqbn, "testdata/build_path_2", "Blink.ino", false},
// 14: use importPath as build.path and Blonk as project name (forced by the sketch), fqbn ignored
{"", "testdata/build_path_2", blonk, fqbn, "testdata/build_path_2", "Blonk.ino", false},
// 15: error: used both importPath and importFile
{"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", blonk, fqbn, "<nil>", "", true},
}
for i, test := range tests {
t.Run(fmt.Sprintf("SubTest%02d", i), func(t *testing.T) {
buildPath, sketchName, err := determineBuildPathAndSketchName(test.importFile, test.importDir, test.sketch, test.fqbn)
if test.hasError {
require.Error(t, err)
} else {
require.NoError(t, err)
}
require.Equal(t, test.resBuildPath, fmt.Sprint(buildPath))
require.Equal(t, test.resSketchName, sketchName)
})
}
}

0 comments on commit de6c443

Please sign in to comment.