Fix upload command not working as expected when certain flags are used (#1429)

* Fix upload command not working as expected when certain flags are used

* Fix absurd upload corner case

Author: silvanocerza

arduino/sketch/sketch.go

@@ -58,6 +58,10 @@ var tr = i18n.Tr
 // New creates an Sketch instance by reading all the files composing a sketch and grouping them
 // by file type.
 func New(path *paths.Path) (*Sketch, error) {
+	if path == nil {
+		return nil, fmt.Errorf(tr("sketch path is not valid"))
+	}
+
 	path = path.Canonical()
 	if !path.IsDir() {
 		path = path.Parent()

arduino/sketch/sketch_test.go

@@ -31,8 +31,12 @@ func TestNew(t *testing.T) {
 	mainFilePath := sketchFolderPath.Join(fmt.Sprintf("%s.ino", "SketchSimple"))
 	otherFile := sketchFolderPath.Join("other.cpp")
 
+	sketch, err := New(nil)
+	assert.Nil(t, sketch)
+	assert.Error(t, err)
+
 	// Loading using Sketch folder path
-	sketch, err := New(sketchFolderPath)
+	sketch, err = New(sketchFolderPath)
 	assert.Nil(t, err)
 	assert.True(t, mainFilePath.EquivalentTo(sketch.MainFile))
 	assert.True(t, sketchFolderPath.EquivalentTo(sketch.FullPath))

cli/upload/upload.go

@@ -84,24 +84,31 @@ func run(command *cobra.Command, args []string) {
 	sketchPath := arguments.InitSketchPath(path)
 
 	// .pde files are still supported but deprecated, this warning urges the user to rename them
-	if files := sketch.CheckForPdeFiles(sketchPath); len(files) > 0 {
+	if files := sketch.CheckForPdeFiles(sketchPath); len(files) > 0 && importDir == "" && importFile == "" {
 		feedback.Error(tr("Sketches with .pde extension are deprecated, please rename the following files to .ino:"))
 		for _, f := range files {
 			feedback.Error(f)
 		}
 	}
 
 	sk, err := sketch.New(sketchPath)
-	if err != nil {
+	if err != nil && importDir == "" && importFile == "" {
 		feedback.Errorf(tr("Error during Upload: %v"), err)
 		os.Exit(errorcodes.ErrGeneric)
 	}
+
 	discoveryPort, err := port.GetPort(instance, sk)
 	if err != nil {
 		feedback.Errorf(tr("Error during Upload: %v"), err)
 		os.Exit(errorcodes.ErrGeneric)
 	}
 
+	if fqbn == "" && sk != nil && sk.Metadata != nil {
+		// If the user didn't specify an FQBN and a sketch.json file is present
+		// read it from there.
+		fqbn = sk.Metadata.CPU.Fqbn
+	}
+
 	userFieldRes, err := upload.SupportedUserFields(context.Background(), &rpc.SupportedUserFieldsRequest{
 		Instance: instance,
 		Fqbn:     fqbn,
@@ -118,10 +125,14 @@ func run(command *cobra.Command, args []string) {
 		fields = arguments.AskForUserFields(userFieldRes.UserFields)
 	}
 
+	if sketchPath != nil {
+		path = sketchPath.String()
+	}
+
 	if _, err := upload.Upload(context.Background(), &rpc.UploadRequest{
 		Instance:   instance,
 		Fqbn:       fqbn,
-		SketchPath: sketchPath.String(),
+		SketchPath: path,
 		Port:       discoveryPort.ToRPC(),
 		Verbose:    verbose,
 		Verify:     verify,

i18n/data/en.po

@@ -726,9 +726,9 @@ msgstr "Error during JSON encoding of the output: %v"
 #: cli/compile/compile.go:212
 #: cli/compile/compile.go:244
 #: cli/upload/upload.go:96
-#: cli/upload/upload.go:101
-#: cli/upload/upload.go:111
-#: cli/upload/upload.go:134
+#: cli/upload/upload.go:102
+#: cli/upload/upload.go:118
+#: cli/upload/upload.go:145
 msgid "Error during Upload: %v"
 msgstr "Error during Upload: %v"
 
@@ -2252,7 +2252,7 @@ msgid "Upload the bootloader."
 msgstr "Upload the bootloader."
 
 #: cli/compile/compile.go:218
-#: cli/upload/upload.go:117
+#: cli/upload/upload.go:124
 msgid "Uploading to specified board using %s protocol requires the following info:"
 msgstr "Uploading to specified board using %s protocol requires the following info:"
 
@@ -2443,7 +2443,7 @@ msgstr "calling %[1]s: %[2]w"
 msgid "can't find latest release of %s"
 msgstr "can't find latest release of %s"
 
-#: arduino/sketch/sketch.go:101
+#: arduino/sketch/sketch.go:105
 msgid "can't find main Sketch file in %s"
 msgstr "can't find main Sketch file in %s"
 
@@ -2535,7 +2535,7 @@ msgstr "creating temp dir for extraction: %s"
 msgid "data section exceeds available space in board"
 msgstr "data section exceeds available space in board"
 
-#: arduino/sketch/sketch.go:207
+#: arduino/sketch/sketch.go:211
 msgid "decoding sketch metadata: %s"
 msgstr "decoding sketch metadata: %s"
 
@@ -2587,7 +2587,7 @@ msgstr "downloading %[1]s tool: %[2]s"
 msgid "empty board identifier"
 msgstr "empty board identifier"
 
-#: arduino/sketch/sketch.go:196
+#: arduino/sketch/sketch.go:200
 msgid "encoding sketch metadata: %s"
 msgstr "encoding sketch metadata: %s"
 
@@ -2710,7 +2710,7 @@ msgstr "getting parent dir of %[1]s: %[2]s"
 msgid "getting tool dependencies for platform %[1]s: %[2]s"
 msgstr "getting tool dependencies for platform %[1]s: %[2]s"
 
-#: arduino/sketch/sketch.go:151
+#: arduino/sketch/sketch.go:155
 msgid "importing sketch metadata: %s"
 msgstr "importing sketch metadata: %s"
 
@@ -2942,7 +2942,7 @@ msgstr "moving extracted archive to destination dir: %s"
 msgid "multiple build artifacts found: '%[1]s' and '%[2]s'"
 msgstr "multiple build artifacts found: '%[1]s' and '%[2]s'"
 
-#: arduino/sketch/sketch.go:73
+#: arduino/sketch/sketch.go:77
 msgid "multiple main sketch files found (%[1]v, %[2]v)"
 msgstr "multiple main sketch files found (%[1]v, %[2]v)"
 
@@ -2970,7 +2970,7 @@ msgstr "no unique root dir in archive, found '%[1]s' and '%[2]s'"
 msgid "no upload port provided"
 msgstr "no upload port provided"
 
-#: arduino/sketch/sketch.go:259
+#: arduino/sketch/sketch.go:263
 msgid "no valid sketch found in %[1]s: missing %[2]s"
 msgstr "no valid sketch found in %[1]s: missing %[2]s"
 
@@ -3095,7 +3095,7 @@ msgstr "reading directory %[1]s: %[2]s"
 msgid "reading file %[1]s: %[2]s"
 msgstr "reading file %[1]s: %[2]s"
 
-#: arduino/sketch/sketch.go:229
+#: arduino/sketch/sketch.go:233
 msgid "reading files: %v"
 msgstr "reading files: %v"
 
@@ -3123,7 +3123,7 @@ msgstr "reading library_index.json: %s"
 msgid "reading package root dir: %s"
 msgstr "reading package root dir: %s"
 
-#: arduino/sketch/sketch.go:188
+#: arduino/sketch/sketch.go:192
 msgid "reading sketch metadata %[1]s: %[2]s"
 msgstr "reading sketch metadata %[1]s: %[2]s"
 
@@ -3185,6 +3185,10 @@ msgstr "searching package root dir: %s"
 msgid "setting DTR to OFF"
 msgstr "setting DTR to OFF"
 
+#: arduino/sketch/sketch.go:62
+msgid "sketch path is not valid"
+msgstr "sketch path is not valid"
+
 #: cli/board/attach.go:35
 #: cli/sketch/archive.go:38
 msgid "sketchPath"
@@ -3339,7 +3343,7 @@ msgstr "unknown package %s"
 msgid "unknown platform %s:%s"
 msgstr "unknown platform %s:%s"
 
-#: arduino/sketch/sketch.go:142
+#: arduino/sketch/sketch.go:146
 msgid "unknown sketch file extension '%s'"
 msgstr "unknown sketch file extension '%s'"
 
@@ -3359,7 +3363,7 @@ msgstr "upgrade everything to the latest version"
 msgid "uploading error: %s"
 msgstr "uploading error: %s"
 
-#: arduino/sketch/sketch.go:212
+#: arduino/sketch/sketch.go:216
 msgid "writing sketch metadata %[1]s: %[2]s"
 msgstr "writing sketch metadata %[1]s: %[2]s"
 

i18n/rice-box.go

@@ -71,7 +71,7 @@ def test_upload_with_input_dir_flag(run_command, data_dir, detected_boards):
         assert run_command(f"compile -b {fqbn} {sketch_path} --output-dir {output_dir}")
 
         # Upload with --input-dir flag
-        assert run_command(f"upload -b {fqbn} -p {address} --input-dir {output_dir} {sketch_path}")
+        assert run_command(f"upload -b {fqbn} -p {address} --input-dir {output_dir}")
 
 
 def test_upload_with_input_file_flag(run_command, data_dir, detected_boards):
@@ -320,7 +320,7 @@ def test_upload_with_input_dir_containing_multiple_binaries(run_command, data_di
         assert res.failed
         assert (
             "Error during Upload: "
-            + "retrieving build artifacts: "
+            + "Error finding build artifacts: "
             + "autodetect build artifact: "
             + "multiple build artifacts found:"
             in res.stderr
@@ -358,7 +358,7 @@ def test_compile_and_upload_combo_sketch_with_mismatched_casing(run_command, dat
         # 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
+        assert "Error during build: Can't open sketch: no valid sketch found in" in res.stderr
 
 
 def test_upload_sketch_with_mismatched_casing(run_command, data_dir, detected_boards, wait_for_board):
@@ -381,4 +381,4 @@ def test_upload_sketch_with_mismatched_casing(run_command, data_dir, detected_bo
         # 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
+        assert "Error during Upload: no valid sketch found in" in res.stderr