Fix loading of sketches with folder and main file mismatched casing

silvanocerza · silvanocerza · commit 5b3b4f12923d · 2021-02-09T09:59:03.000+01:00
diff --git a/arduino/builder/sketch_test.go b/arduino/builder/sketch_test.go
@@ -230,3 +230,17 @@ func TestCopyAdditionalFiles(t *testing.T) {
 	info2, err := os.Stat(s2.AdditionalFiles[0].Path)
 	require.Equal(t, info1.ModTime(), info2.ModTime())
 }
+
+func TestLoadSketchCaseMismatch(t *testing.T) {
+	// pass the path to the sketch folder
+	sketchPath := filepath.Join("testdata", t.Name())
+	mainFilePath := filepath.Join(sketchPath, t.Name()+".ino")
+	s, err := builder.SketchLoad(sketchPath, "")
+	require.Nil(t, s)
+	require.Error(t, err)
+
+	// pass the path to the main file
+	s, err = builder.SketchLoad(mainFilePath, "")
+	require.Nil(t, s)
+	require.Error(t, err)
+}
diff --git a/arduino/builder/testdata/TestLoadSketchCaseMismatch/testloadsketchcasemismatch.ino b/arduino/builder/testdata/TestLoadSketchCaseMismatch/testloadsketchcasemismatch.ino
@@ -0,0 +1,7 @@
+void setup() {
+
+}
+
+void loop() {
+
+}
diff --git a/arduino/sketch/sketch.go b/arduino/sketch/sketch.go
@@ -22,6 +22,7 @@ import (
 	"strings"
 
 	"github.com/arduino/arduino-cli/arduino/globals"
+	"github.com/arduino/go-paths-helper"
 	"github.com/pkg/errors"
 )
 
@@ -116,10 +117,43 @@ func New(sketchFolderPath, mainFilePath, buildPath string, allFilesPaths []strin
 	sort.Sort(ItemByPath(additionalFiles))
 	sort.Sort(ItemByPath(otherSketchFiles))
 
+	if err := CheckSketchCasing(sketchFolderPath); err != nil {
+		return nil, err
+	}
+
 	return &Sketch{
 		MainFile:         mainFile,
 		LocationPath:     sketchFolderPath,
 		OtherSketchFiles: otherSketchFiles,
 		AdditionalFiles:  additionalFiles,
 	}, nil
 }
