From 169f3bf697c1465347b045c13f68b261f7d268ed Mon Sep 17 00:00:00 2001 From: Silvano Cerza Date: Wed, 27 Jan 2021 15:51:41 +0100 Subject: [PATCH 1/6] Compile command now works with sketches containing .pde files --- arduino/globals/globals.go | 6 +- arduino/sketches/sketches.go | 59 ++++++++++++++--- arduino/sketches/sketches_test.go | 50 ++++++++++++++ .../SketchMultipleMainFiles.ino | 3 + .../SketchMultipleMainFiles.pde | 3 + .../sketches/testdata/SketchPde/SketchPde.pde | 3 + cli/compile/compile.go | 10 +++ docs/sketch-specification.md | 7 +- test/test_compile.py | 66 +++++++++++++++++++ 9 files changed, 195 insertions(+), 12 deletions(-) create mode 100644 arduino/sketches/testdata/SketchMultipleMainFiles/SketchMultipleMainFiles.ino create mode 100644 arduino/sketches/testdata/SketchMultipleMainFiles/SketchMultipleMainFiles.pde create mode 100644 arduino/sketches/testdata/SketchPde/SketchPde.pde diff --git a/arduino/globals/globals.go b/arduino/globals/globals.go index a7802d6dff2..b58d00f3e0e 100644 --- a/arduino/globals/globals.go +++ b/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, } diff --git a/arduino/sketches/sketches.go b/arduino/sketches/sketches.go index fef80969b08..4aefd2c990a 100644 --- a/arduino/sketches/sketches.go +++ b/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,24 @@ 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() + } + + // It's not a problem if we can't read files right now + files, _ := sketch.ReadDirRecursive() + files.FilterOutDirs() + res := []*paths.Path{} + for _, f := range files { + if f.Ext() == ".pde" { + res = append(res, f) + } + } + return res +} diff --git a/arduino/sketches/sketches_test.go b/arduino/sketches/sketches_test.go index 65a30c37cea..49ea771e4fd 100644 --- a/arduino/sketches/sketches_test.go +++ b/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]) +} diff --git a/arduino/sketches/testdata/SketchMultipleMainFiles/SketchMultipleMainFiles.ino b/arduino/sketches/testdata/SketchMultipleMainFiles/SketchMultipleMainFiles.ino new file mode 100644 index 00000000000..5054c040393 --- /dev/null +++ b/arduino/sketches/testdata/SketchMultipleMainFiles/SketchMultipleMainFiles.ino @@ -0,0 +1,3 @@ + +void setup() {} +void loop() {} diff --git a/arduino/sketches/testdata/SketchMultipleMainFiles/SketchMultipleMainFiles.pde b/arduino/sketches/testdata/SketchMultipleMainFiles/SketchMultipleMainFiles.pde new file mode 100644 index 00000000000..5054c040393 --- /dev/null +++ b/arduino/sketches/testdata/SketchMultipleMainFiles/SketchMultipleMainFiles.pde @@ -0,0 +1,3 @@ + +void setup() {} +void loop() {} diff --git a/arduino/sketches/testdata/SketchPde/SketchPde.pde b/arduino/sketches/testdata/SketchPde/SketchPde.pde new file mode 100644 index 00000000000..5054c040393 --- /dev/null +++ b/arduino/sketches/testdata/SketchPde/SketchPde.pde @@ -0,0 +1,3 @@ + +void setup() {} +void loop() {} diff --git a/cli/compile/compile.go b/cli/compile/compile.go index e77b3fb5740..4b095cdf592 100644 --- a/cli/compile/compile.go +++ b/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. diff --git a/docs/sketch-specification.md b/docs/sketch-specification.md index fb1a59060e6..f1f2dd9c292 100644 --- a/docs/sketch-specification.md +++ b/docs/sketch-specification.md @@ -18,7 +18,10 @@ Support for sketch folder names starting with a number was added in Arduino IDE ### Primary sketch file -Every sketch must contain a .ino or .pde file with a file name matching the sketch root folder name. +Every sketch must contain a `.ino` file with a file name matching the sketch root folder name. + +`.pde` is also supported but **deprecated** and will be removed in the future, using the `.ino` extension is strongly +recommended. ### Additional code files @@ -28,7 +31,7 @@ The following extensions are supported: - .ino - [Arduino language](https://www.arduino.cc/reference/en/) files. - .pde - Alternate extension for Arduino language files. This file extension is also used by Processing sketches. .ino - is recommended to avoid confusion. + is recommended to avoid confusion. **`.pde` extension is deprecated and will be removed in the future.** - .cpp - C++ files. - .c - C Files. - .S - Assembly language files. diff --git a/test/test_compile.py b/test/test_compile.py index 8b5337e02cf..aee5b0c8a1d 100644 --- a/test/test_compile.py +++ b/test/test_compile.py @@ -16,6 +16,7 @@ import platform import tempfile import hashlib +import shutil from pathlib import Path import simplejson as json @@ -629,3 +630,68 @@ def test_compile_with_fully_precompiled_library(run_command, data_dir): result = run_command(f"compile -b {fqbn} {sketch_folder} -v") assert result.ok assert "Skipping dependencies detection for precompiled library Arduino_TensorFlowLite" in result.stdout + + +def test_compile_sketch_with_pde_extension(run_command, data_dir): + # Init the environment explicitly + assert run_command("update") + + # Install core to compile + assert run_command("core install arduino:avr@1.8.3") + + sketch_name = "CompilePdeSketch" + sketch_path = Path(data_dir, sketch_name) + fqbn = "arduino:avr:uno" + + # Create a test sketch + assert run_command(f"sketch new {sketch_path}") + + # Renames sketch file to pde + sketch_file = Path(sketch_path, f"{sketch_name}.ino").rename(sketch_path / f"{sketch_name}.pde") + + # Build sketch from folder + res = run_command(f"compile --clean -b {fqbn} {sketch_path}") + assert res.ok + assert "Sketches with .pde extension are deprecated, please rename the following files to .ino:" in res.stderr + assert str(sketch_file) in res.stderr + + # Build sketch from file + res = run_command(f"compile --clean -b {fqbn} {sketch_file}") + assert res.ok + assert "Sketches with .pde extension are deprecated, please rename the following files to .ino" in res.stderr + assert str(sketch_file) in res.stderr + + +def test_compile_sketch_with_multiple_main_files(run_command, data_dir): + # Init the environment explicitly + assert run_command("update") + + # Install core to compile + assert run_command("core install arduino:avr@1.8.3") + + sketch_name = "CompileSketchMultipleMainFiles" + sketch_path = Path(data_dir, sketch_name) + fqbn = "arduino:avr:uno" + + # Create a test sketch + assert run_command(f"sketch new {sketch_path}") + + # Copy .ino sketch file to .pde + sketch_ino_file = Path(sketch_path, f"{sketch_name}.ino") + sketch_pde_file = Path(sketch_path / f"{sketch_name}.pde") + shutil.copyfile(sketch_ino_file, sketch_pde_file) + + # Build sketch from folder + res = run_command(f"compile --clean -b {fqbn} {sketch_path}") + assert res.failed + assert "Error during build: opening sketch: multiple main sketch files found" in res.stderr + + # Build sketch from .ino file + res = run_command(f"compile --clean -b {fqbn} {sketch_ino_file}") + assert res.failed + assert "Error during build: opening sketch: multiple main sketch files found" in res.stderr + + # Build sketch from .pde file + res = run_command(f"compile --clean -b {fqbn} {sketch_pde_file}") + assert res.failed + assert "Error during build: opening sketch: multiple main sketch files found" in res.stderr From ab2fd241ebc435c98b68e39c2097f155d26ad446 Mon Sep 17 00:00:00 2001 From: Silvano Cerza Date: Wed, 27 Jan 2021 18:19:04 +0100 Subject: [PATCH 2/6] Upload command now works with sketches and builds from .pde files --- cli/upload/upload.go | 9 +++ commands/upload/upload.go | 17 +++-- test/test_upload.py | 143 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 7 deletions(-) diff --git a/cli/upload/upload.go b/cli/upload/upload.go index fc75999ca3b..6476f8a8a9f 100644 --- a/cli/upload/upload.go +++ b/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, diff --git a/commands/upload/upload.go b/commands/upload/upload.go index bd55c7ecd5d..8da6876c287 100644 --- a/commands/upload/upload.go +++ b/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 } diff --git a/test/test_upload.py b/test/test_upload.py index 60d0149dde4..5d6b835984c 100644 --- a/test/test_upload.py +++ b/test/test_upload.py @@ -13,6 +13,9 @@ # software without disclosing the source code of your own applications. To purchase # a commercial license, send an email to license@arduino.cc. import os +import hashlib +import tempfile +import shutil from pathlib import Path import pytest @@ -195,3 +198,143 @@ def test_compile_and_upload_combo_with_custom_build_path(run_command, data_dir, assert f"Compile {sketch_name} for {board.fqbn} successful" in traces assert f"Upload {sketch_path} on {board.fqbn} started" in traces assert "Upload successful" in traces + + +def test_compile_and_upload_combo_sketch_with_pde_extension(run_command, data_dir, detected_boards, wait_for_board): + assert run_command("update") + + sketch_name = "CompileAndUploadPdeSketch" + sketch_path = Path(data_dir, sketch_name) + + # Create a test sketch + assert run_command(f"sketch new {sketch_path}") + + # Renames sketch file to pde + sketch_file = Path(sketch_path, f"{sketch_name}.ino").rename(sketch_path / f"{sketch_name}.pde") + + for board in detected_boards: + # Install core + core = ":".join(board.fqbn.split(":")[:2]) + assert run_command(f"core install {core}") + + # Build sketch and upload from folder + wait_for_board() + res = run_command(f"compile --clean -b {board.fqbn} -u -p {board.address} {sketch_path}") + assert res.ok + assert "Sketches with .pde extension are deprecated, please rename the following files to .ino" in res.stderr + assert str(sketch_file) in res.stderr + + # Build sketch and upload from file + wait_for_board() + res = run_command(f"compile --clean -b {board.fqbn} -u -p {board.address} {sketch_file}") + assert res.ok + assert "Sketches with .pde extension are deprecated, please rename the following files to .ino" in res.stderr + assert str(sketch_file) in res.stderr + + +def test_upload_sketch_with_pde_extension(run_command, data_dir, detected_boards, wait_for_board): + assert run_command("update") + + sketch_name = "UploadPdeSketch" + sketch_path = Path(data_dir, sketch_name) + + # Create a test sketch + assert run_command(f"sketch new {sketch_path}") + + # Renames sketch file to pde + sketch_file = Path(sketch_path, f"{sketch_name}.ino").rename(sketch_path / f"{sketch_name}.pde") + + for board in detected_boards: + # Install core + core = ":".join(board.fqbn.split(":")[:2]) + assert run_command(f"core install {core}") + + # Compile sketch first + assert run_command(f"compile --clean -b {board.fqbn} {sketch_path}") + + # Upload from sketch folder + wait_for_board() + assert run_command(f"upload -b {board.fqbn} -p {board.address} {sketch_path}") + + # Upload from sketch file + wait_for_board() + assert run_command(f"upload -b {board.fqbn} -p {board.address} {sketch_file}") + + # Upload from build folder + sketch_path_md5 = hashlib.md5(bytes(sketch_path)).hexdigest().upper() + build_dir = Path(tempfile.gettempdir(), f"arduino-sketch-{sketch_path_md5}") + wait_for_board() + res = run_command(f"upload -b {board.fqbn} -p {board.address} --input-dir {build_dir}") + assert ( + "Sketches with .pde extension are deprecated, please rename the following files to .ino:" not in res.stderr + ) + + # Upload from binary file + wait_for_board() + # We don't need a specific file when using the --input-file flag to upload since + # it's just used to calculate the directory, so it's enough to get a random file + # that's inside that directory + binary_file = next(build_dir.glob(f"{sketch_name}.pde.*")) + res = run_command(f"upload -b {board.fqbn} -p {board.address} --input-file {binary_file}") + assert ( + "Sketches with .pde extension are deprecated, please rename the following files to .ino:" not in res.stderr + ) + + +def test_upload_with_input_dir_containing_multiple_binaries(run_command, data_dir, detected_boards, wait_for_board): + # This tests verifies the behaviour outlined in this issue: + # https://github.com/arduino/arduino-cli/issues/765#issuecomment-699678646 + assert run_command("update") + + # Create a two different sketches + sketch_one_name = "UploadMultipleBinariesSketchOne" + sketch_one_path = Path(data_dir, sketch_one_name) + assert run_command(f"sketch new {sketch_one_path}") + + sketch_two_name = "UploadMultipleBinariesSketchTwo" + sketch_two_path = Path(data_dir, sketch_two_name) + assert run_command(f"sketch new {sketch_two_path}") + + for board in detected_boards: + # Install core + core = ":".join(board.fqbn.split(":")[:2]) + assert run_command(f"core install {core}") + + # Compile both sketches and copy binaries in the same directory same build directory + assert run_command(f"compile --clean -b {board.fqbn} {sketch_one_path}") + assert run_command(f"compile --clean -b {board.fqbn} {sketch_two_path}") + + sketch_path_md5 = hashlib.md5(bytes(sketch_one_path)).hexdigest().upper() + build_dir_one = Path(tempfile.gettempdir(), f"arduino-sketch-{sketch_path_md5}") + + sketch_path_md5 = hashlib.md5(bytes(sketch_two_path)).hexdigest().upper() + build_dir_two = Path(tempfile.gettempdir(), f"arduino-sketch-{sketch_path_md5}") + + # Copy binaries to same folder + binaries_dir = Path(data_dir, "build", "BuiltBinaries") + shutil.copytree(build_dir_one, binaries_dir, dirs_exist_ok=True) + shutil.copytree(build_dir_two, binaries_dir, dirs_exist_ok=True) + + wait_for_board() + # Verifies upload fails because multiple binaries are found + res = run_command(f"upload -b {board.fqbn} -p {board.address} --input-dir {binaries_dir}") + assert res.failed + assert ( + "Error during Upload: " + + "retrieving build artifacts: " + + "autodetect build artifact: " + + "multiple build artifacts found:" + in res.stderr + ) + + # Copy binaries to folder with same name of a sketch + binaries_dir = Path(data_dir, "build", "UploadMultipleBinariesSketchOne") + shutil.copytree(build_dir_one, binaries_dir, dirs_exist_ok=True) + shutil.copytree(build_dir_two, binaries_dir, dirs_exist_ok=True) + + wait_for_board() + # Verifies upload is successful using the binaries with the same name of the containing folder + res = run_command(f"upload -b {board.fqbn} -p {board.address} --input-dir {binaries_dir}") + assert ( + "Sketches with .pde extension are deprecated, please rename the following files to .ino:" not in res.stderr + ) From bbdf5dc03bb986b12b349041a4df8681c00e7aeb Mon Sep 17 00:00:00 2001 From: Silvano Cerza Date: Thu, 28 Jan 2021 12:22:33 +0100 Subject: [PATCH 3/6] Archive command now works with sketches containing .pde files --- cli/sketch/archive.go | 12 +++++- commands/sketch/archive.go | 40 ++++++++++++++----- test/test_sketch.py | 28 +++++++++++++ .../sketch_multiple_main_files.ino | 3 ++ .../sketch_multiple_main_files.pde | 3 ++ .../sketch_pde_main_file.pde | 3 ++ 6 files changed, 77 insertions(+), 12 deletions(-) create mode 100644 test/testdata/sketch_multiple_main_files/sketch_multiple_main_files.ino create mode 100644 test/testdata/sketch_multiple_main_files/sketch_multiple_main_files.pde create mode 100644 test/testdata/sketch_pde_main_file/sketch_pde_main_file.pde diff --git a/cli/sketch/archive.go b/cli/sketch/archive.go index 65eff2d8a98..b44f9990ab8 100644 --- a/cli/sketch/archive.go +++ b/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] diff --git a/commands/sketch/archive.go b/commands/sketch/archive.go index d5b77fd792b..f8fd75f57da 100644 --- a/commands/sketch/archive.go +++ b/commands/sketch/archive.go @@ -23,8 +23,10 @@ import ( "path/filepath" "strings" + "github.com/arduino/arduino-cli/arduino/globals" rpc "github.com/arduino/arduino-cli/rpc/commands" paths "github.com/arduino/go-paths-helper" + "github.com/pkg/errors" ) // ArchiveSketch FIXMEDOC @@ -42,22 +44,38 @@ func ArchiveSketch(ctx context.Context, req *rpc.ArchiveSketchReq) (*rpc.Archive return nil, fmt.Errorf("Error getting absolute sketch path %v", 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") + // Make sketchPath point to the sketch folder + if sketchPath.IsNotDir() { + sketchPath = sketchPath.Parent() + } + + sketchName = sketchPath.Base() + + // sketchMainFile is the path to the sketch main file + var sketchMainFile *paths.Path + + for ext := range globals.MainFileValidExtensions { + candidateSketchMainFile := sketchPath.Join(sketchPath.Base() + ext) + if candidateSketchMainFile.Exist() { + if sketchMainFile == nil { + sketchMainFile = candidateSketchMainFile + } else { + return nil, errors.Errorf("multiple main sketch files found (%v, %v)", + sketchMainFile, + candidateSketchMainFile, + ) + } + } } // Checks if it's really a sketch - if sketchPath.NotExist() { + if sketchMainFile == nil { return nil, fmt.Errorf("specified path is not a sketch: %v", sketchPath.String()) } archivePath := paths.New(req.ArchivePath) if archivePath == nil { - archivePath = sketchPath.Parent().Parent() + archivePath = sketchPath.Parent() } archivePath, err = archivePath.Clean().Abs() @@ -76,7 +94,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 +112,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 +125,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) } diff --git a/test/test_sketch.py b/test/test_sketch.py index 34ac263a71c..d3fb4126faa 100644 --- a/test/test_sketch.py +++ b/test/test_sketch.py @@ -818,3 +818,31 @@ def test_sketch_archive_absolute_sketch_path_with_absolute_zip_path_and_name_wit verify_zip_contains_sketch_including_build_dir(archive_files) archive.close() + + +def test_sketch_archive_with_pde_main_file(run_command, copy_sketch, working_dir): + sketch_name = "sketch_pde_main_file" + sketch_dir = copy_sketch(sketch_name) + sketch_file = Path(sketch_dir, f"{sketch_name}.pde") + res = run_command("sketch archive", sketch_dir) + assert res.ok + assert "Sketches with .pde extension are deprecated, please rename the following files to .ino" in res.stderr + assert str(sketch_file.relative_to(sketch_dir)) in res.stderr + + archive = zipfile.ZipFile(f"{working_dir}/{sketch_name}.zip") + archive_files = archive.namelist() + + assert f"{sketch_name}/{sketch_name}.pde" in archive_files + + archive.close() + + +def test_sketch_archive_with_multiple_main_files(run_command, copy_sketch, working_dir): + sketch_name = "sketch_multiple_main_files" + sketch_dir = copy_sketch(sketch_name) + sketch_file = Path(sketch_dir, f"{sketch_name}.pde") + res = run_command("sketch archive", sketch_dir) + assert res.failed + assert "Sketches with .pde extension are deprecated, please rename the following files to .ino" in res.stderr + assert str(sketch_file.relative_to(sketch_dir)) in res.stderr + assert "Error archiving: multiple main sketch files found" in res.stderr diff --git a/test/testdata/sketch_multiple_main_files/sketch_multiple_main_files.ino b/test/testdata/sketch_multiple_main_files/sketch_multiple_main_files.ino new file mode 100644 index 00000000000..2263c4bcd9e --- /dev/null +++ b/test/testdata/sketch_multiple_main_files/sketch_multiple_main_files.ino @@ -0,0 +1,3 @@ +void setup() { } + +void loop() { } \ No newline at end of file diff --git a/test/testdata/sketch_multiple_main_files/sketch_multiple_main_files.pde b/test/testdata/sketch_multiple_main_files/sketch_multiple_main_files.pde new file mode 100644 index 00000000000..2263c4bcd9e --- /dev/null +++ b/test/testdata/sketch_multiple_main_files/sketch_multiple_main_files.pde @@ -0,0 +1,3 @@ +void setup() { } + +void loop() { } \ No newline at end of file diff --git a/test/testdata/sketch_pde_main_file/sketch_pde_main_file.pde b/test/testdata/sketch_pde_main_file/sketch_pde_main_file.pde new file mode 100644 index 00000000000..2263c4bcd9e --- /dev/null +++ b/test/testdata/sketch_pde_main_file/sketch_pde_main_file.pde @@ -0,0 +1,3 @@ +void setup() { } + +void loop() { } \ No newline at end of file From 5175343c9ee31c00dba6bcd343b8b62787271189 Mon Sep 17 00:00:00 2001 From: Silvano Cerza Date: Thu, 28 Jan 2021 12:55:38 +0100 Subject: [PATCH 4/6] [skip changelog] Add test to verify debug command works with pde sketches --- test/test_debug.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/test_debug.py b/test/test_debug.py index 9c3d4079396..30fcfd09777 100644 --- a/test/test_debug.py +++ b/test/test_debug.py @@ -37,3 +37,27 @@ def test_debugger_starts(run_command, data_dir): programmer = "atmel_ice" # Starts debugger assert run_command(f"debug -b {fqbn} -P {programmer} {sketch_path} --info") + + +def test_debugger_with_pde_sketch_starts(run_command, data_dir): + assert run_command("update") + + # Install core + assert run_command("core install arduino:samd") + + # Create sketch for testing + sketch_name = "DebuggerPdeSketchStartTest" + sketch_path = Path(data_dir, sketch_name) + fqbn = "arduino:samd:mkr1000" + + assert run_command(f"sketch new {sketch_path}") + + # Renames sketch file to pde + Path(sketch_path, f"{sketch_name}.ino").rename(sketch_path / f"{sketch_name}.pde") + + # Build sketch + assert run_command(f"compile -b {fqbn} {sketch_path}") + + programmer = "atmel_ice" + # Starts debugger + assert run_command(f"debug -b {fqbn} -P {programmer} {sketch_path} --info") From e82f252dfdf512e872c5d12e72970a23bca300d7 Mon Sep 17 00:00:00 2001 From: Silvano Cerza Date: Thu, 28 Jan 2021 13:24:52 +0100 Subject: [PATCH 5/6] Fix lib examples not showing sketches with .pde files --- arduino/libraries/loader.go | 10 +++------- test/test_lib.py | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/arduino/libraries/loader.go b/arduino/libraries/loader.go index 8e8462536e4..70e738d3fdd 100644 --- a/arduino/libraries/loader.go +++ b/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() -} diff --git a/test/test_lib.py b/test/test_lib.py index 77e03cd13c5..5a164b95f05 100644 --- a/test/test_lib.py +++ b/test/test_lib.py @@ -621,3 +621,36 @@ def test_install_with_zip_path_multiple_libraries(run_command, downloads_dir, da # Verifies library are installed assert wifi_install_dir.exists() assert ble_install_dir.exists() + + +def test_lib_examples(run_command, data_dir): + assert run_command("update") + + assert run_command("lib install Arduino_JSON@0.1.0") + + res = run_command("lib examples Arduino_JSON --format json") + assert res.ok + data = json.loads(res.stdout) + assert len(data) == 1 + examples = data[0]["examples"] + + assert str(Path(data_dir, "libraries", "Arduino_JSON", "examples", "JSONArray")) in examples + assert str(Path(data_dir, "libraries", "Arduino_JSON", "examples", "JSONKitchenSink")) in examples + assert str(Path(data_dir, "libraries", "Arduino_JSON", "examples", "JSONObject")) in examples + + +def test_lib_examples_with_pde_file(run_command, data_dir): + assert run_command("update") + + assert run_command("lib install Encoder@1.4.1") + + res = run_command("lib examples Encoder --format json") + assert res.ok + data = json.loads(res.stdout) + assert len(data) == 1 + examples = data[0]["examples"] + + assert str(Path(data_dir, "libraries", "Encoder", "examples", "Basic")) in examples + assert str(Path(data_dir, "libraries", "Encoder", "examples", "NoInterrupts")) in examples + assert str(Path(data_dir, "libraries", "Encoder", "examples", "SpeedTest")) in examples + assert str(Path(data_dir, "libraries", "Encoder", "examples", "TwoKnobs")) in examples From 686d3f87a310919c16fd19a7cb78ae3d4155e584 Mon Sep 17 00:00:00 2001 From: Silvano Cerza Date: Fri, 29 Jan 2021 11:09:01 +0100 Subject: [PATCH 6/6] [skip changelog] Remove duplicated code and enhance tests --- arduino/sketches/sketches.go | 14 +++++--------- commands/sketch/archive.go | 37 +++++------------------------------- test/test_upload.py | 27 +++++++++++++------------- 3 files changed, 23 insertions(+), 55 deletions(-) diff --git a/arduino/sketches/sketches.go b/arduino/sketches/sketches.go index 4aefd2c990a..d219e936f62 100644 --- a/arduino/sketches/sketches.go +++ b/arduino/sketches/sketches.go @@ -138,14 +138,10 @@ func CheckForPdeFiles(sketch *paths.Path) []*paths.Path { sketch = sketch.Parent() } - // It's not a problem if we can't read files right now - files, _ := sketch.ReadDirRecursive() - files.FilterOutDirs() - res := []*paths.Path{} - for _, f := range files { - if f.Ext() == ".pde" { - res = append(res, f) - } + files, err := sketch.ReadDirRecursive() + if err != nil { + return []*paths.Path{} } - return res + files.FilterSuffix(".pde") + return files } diff --git a/commands/sketch/archive.go b/commands/sketch/archive.go index f8fd75f57da..d702489a83b 100644 --- a/commands/sketch/archive.go +++ b/commands/sketch/archive.go @@ -23,10 +23,9 @@ import ( "path/filepath" "strings" - "github.com/arduino/arduino-cli/arduino/globals" + "github.com/arduino/arduino-cli/arduino/sketches" rpc "github.com/arduino/arduino-cli/rpc/commands" paths "github.com/arduino/go-paths-helper" - "github.com/pkg/errors" ) // ArchiveSketch FIXMEDOC @@ -39,39 +38,13 @@ 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 } - // Make sketchPath point to the sketch folder - if sketchPath.IsNotDir() { - sketchPath = sketchPath.Parent() - } - - sketchName = sketchPath.Base() - - // sketchMainFile is the path to the sketch main file - var sketchMainFile *paths.Path - - for ext := range globals.MainFileValidExtensions { - candidateSketchMainFile := sketchPath.Join(sketchPath.Base() + ext) - if candidateSketchMainFile.Exist() { - if sketchMainFile == nil { - sketchMainFile = candidateSketchMainFile - } else { - return nil, errors.Errorf("multiple main sketch files found (%v, %v)", - sketchMainFile, - candidateSketchMainFile, - ) - } - } - } - - // Checks if it's really a sketch - if sketchMainFile == nil { - 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 { diff --git a/test/test_upload.py b/test/test_upload.py index 5d6b835984c..7f8d272a822 100644 --- a/test/test_upload.py +++ b/test/test_upload.py @@ -13,9 +13,8 @@ # software without disclosing the source code of your own applications. To purchase # a commercial license, send an email to license@arduino.cc. import os -import hashlib -import tempfile import shutil +import json from pathlib import Path import pytest @@ -250,7 +249,10 @@ def test_upload_sketch_with_pde_extension(run_command, data_dir, detected_boards assert run_command(f"core install {core}") # Compile sketch first - assert run_command(f"compile --clean -b {board.fqbn} {sketch_path}") + res = run_command(f"compile --clean -b {board.fqbn} {sketch_path} --format json") + assert res.ok + data = json.loads(res.stdout) + build_dir = Path(data["builder_result"]["build_path"]) # Upload from sketch folder wait_for_board() @@ -260,9 +262,6 @@ def test_upload_sketch_with_pde_extension(run_command, data_dir, detected_boards wait_for_board() assert run_command(f"upload -b {board.fqbn} -p {board.address} {sketch_file}") - # Upload from build folder - sketch_path_md5 = hashlib.md5(bytes(sketch_path)).hexdigest().upper() - build_dir = Path(tempfile.gettempdir(), f"arduino-sketch-{sketch_path_md5}") wait_for_board() res = run_command(f"upload -b {board.fqbn} -p {board.address} --input-dir {build_dir}") assert ( @@ -301,14 +300,14 @@ def test_upload_with_input_dir_containing_multiple_binaries(run_command, data_di assert run_command(f"core install {core}") # Compile both sketches and copy binaries in the same directory same build directory - assert run_command(f"compile --clean -b {board.fqbn} {sketch_one_path}") - assert run_command(f"compile --clean -b {board.fqbn} {sketch_two_path}") - - sketch_path_md5 = hashlib.md5(bytes(sketch_one_path)).hexdigest().upper() - build_dir_one = Path(tempfile.gettempdir(), f"arduino-sketch-{sketch_path_md5}") - - sketch_path_md5 = hashlib.md5(bytes(sketch_two_path)).hexdigest().upper() - build_dir_two = Path(tempfile.gettempdir(), f"arduino-sketch-{sketch_path_md5}") + res = run_command(f"compile --clean -b {board.fqbn} {sketch_one_path} --format json") + assert res.ok + data = json.loads(res.stdout) + build_dir_one = Path(data["builder_result"]["build_path"]) + res = run_command(f"compile --clean -b {board.fqbn} {sketch_two_path} --format json") + assert res.ok + data = json.loads(res.stdout) + build_dir_two = Path(data["builder_result"]["build_path"]) # Copy binaries to same folder binaries_dir = Path(data_dir, "build", "BuiltBinaries")