Add back support for sketches with .pde extension and deprecate it (#1157)

* Compile command now works with sketches containing .pde files

* Upload command now works with sketches and builds from .pde files

* Archive command now works with sketches containing .pde files

* [skip changelog] Add test to verify debug command works with pde sketches

* Fix lib examples not showing sketches with .pde files

* [skip changelog] Remove duplicated code and enhance tests

Author: silvanocerza

arduino/globals/globals.go

@@ -18,9 +18,13 @@ package globals
 var (
 	empty struct{}
 
+	// MainFileValidExtension is the extension that must be used for files in new sketches
+	MainFileValidExtension string = ".ino"
+
 	// MainFileValidExtensions lists valid extensions for a sketch file
 	MainFileValidExtensions = map[string]struct{}{
-		".ino": empty,
+		MainFileValidExtension: empty,
+		// .pde extension is deprecated and must not be used for new sketches
 		".pde": empty,
 	}
 

arduino/libraries/loader.go

@@ -19,6 +19,7 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/arduino/arduino-cli/arduino/sketches"
 	"github.com/arduino/go-paths-helper"
 	properties "github.com/arduino/go-properties-orderedmap"
 	"github.com/pkg/errors"
@@ -172,7 +173,8 @@ func addExamplesToPathList(examplesPath *paths.Path, list *paths.PathList) error
 		return err
 	}
 	for _, file := range files {
-		if isExample(file) {
+		_, err := sketches.NewSketchFromPath(file)
+		if err == nil {
 			list.Add(file)
 		} else if file.IsDir() {
 			if err := addExamplesToPathList(file, list); err != nil {
@@ -182,9 +184,3 @@ func addExamplesToPathList(examplesPath *paths.Path, list *paths.PathList) error
 	}
 	return nil
 }
-
-// isExample returns true if examplePath contains an example
-func isExample(examplePath *paths.Path) bool {
-	mainIno := examplePath.Join(examplePath.Base() + ".ino")
-	return mainIno.Exist() && mainIno.IsNotDir()
-}

arduino/sketches/sketches.go

@@ -20,15 +20,17 @@ import (
 	"fmt"
 
 	"github.com/arduino/arduino-cli/arduino/builder"
+	"github.com/arduino/arduino-cli/arduino/globals"
 	"github.com/arduino/go-paths-helper"
 	"github.com/pkg/errors"
 )
 
 // Sketch is a sketch for Arduino
 type Sketch struct {
-	Name     string
-	FullPath *paths.Path
-	Metadata *Metadata
+	Name              string
+	MainFileExtension string
+	FullPath          *paths.Path
+	Metadata          *Metadata
 }
 
 // Metadata is the kind of data associated to a project such as the connected board
@@ -52,14 +54,32 @@ func NewSketchFromPath(path *paths.Path) (*Sketch, error) {
 	if !path.IsDir() {
 		path = path.Parent()
 	}
-	sketchFile := path.Join(path.Base() + ".ino")
-	if !sketchFile.Exist() {
-		return nil, errors.Errorf("no valid sketch found in %s: missing %s", path, sketchFile.Base())
+
+	var mainSketchFile *paths.Path
+	for ext := range globals.MainFileValidExtensions {
+		candidateSketchMainFile := path.Join(path.Base() + ext)
+		if candidateSketchMainFile.Exist() {
+			if mainSketchFile == nil {
+				mainSketchFile = candidateSketchMainFile
+			} else {
+				return nil, errors.Errorf("multiple main sketch files found (%v, %v)",
+					mainSketchFile,
+					candidateSketchMainFile,
+				)
+			}
+		}
+	}
+
+	if mainSketchFile == nil {
+		sketchFile := path.Join(path.Base() + globals.MainFileValidExtension)
+		return nil, errors.Errorf("no valid sketch found in %s: missing %s", path, sketchFile)
 	}
+
 	sketch := &Sketch{
-		FullPath: path,
-		Name:     path.Base(),
-		Metadata: &Metadata{},
+		FullPath:          path,
+		MainFileExtension: mainSketchFile.Ext(),
+		Name:              path.Base(),
+		Metadata:          &Metadata{},
 	}
 	sketch.ImportMetadata()
 	return sketch, nil
@@ -108,3 +128,20 @@ func (s *Sketch) BuildPath() (*paths.Path, error) {
 	}
 	return builder.GenBuildPath(s.FullPath), nil
 }
+
+// CheckForPdeFiles returns all files ending with .pde extension
+// in dir, this is mainly used to warn the user that these files
+// must be changed to .ino extension.
+// When .pde files won't be supported anymore this function must be removed.
+func CheckForPdeFiles(sketch *paths.Path) []*paths.Path {
+	if sketch.IsNotDir() {
+		sketch = sketch.Parent()
+	}
+
+	files, err := sketch.ReadDirRecursive()
+	if err != nil {
+		return []*paths.Path{}
+	}
+	files.FilterSuffix(".pde")
+	return files
+}

arduino/sketches/sketches_test.go

@@ -53,9 +53,59 @@ func TestSketchBuildPath(t *testing.T) {
 	require.NoError(t, err)
 	require.Contains(t, buildPath.String(), "arduino-sketch-")
 
+	// Verifies sketch path is returned if sketch has .pde extension
+	sketchPath = paths.New("testdata", "SketchPde")
+	sketch, err = NewSketchFromPath(sketchPath)
+	require.NoError(t, err)
+	require.NotNil(t, sketch)
+	buildPath, err = sketch.BuildPath()
+	require.NoError(t, err)
+	require.Contains(t, buildPath.String(), "arduino-sketch-")
+
+	// Verifies error is returned if there are multiple main files
+	sketchPath = paths.New("testdata", "SketchMultipleMainFiles")
+	sketch, err = NewSketchFromPath(sketchPath)
+	require.Nil(t, sketch)
+	require.Error(t, err, "multiple main sketch files found")
+
 	// Verifies error is returned if sketch path is not set
 	sketch = &Sketch{}
 	buildPath, err = sketch.BuildPath()
 	require.Nil(t, buildPath)
 	require.Error(t, err, "sketch path is empty")
 }
+
+func TestCheckForPdeFiles(t *testing.T) {
+	sketchPath := paths.New("testdata", "Sketch1")
+	files := CheckForPdeFiles(sketchPath)
+	require.Empty(t, files)
+
+	sketchPath = paths.New("testdata", "SketchPde")
+	files = CheckForPdeFiles(sketchPath)
+	require.Len(t, files, 1)
+	require.Equal(t, sketchPath.Join("SketchPde.pde"), files[0])
+
+	sketchPath = paths.New("testdata", "SketchMultipleMainFiles")
+	files = CheckForPdeFiles(sketchPath)
+	require.Len(t, files, 1)
+	require.Equal(t, sketchPath.Join("SketchMultipleMainFiles.pde"), files[0])
+
+	sketchPath = paths.New("testdata", "Sketch1", "Sketch1.ino")
+	files = CheckForPdeFiles(sketchPath)
+	require.Empty(t, files)
+
+	sketchPath = paths.New("testdata", "SketchPde", "SketchPde.pde")
+	files = CheckForPdeFiles(sketchPath)
+	require.Len(t, files, 1)
+	require.Equal(t, sketchPath.Parent().Join("SketchPde.pde"), files[0])
+
+	sketchPath = paths.New("testdata", "SketchMultipleMainFiles", "SketchMultipleMainFiles.ino")
+	files = CheckForPdeFiles(sketchPath)
+	require.Len(t, files, 1)
+	require.Equal(t, sketchPath.Parent().Join("SketchMultipleMainFiles.pde"), files[0])
+
+	sketchPath = paths.New("testdata", "SketchMultipleMainFiles", "SketchMultipleMainFiles.pde")
+	files = CheckForPdeFiles(sketchPath)
+	require.Len(t, files, 1)
+	require.Equal(t, sketchPath.Parent().Join("SketchMultipleMainFiles.pde"), files[0])
+}

arduino/sketches/testdata/SketchMultipleMainFiles/SketchMultipleMainFiles.ino

@@ -0,0 +1,3 @@
+
+void setup() {}
+void loop() {}

arduino/sketches/testdata/SketchMultipleMainFiles/SketchMultipleMainFiles.pde

@@ -0,0 +1,3 @@
+
+void setup() {}
+void loop() {}

arduino/sketches/testdata/SketchPde/SketchPde.pde

@@ -0,0 +1,3 @@
+
+void setup() {}
+void loop() {}

cli/compile/compile.go

@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"os"
 
+	"github.com/arduino/arduino-cli/arduino/sketches"
 	"github.com/arduino/arduino-cli/cli/feedback"
 	"github.com/arduino/arduino-cli/cli/output"
 	"github.com/arduino/arduino-cli/configuration"
@@ -127,6 +128,15 @@ func run(cmd *cobra.Command, args []string) {
 	}
 
 	sketchPath := initSketchPath(path)
+
+	// .pde files are still supported but deprecated, this warning urges the user to rename them
+	if files := sketches.CheckForPdeFiles(sketchPath); len(files) > 0 {
+		feedback.Error("Sketches with .pde extension are deprecated, please rename the following files to .ino:")
+		for _, f := range files {
+			feedback.Error(f)
+		}
+	}
+
 	// We must read this from settings since the value is set when the binding is accessed from viper,
 	// accessing it from cobra would only read it if the flag is explicitly set by the user and ignore
 	// the config file and the env vars.

cli/sketch/archive.go

@@ -19,10 +19,12 @@ import (
 	"context"
 	"os"
 
+	"github.com/arduino/arduino-cli/arduino/sketches"
 	"github.com/arduino/arduino-cli/cli/errorcodes"
 	"github.com/arduino/arduino-cli/cli/feedback"
 	"github.com/arduino/arduino-cli/commands/sketch"
 	rpc "github.com/arduino/arduino-cli/rpc/commands"
+	"github.com/arduino/go-paths-helper"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 )
@@ -53,11 +55,19 @@ func initArchiveCommand() *cobra.Command {
 func runArchiveCommand(cmd *cobra.Command, args []string) {
 	logrus.Info("Executing `arduino sketch archive`")
 
-	sketchPath := ""
+	sketchPath := "."
 	if len(args) >= 1 {
 		sketchPath = args[0]
 	}
 
+	// .pde files are still supported but deprecated, this warning urges the user to rename them
+	if files := sketches.CheckForPdeFiles(paths.New(sketchPath)); len(files) > 0 {
+		feedback.Error("Sketches with .pde extension are deprecated, please rename the following files to .ino:")
+		for _, f := range files {
+			feedback.Error(f)
+		}
+	}
+
 	archivePath := ""
 	if len(args) == 2 {
 		archivePath = args[1]

cli/upload/upload.go

@@ -19,6 +19,7 @@ import (
 	"context"
 	"os"
 
+	"github.com/arduino/arduino-cli/arduino/sketches"
 	"github.com/arduino/arduino-cli/cli/errorcodes"
 	"github.com/arduino/arduino-cli/cli/feedback"
 	"github.com/arduino/arduino-cli/cli/instance"
@@ -83,6 +84,14 @@ func run(command *cobra.Command, args []string) {
 	}
 	sketchPath := initSketchPath(path)
 
+	// .pde files are still supported but deprecated, this warning urges the user to rename them
+	if files := sketches.CheckForPdeFiles(sketchPath); len(files) > 0 {
+		feedback.Error("Sketches with .pde extension are deprecated, please rename the following files to .ino:")
+		for _, f := range files {
+			feedback.Error(f)
+		}
+	}
+
 	if _, err := upload.Upload(context.Background(), &rpc.UploadReq{
 		Instance:   instance,
 		Fqbn:       fqbn,

commands/sketch/archive.go

@@ -23,6 +23,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/arduino/arduino-cli/arduino/sketches"
 	rpc "github.com/arduino/arduino-cli/rpc/commands"
 	paths "github.com/arduino/go-paths-helper"
 )
@@ -37,27 +38,17 @@ func ArchiveSketch(ctx context.Context, req *rpc.ArchiveSketchReq) (*rpc.Archive
 		sketchPath = paths.New(".")
 	}
 
-	sketchPath, err := sketchPath.Clean().Abs()
+	sketch, err := sketches.NewSketchFromPath(sketchPath)
 	if err != nil {
-		return nil, fmt.Errorf("Error getting absolute sketch path %v", err)
+		return nil, err
 	}
 
-	// Get the sketch name and make sketchPath point to the ino file
-	if sketchPath.IsDir() {
-		sketchName = sketchPath.Base()
-		sketchPath = sketchPath.Join(sketchName + ".ino")
-	} else if sketchPath.Ext() == ".ino" {
-		sketchName = strings.TrimSuffix(sketchPath.Base(), ".ino")
-	}
-
-	// Checks if it's really a sketch
-	if sketchPath.NotExist() {
-		return nil, fmt.Errorf("specified path is not a sketch: %v", sketchPath.String())
-	}
+	sketchPath = sketch.FullPath
+	sketchName = sketch.Name
 
 	archivePath := paths.New(req.ArchivePath)
 	if archivePath == nil {
-		archivePath = sketchPath.Parent().Parent()
+		archivePath = sketchPath.Parent()
 	}
 
 	archivePath, err = archivePath.Clean().Abs()
@@ -76,7 +67,7 @@ func ArchiveSketch(ctx context.Context, req *rpc.ArchiveSketchReq) (*rpc.Archive
 		return nil, fmt.Errorf("archive already exists")
 	}
 
-	filesToZip, err := sketchPath.Parent().ReadDirRecursive()
+	filesToZip, err := sketchPath.ReadDirRecursive()
 	if err != nil {
 		return nil, fmt.Errorf("Error retrieving sketch files: %v", err)
 	}
@@ -94,7 +85,7 @@ func ArchiveSketch(ctx context.Context, req *rpc.ArchiveSketchReq) (*rpc.Archive
 	for _, f := range filesToZip {
 
 		if !req.IncludeBuildDir {
-			filePath, err := sketchPath.Parent().Parent().RelTo(f)
+			filePath, err := sketchPath.Parent().RelTo(f)
 			if err != nil {
 				return nil, fmt.Errorf("Error calculating relative file path: %v", err)
 			}
@@ -107,7 +98,7 @@ func ArchiveSketch(ctx context.Context, req *rpc.ArchiveSketchReq) (*rpc.Archive
 
 		// We get the parent path since we want the archive to unpack as a folder.
 		// If we don't do this the archive would contain all the sketch files as top level.
-		err = addFileToSketchArchive(zipWriter, f, sketchPath.Parent().Parent())
+		err = addFileToSketchArchive(zipWriter, f, sketchPath.Parent())
 		if err != nil {
 			return nil, fmt.Errorf("Error adding file to archive: %v", err)
 		}

commands/upload/upload.go

@@ -26,6 +26,7 @@ import (
 	bldr "github.com/arduino/arduino-cli/arduino/builder"
 	"github.com/arduino/arduino-cli/arduino/cores"
 	"github.com/arduino/arduino-cli/arduino/cores/packagemanager"
+	"github.com/arduino/arduino-cli/arduino/globals"
 	"github.com/arduino/arduino-cli/arduino/serialutils"
 	"github.com/arduino/arduino-cli/arduino/sketches"
 	"github.com/arduino/arduino-cli/commands"
@@ -452,7 +453,7 @@ func determineBuildPathAndSketchName(importFile, importDir string, sketch *sketc
 
 	// Case 4: only sketch specified. In this case we use the generated build path
 	// and the given sketch name.
-	return bldr.GenBuildPath(sketch.FullPath), sketch.Name + ".ino", nil
+	return bldr.GenBuildPath(sketch.FullPath), sketch.Name + sketch.MainFileExtension, nil
 }
 
 func detectSketchNameFromBuildPath(buildPath *paths.Path) (string, error) {
@@ -462,11 +463,13 @@ func detectSketchNameFromBuildPath(buildPath *paths.Path) (string, error) {
 	}
 
 	if absBuildPath, err := buildPath.Abs(); err == nil {
-		candidateName := absBuildPath.Base() + ".ino"
-		f := files.Clone()
-		f.FilterPrefix(candidateName + ".")
-		if f.Len() > 0 {
-			return candidateName, nil
+		for ext := range globals.MainFileValidExtensions {
+			candidateName := absBuildPath.Base() + ext
+			f := files.Clone()
+			f.FilterPrefix(candidateName + ".")
+			if f.Len() > 0 {
+				return candidateName, nil
+			}
 		}
 	}
 
@@ -479,7 +482,7 @@ func detectSketchNameFromBuildPath(buildPath *paths.Path) (string, error) {
 
 		// Sometimes we may have particular files like:
 		// Blink.ino.with_bootloader.bin
-		if filepath.Ext(name) != ".ino" {
+		if _, ok := globals.MainFileValidExtensions[filepath.Ext(name)]; !ok {
 			// just ignore those files
 			continue
 		}