+
+// CheckSketchCasing returns an error if the casing of the sketch folder and the main file are different.
+// Correct:
+//    MySketch/MySketch.ino
+// Wrong:
+//    MySketch/mysketch.ino
+//    mysketch/MySketch.ino
+//
+// This is mostly necessary to avoid errors on Mac OS X.
+// For more info see: https://github.com/arduino/arduino-cli/issues/1174
+func CheckSketchCasing(sketchFolder string) error {
+	sketchPath := paths.New(sketchFolder)
+	files, err := sketchPath.ReadDir()
+	if err != nil {
+		return errors.Errorf("reading files: %v", err)
+	}
+	files.FilterOutDirs()
+
+	sketchName := sketchPath.Base()
+	files.FilterPrefix(sketchName)
+
+	if files.Len() == 0 {
+		sketchFolderPath := paths.New(sketchFolder)
+		sketchFile := sketchFolderPath.Join(sketchFolderPath.Base() + globals.MainFileValidExtension)
+		return errors.Errorf("no valid sketch found in %s: missing %s", sketchFolderPath, sketchFile)
+	}
+
+	return nil
+}
diff --git a/arduino/sketch/sketch_test.go b/arduino/sketch/sketch_test.go
@@ -16,12 +16,15 @@
 package sketch_test
 
 import (
+	"fmt"
 	"path/filepath"
 	"sort"
 	"testing"
 
 	"github.com/arduino/arduino-cli/arduino/sketch"
+	"github.com/arduino/go-paths-helper"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func TestNewItem(t *testing.T) {
@@ -43,9 +46,9 @@ func TestNewItem(t *testing.T) {
 
 func TestSort(t *testing.T) {
 	items := []*sketch.Item{
-		&sketch.Item{"foo"},
-		&sketch.Item{"baz"},
-		&sketch.Item{"bar"},
+		{"foo"},
+		{"baz"},
+		{"bar"},
 	}
 
 	sort.Sort(sketch.ItemByPath(items))
@@ -71,3 +74,37 @@ func TestNew(t *testing.T) {
 	assert.Len(t, sketch.OtherSketchFiles, 0)
 	assert.Len(t, sketch.AdditionalFiles, 1)
 }
+
+func TestNewSketchCasingWrong(t *testing.T) {
+	sketchPath := paths.New("testdata", "SketchCasingWrong")
+	mainFilePath := paths.New("testadata", "sketchcasingwrong.ino").String()
+	sketch, err := sketch.New(sketchPath.String(), mainFilePath, "", []string{mainFilePath})
+	assert.Nil(t, sketch)
+	expectedError := fmt.Sprintf("no valid sketch found in %s: missing %s", sketchPath.String(), sketchPath.Join(sketchPath.Base()+".ino"))
+	assert.EqualError(t, err, expectedError)
+}
+
+func TestNewSketchCasingCorrect(t *testing.T) {
+	sketchPath := paths.New("testdata", "SketchCasingCorrect").String()
+	mainFilePath := paths.New("testadata", "SketchCasingCorrect.ino").String()
+	sketch, err := sketch.New(sketchPath, mainFilePath, "", []string{mainFilePath})
+	assert.NotNil(t, sketch)
+	assert.NoError(t, err)
+	assert.Equal(t, sketchPath, sketch.LocationPath)
+	assert.Equal(t, mainFilePath, sketch.MainFile.Path)
+	assert.Len(t, sketch.OtherSketchFiles, 0)
+	assert.Len(t, sketch.AdditionalFiles, 0)
+}
+
+func TestCheckSketchCasingWrong(t *testing.T) {
+	sketchFolder := paths.New("testdata", "SketchCasingWrong")
+	err := sketch.CheckSketchCasing(sketchFolder.String())
+	expectedError := fmt.Sprintf("no valid sketch found in %s: missing %s", sketchFolder, sketchFolder.Join(sketchFolder.Base()+".ino"))
+	assert.EqualError(t, err, expectedError)
+}
+
+func TestCheckSketchCasingCorrect(t *testing.T) {
+	sketchFolder := paths.New("testdata", "SketchCasingCorrect").String()
+	err := sketch.CheckSketchCasing(sketchFolder)
+	require.NoError(t, err)
+}
diff --git a/arduino/sketch/testdata/SketchCasingCorrect/SketchCasingCorrect.ino b/arduino/sketch/testdata/SketchCasingCorrect/SketchCasingCorrect.ino
@@ -0,0 +1,7 @@
+void setup() {
+
+}
+
+void loop() {
+
+}
diff --git a/arduino/sketch/testdata/SketchCasingWrong/sketchcasingwrong.ino b/arduino/sketch/testdata/SketchCasingWrong/sketchcasingwrong.ino
@@ -0,0 +1,7 @@
+void setup() {
+
+}
+
+void loop() {
+
+}
diff --git a/arduino/sketches/sketches.go b/arduino/sketches/sketches.go
@@ -21,6 +21,7 @@ import (
 
 	"github.com/arduino/arduino-cli/arduino/builder"
 	"github.com/arduino/arduino-cli/arduino/globals"
+	"github.com/arduino/arduino-cli/arduino/sketch"
 	"github.com/arduino/go-paths-helper"
 	"github.com/pkg/errors"
 )
@@ -70,19 +71,19 @@ func NewSketchFromPath(path *paths.Path) (*Sketch, error) {
 		}
 	}
 
-	if mainSketchFile == nil {
+	if mainSketchFile == nil || sketch.CheckSketchCasing(path.String()) != nil {
 		sketchFile := path.Join(path.Base() + globals.MainFileValidExtension)
 		return nil, errors.Errorf("no valid sketch found in %s: missing %s", path, sketchFile)
 	}
 
-	sketch := &Sketch{
+	s := &Sketch{
 		FullPath:          path,
 		MainFileExtension: mainSketchFile.Ext(),
 		Name:              path.Base(),
 		Metadata:          &Metadata{},
 	}
-	sketch.ImportMetadata()
-	return sketch, nil
+	s.ImportMetadata()
+	return s, nil
 }
 
 // ImportMetadata imports metadata into the sketch from a sketch.json file in the root
diff --git a/arduino/sketches/sketches_test.go b/arduino/sketches/sketches_test.go
@@ -109,3 +109,24 @@ func TestCheckForPdeFiles(t *testing.T) {
 	require.Len(t, files, 1)
 	require.Equal(t, sketchPath.Parent().Join("SketchMultipleMainFiles.pde"), files[0])
 }
+
+func TestSketchLoadWithCasing(t *testing.T) {
+	sketchFolder := paths.New("testdata", "SketchCasingWrong")
+
+	sketch, err := NewSketchFromPath(sketchFolder)
+	require.Nil(t, sketch)
+
+	sketchFolderAbs, _ := sketchFolder.Abs()
+	sketchMainFileAbs := sketchFolderAbs.Join("SketchCasingWrong.ino")
+	expectedError := fmt.Sprintf("no valid sketch found in %s: missing %s", sketchFolderAbs, sketchMainFileAbs)
+	require.EqualError(t, err, expectedError)
+}
+
+func TestSketchLoadingCorrectCasing(t *testing.T) {
+	sketchFolder := paths.New("testdata", "SketchCasingCorrect")
+	sketch, err := NewSketchFromPath(sketchFolder)
+	require.NotNil(t, sketch)
+	require.NoError(t, err)
+	require.Equal(t, sketch.Name, "SketchCasingCorrect")
+	require.True(t, sketch.FullPath.EquivalentTo(sketchFolder))
+}
diff --git a/arduino/sketches/testdata/SketchCasingCorrect/SketchCasingCorrect.ino b/arduino/sketches/testdata/SketchCasingCorrect/SketchCasingCorrect.ino
@@ -0,0 +1,7 @@
+void setup() {
+
+}
+
+void loop() {
+
+}
diff --git a/arduino/sketches/testdata/SketchCasingWrong/sketchcasingwrong.ino b/arduino/sketches/testdata/SketchCasingWrong/sketchcasingwrong.ino
@@ -0,0 +1,7 @@
+void setup() {
+
+}
+
+void loop() {
+
+}
diff --git a/test/test_compile.py b/test/test_compile.py
@@ -695,3 +695,34 @@ def test_compile_sketch_with_multiple_main_files(run_command, data_dir):
     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
+
+
+def test_compile_sketch_case_mismatch_fails(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 = "CompileSketchCaseMismatch"
+    sketch_path = Path(data_dir, sketch_name)
+    fqbn = "arduino:avr:uno"
+
+    assert run_command(f"sketch new {sketch_path}")
+
+    # Rename main .ino file so casing is different from sketch name
+    sketch_main_file = Path(sketch_path, f"{sketch_name}.ino").rename(sketch_path / f"{sketch_name.lower()}.ino")
+
+    # Verifies compilation fails when:
+    # * Compiling with sketch path
+    res = run_command(f"compile --clean -b {fqbn} {sketch_path}")
+    assert res.failed
+    assert "Error during build: opening sketch: no valid sketch found" in res.stderr
+    # * Compiling with sketch main file
+    res = run_command(f"compile --clean -b {fqbn} {sketch_main_file}")
+    assert res.failed
+    assert "Error during build: opening sketch: no valid sketch found" in res.stderr
+    # * Compiling in sketch path
+    res = run_command(f"compile --clean -b {fqbn}", custom_working_dir=sketch_path)
+    assert res.failed
+    assert "Error during build: opening sketch: no valid sketch found" in res.stderr
diff --git a/test/test_lib.py b/test/test_lib.py
@@ -668,3 +668,38 @@ def test_lib_examples_with_pde_file(run_command, data_dir):
     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
+
+
+def test_lib_examples_with_case_mismatch(run_command, data_dir):
+    assert run_command("update")
+
+    assert run_command("lib install WiFiManager@2.0.3-alpha")
+
+    res = run_command("lib examples WiFiManager --format json")
+    assert res.ok
+    data = json.loads(res.stdout)
+    assert len(data) == 1
+    examples = data[0]["examples"]
+
+    assert len(examples) == 14
+
+    examples_path = Path(data_dir, "libraries", "WiFiManager", "examples")
+    # Verifies sketches with correct casing are listed
+    assert str(examples_path / "Advanced") in examples
+    assert str(examples_path / "AutoConnect" / "AutoConnectWithFeedbackLED") in examples
+    assert str(examples_path / "AutoConnect" / "AutoConnectWithFSParameters") in examples
+    assert str(examples_path / "AutoConnect" / "AutoConnectWithFSParametersAndCustomIP") in examples
+    assert str(examples_path / "Basic") in examples
+    assert str(examples_path / "DEV" / "OnDemandConfigPortal") in examples
+    assert str(examples_path / "NonBlocking" / "AutoConnectNonBlocking") in examples
+    assert str(examples_path / "NonBlocking" / "AutoConnectNonBlockingwParams") in examples
+    assert str(examples_path / "Old_examples" / "AutoConnectWithFeedback") in examples
+    assert str(examples_path / "Old_examples" / "AutoConnectWithReset") in examples
+    assert str(examples_path / "Old_examples" / "AutoConnectWithStaticIP") in examples
+    assert str(examples_path / "Old_examples" / "AutoConnectWithTimeout") in examples
+    assert str(examples_path / "OnDemand" / "OnDemandConfigPortal") in examples
+    assert str(examples_path / "ParamsChildClass") in examples
+
+    # Verifies sketches with wrong casing are not returned
+    assert str(examples_path / "NonBlocking" / "OnDemandNonBlocking") not in examples
+    assert str(examples_path / "OnDemand" / "OnDemandWebPortal") not in examples
diff --git a/test/test_sketch.py b/test/test_sketch.py
@@ -846,3 +846,17 @@ def test_sketch_archive_with_multiple_main_files(run_command, copy_sketch, worki
     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
+
+
+def test_sketch_archive_case_mismatch_fails(run_command, data_dir):
+    sketch_name = "ArchiveSketchCaseMismatch"
+    sketch_path = Path(data_dir, sketch_name)
+
+    assert run_command(f"sketch new {sketch_path}")
+
+    # Rename main .ino file so casing is different from sketch name
+    Path(sketch_path, f"{sketch_name}.ino").rename(sketch_path / f"{sketch_name.lower()}.ino")
+
+    res = run_command(f'sketch archive "{sketch_path}"')
+    assert res.failed
+    assert "Error archiving: no valid sketch found" in res.stderr
diff --git a/test/test_upload.py b/test/test_upload.py
@@ -337,3 +337,48 @@ def test_upload_with_input_dir_containing_multiple_binaries(run_command, data_di
         assert (
             "Sketches with .pde extension are deprecated, please rename the following files to .ino:" not in res.stderr
         )
+
+
+def test_compile_and_upload_combo_sketch_with_mismatched_casing(run_command, data_dir, detected_boards, wait_for_board):
+    assert run_command("update")
+
+    # Create a sketch
+    sketch_name = "CompileUploadComboMismatchCasing"
+    sketch_path = Path(data_dir, sketch_name)
+    assert run_command(f"sketch new {sketch_path}")
+
+    # Rename main .ino file so casing is different from sketch name
+    Path(sketch_path, f"{sketch_name}.ino").rename(sketch_path / f"{sketch_name.lower()}.ino")
+
+    for board in detected_boards:
+        # Install core
+        core = ":".join(board.fqbn.split(":")[:2])
+        assert run_command(f"core install {core}")
+
+        # Try to compile
+        res = run_command(f"compile --clean -b {board.fqbn} -u -p {board.address} {sketch_path}")
+        assert res.failed
+        assert "Error during build: opening sketch: no valid sketch found" in res.stderr
+
+
+def test_upload_sketch_with_mismatched_casing(run_command, data_dir, detected_boards, wait_for_board):
+    assert run_command("update")
+
+    # Create a sketch
+    sketch_name = "UploadMismatchCasing"
+    sketch_path = Path(data_dir, sketch_name)
+    assert run_command(f"sketch new {sketch_path}")
+
+    # Rename main .ino file so casing is different from sketch name
+    Path(sketch_path, f"{sketch_name}.ino").rename(sketch_path / f"{sketch_name.lower()}.ino")
+
+    for board in detected_boards:
+        # Install core
+        core = ":".join(board.fqbn.split(":")[:2])
+        assert run_command(f"core install {core}")
+
+        # Tries to upload given sketch, it has not been compiled but it fails even before
+        # searching for binaries since the sketch is not valid
+        res = run_command(f"upload -b {board.fqbn} -p {board.address} {sketch_path}")
+        assert res.failed
+        assert "Error during Upload: opening sketch: no valid sketch found" in res.stderr

-Original file line number
+Diff line change
@@ @@ -0,0 +1,7 @@ @@
 +void setup() {
++
 +}
++
 +void loop() {
++
 +}
Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,7 @@ import (`
`21`	`21`
`22`	`22`	`"github.com/arduino/arduino-cli/arduino/builder"`
`23`	`23`	`"github.com/arduino/arduino-cli/arduino/globals"`
	`24`	`+ "github.com/arduino/arduino-cli/arduino/sketch"`
`24`	`25`	`"github.com/arduino/go-paths-helper"`
`25`	`26`	`"github.com/pkg/errors"`
`26`	`27`	`)`
`@@ -70,19 +71,19 @@ func NewSketchFromPath(path paths.Path) (Sketch, error) {`
`70`	`71`	`}`
`71`	`72`	`}`
`72`	`73`
`73`		`- if mainSketchFile == nil {`
	`74`	`+ if mainSketchFile == nil \|\| sketch.CheckSketchCasing(path.String()) != nil {`
`74`	`75`	`sketchFile := path.Join(path.Base() + globals.MainFileValidExtension)`
`75`	`76`	`return nil, errors.Errorf("no valid sketch found in %s: missing %s", path, sketchFile)`
`76`	`77`	`}`
`77`	`78`
`78`		`- sketch := &Sketch{`
	`79`	`+ s := &Sketch{`
`79`	`80`	`FullPath: path,`
`80`	`81`	`MainFileExtension: mainSketchFile.Ext(),`
`81`	`82`	`Name: path.Base(),`
`82`	`83`	`Metadata: &Metadata{},`
`83`	`84`	`}`
`84`		`- sketch.ImportMetadata()`
`85`		`- return sketch, nil`
	`85`	`+ s.ImportMetadata()`
	`86`	`+ return s, nil`
`86`	`87`	`}`
`87`	`88`
`88`	`89`	`// ImportMetadata imports metadata into the sketch from a sketch.json file in the root`