Allow naming sketches like "RCS" and "CVS" (#537)

* avoid paniking if main sketch file is missing

* rewrite the sketch contents collection logic + tests

* fix tests

Author: Massimiliano Pippi

Diff for: arduino/builder/sketch.go

@@ -112,29 +112,31 @@ func SketchLoad(sketchPath, buildPath string) (*sketch.Sketch, error) {
 	if stat.IsDir() {
 		sketchFolder = sketchPath
 		// allowed extensions are .ino and .pde (but not both)
-		allowedSketchExtensions := [...]string{".ino", ".pde"}
-		for _, extension := range allowedSketchExtensions {
+		for extension := range globals.MainFileValidExtensions {
 			candidateSketchFile := filepath.Join(sketchPath, stat.Name()+extension)
 			if _, err := os.Stat(candidateSketchFile); !os.IsNotExist(err) {
 				if mainSketchFile == "" {
 					mainSketchFile = candidateSketchFile
 				} else {
-					return nil, errors.Errorf("more than one main sketch file found (%v,%v)",
+					return nil, errors.Errorf("multiple main sketch files found (%v,%v)",
 						filepath.Base(mainSketchFile),
 						filepath.Base(candidateSketchFile))
 				}
 			}
 		}
-		// check that .pde or .ino was found
+
+		// check main file was found
 		if mainSketchFile == "" {
-			return nil, errors.Errorf("unable to find an sketch file in directory %v", sketchFolder)
+			return nil, errors.Errorf("unable to find a sketch file in directory %v", sketchFolder)
 		}
-		// in the case a dir was passed, ensure the main file exists and is readable
+
+		// check main file is readable
 		f, err := os.Open(mainSketchFile)
 		if err != nil {
 			return nil, errors.Wrap(err, "unable to open the main sketch file")
 		}
 		f.Close()
+
 		// ensure it is not a directory
 		info, err := os.Stat(mainSketchFile)
 		if err != nil {
@@ -150,28 +152,37 @@ func SketchLoad(sketchPath, buildPath string) (*sketch.Sketch, error) {
 
 	// collect all the sketch files
 	var files []string
+	rootVisited := false
 	err = simpleLocalWalk(sketchFolder, maxFileSystemDepth, func(path string, info os.FileInfo, err error) error {
-
 		if err != nil {
 			feedback.Errorf("Error during sketch processing: %v", err)
 			os.Exit(errorcodes.ErrGeneric)
 		}
 
-		// ignore hidden files and skip hidden directories
-		if strings.HasPrefix(info.Name(), ".") {
-			if info.IsDir() {
-				return filepath.SkipDir
+		if info.IsDir() {
+			// Filters in this if-block are NOT applied to the sketch folder itself.
+			// Since the sketch folder is the first one processed by simpleLocalWalk,
+			// we can set the `rootVisited` guard to exclude it.
+			if rootVisited {
+				// skip hidden folders
+				if strings.HasPrefix(info.Name(), ".") {
+					return filepath.SkipDir
+				}
+
+				// skip legacy SCM directories
+				if info.Name() == "CVS" || info.Name() == "RCS" {
+					return filepath.SkipDir
+				}
+			} else {
+				rootVisited = true
 			}
-			return nil
-		}
 
-		// skip legacy SCM directories
-		if info.IsDir() && strings.HasPrefix(info.Name(), "CVS") || strings.HasPrefix(info.Name(), "RCS") {
-			return filepath.SkipDir
+			// ignore (don't skip) directory
+			return nil
 		}
 
-		// ignore directory entries
-		if info.IsDir() {
+		// ignore hidden files
+		if strings.HasPrefix(info.Name(), ".") {
 			return nil
 		}
 

Diff for: arduino/builder/sketch_test.go

@@ -105,7 +105,7 @@ func TestLoadSketchFolderBothInoAndPde(t *testing.T) {
 	sketchPath := filepath.Join("testdata", t.Name())
 	_, err := builder.SketchLoad(sketchPath, "")
 	require.Error(t, err)
-	require.Contains(t, err.Error(), "more than one main sketch file found")
+	require.Contains(t, err.Error(), "multiple main sketch files found")
 	require.Contains(t, err.Error(), t.Name()+".ino")
 	require.Contains(t, err.Error(), t.Name()+".pde")
 }
@@ -157,7 +157,7 @@ func TestLoadSketchFolderWrongMain(t *testing.T) {
 	sketchPath := filepath.Join("testdata", t.Name())
 	_, err := builder.SketchLoad(sketchPath, "")
 	require.Error(t, err)
-	require.Contains(t, err.Error(), "unable to find an sketch file in directory testdata")
+	require.Contains(t, err.Error(), "unable to find a sketch file in directory testdata")
 
 	_, err = builder.SketchLoad("does/not/exist", "")
 	require.Error(t, err)

Diff for: legacy/builder/container_setup.go

@@ -16,6 +16,8 @@
 package builder
 
 import (
+	"fmt"
+
 	bldr "github.com/arduino/arduino-cli/arduino/builder"
 	"github.com/arduino/arduino-cli/legacy/builder/builder_utils"
 	"github.com/arduino/arduino-cli/legacy/builder/i18n"
@@ -61,6 +63,9 @@ func (s *ContainerSetupHardwareToolsLibsSketchAndProps) Run(ctx *types.Context)
 		if err != nil {
 			return i18n.WrapError(err)
 		}
+		if sketch.MainFile == nil {
+			return fmt.Errorf("main file missing from sketch")
+		}
 		ctx.SketchLocation = paths.New(sketch.MainFile.Path)
 		ctx.Sketch = types.SketchToLegacy(sketch)
 	}

Diff for: test/test_compile.py

@@ -57,17 +57,21 @@ def test_compile_with_simple_sketch(run_command, data_dir):
     log_file_path = os.path.join(data_dir, log_file_name)
     result = run_command(
         "compile -b {fqbn} {sketch_path} --log-format json --log-file {log_file} --log-level trace".format(
-            fqbn=fqbn, sketch_path=sketch_path, log_file=log_file_path))
+            fqbn=fqbn, sketch_path=sketch_path, log_file=log_file_path
+        )
+    )
     assert result.ok
 
     # let's test from the logs if the hex file produced by successful compile is moved to our sketch folder
-    log_json = open(log_file_path, 'r')
+    log_json = open(log_file_path, "r")
     json_log_lines = log_json.readlines()
     expected_trace_sequence = [
         "Compile {sketch} for {fqbn} started".format(sketch=sketch_path, fqbn=fqbn),
-        "Compile {sketch} for {fqbn} successful".format(sketch=sketch_name, fqbn=fqbn)
+        "Compile {sketch} for {fqbn} successful".format(sketch=sketch_name, fqbn=fqbn),
     ]
-    assert is_message_sequence_in_json_log_traces(expected_trace_sequence, json_log_lines)
+    assert is_message_sequence_in_json_log_traces(
+        expected_trace_sequence, json_log_lines
+    )
 
 
 def test_compile_with_sketch_with_symlink_selfloop(run_command, data_dir):
@@ -94,8 +98,8 @@ def test_compile_with_sketch_with_symlink_selfloop(run_command, data_dir):
 
     # Build sketch for arduino:avr:uno
     result = run_command(
-        "compile -b {fqbn} {sketch_path}".format(
-            fqbn=fqbn, sketch_path=sketch_path))
+        "compile -b {fqbn} {sketch_path}".format(fqbn=fqbn, sketch_path=sketch_path)
+    )
     # The assertion is a bit relaxed in this case because win behaves differently from macOs and linux
     # returning a different error detailed message
     assert "Error during sketch processing" in result.stderr
@@ -118,8 +122,8 @@ def test_compile_with_sketch_with_symlink_selfloop(run_command, data_dir):
 
     # Build sketch for arduino:avr:uno
     result = run_command(
-        "compile -b {fqbn} {sketch_path}".format(
-            fqbn=fqbn, sketch_path=sketch_path))
+        "compile -b {fqbn} {sketch_path}".format(fqbn=fqbn, sketch_path=sketch_path)
+    )
     # The assertion is a bit relaxed also in this case because macOS behaves differently from win and linux:
     # the cli does not follow recursively the symlink til breaking
     assert "Error during sketch processing" in result.stderr
@@ -177,32 +181,50 @@ def test_compile_and_compile_combo(run_command, data_dir):
             continue
         assert isinstance(boards, list)
         for board in boards:
-            detected_boards.append(dict(address=port.get('address'), fqbn=board.get('FQBN')))
+            detected_boards.append(
+                dict(address=port.get("address"), fqbn=board.get("FQBN"))
+            )
 
     assert len(detected_boards) >= 1, "There are no boards available for testing"
 
     # Build sketch for each detected board
     for board in detected_boards:
-        log_file_name = "{fqbn}-compile.log".format(fqbn=board.get('fqbn').replace(":", "-"))
+        log_file_name = "{fqbn}-compile.log".format(
+            fqbn=board.get("fqbn").replace(":", "-")
+        )
         log_file_path = os.path.join(data_dir, log_file_name)
-        command_log_flags = "--log-format json --log-file {} --log-level trace".format(log_file_path)
-        result = run_command("compile -b {fqbn} --upload -p {address} {sketch_path} {log_flags}".format(
-            fqbn=board.get('fqbn'),
-            address=board.get('address'),
-            sketch_path=sketch_path,
-            log_flags=command_log_flags
-        ))
+        command_log_flags = "--log-format json --log-file {} --log-level trace".format(
+            log_file_path
+        )
+        result = run_command(
+            "compile -b {fqbn} --upload -p {address} {sketch_path} {log_flags}".format(
+                fqbn=board.get("fqbn"),
+                address=board.get("address"),
+                sketch_path=sketch_path,
+                log_flags=command_log_flags,
+            )
+        )
         assert result.ok
         # check from the logs if the bin file were uploaded on the current board
-        log_json = open(log_file_path, 'r')
+        log_json = open(log_file_path, "r")
         json_log_lines = log_json.readlines()
         expected_trace_sequence = [
-            "Compile {sketch} for {fqbn} started".format(sketch=sketch_path, fqbn=board.get('fqbn')),
-            "Compile {sketch} for {fqbn} successful".format(sketch=sketch_name, fqbn=board.get('fqbn')),
-            "Upload {sketch} on {fqbn} started".format(sketch=sketch_path, fqbn=board.get('fqbn')),
-            "Upload {sketch} on {fqbn} successful".format(sketch=sketch_name, fqbn=board.get('fqbn'))
+            "Compile {sketch} for {fqbn} started".format(
+                sketch=sketch_path, fqbn=board.get("fqbn")
+            ),
+            "Compile {sketch} for {fqbn} successful".format(
+                sketch=sketch_name, fqbn=board.get("fqbn")
+            ),
+            "Upload {sketch} on {fqbn} started".format(
+                sketch=sketch_path, fqbn=board.get("fqbn")
+            ),
+            "Upload {sketch} on {fqbn} successful".format(
+                sketch=sketch_name, fqbn=board.get("fqbn")
+            ),
         ]
-        assert is_message_sequence_in_json_log_traces(expected_trace_sequence, json_log_lines)
+        assert is_message_sequence_in_json_log_traces(
+            expected_trace_sequence, json_log_lines
+        )
 
 
 def is_message_sequence_in_json_log_traces(message_sequence, log_json_lines):
@@ -213,3 +235,34 @@ def is_message_sequence_in_json_log_traces(message_sequence, log_json_lines):
             if entry.get("msg") in message_sequence:
                 trace_entries.append(entry.get("msg"))
     return message_sequence == trace_entries
+
+
+def test_compile_blacklisted_sketchname(run_command, data_dir):
+    """
+    Compile should ignore folders named `RCS`, `.git` and the likes, but
+    it should be ok for a sketch to be named like RCS.ino
+    """
+    # Init the environment explicitly
+    result = run_command("core update-index")
+    assert result.ok
+
+    # Download latest AVR
+    result = run_command("core install arduino:avr")
+    assert result.ok
+
+    sketch_name = "RCS"
+    sketch_path = os.path.join(data_dir, sketch_name)
+    fqbn = "arduino:avr:uno"
+
+    # Create a test sketch
+    result = run_command("sketch new {}".format(sketch_path))
+    assert result.ok
+    assert "Sketch created in: {}".format(sketch_path) in result.stdout
+
+    # Build sketch for arduino:avr:uno
+    log_file_name = "compile.log"
+    log_file_path = os.path.join(data_dir, log_file_name)
+    result = run_command(
+        "compile -b {fqbn} {sketch_path}".format(fqbn=fqbn, sketch_path=sketch_path)
+    )
+    assert result.ok