Deprecate compile --build-properties flag in favor of new featureful --build-property flag (#1044)

* Add support for definitions containing quotes in compile --build-properties flag

* Deprecate --build-properties flag in favor of --build-property

* Fix some escaping issues on Windows

* Simplify compile examples

Author: silvanocerza

cli/compile/compile.go

@@ -57,12 +57,16 @@ var (
 // NewCommand created a new `compile` command
 func NewCommand() *cobra.Command {
 	command := &cobra.Command{
-		Use:     "compile",
-		Short:   "Compiles Arduino sketches.",
-		Long:    "Compiles Arduino sketches.",
-		Example: "  " + os.Args[0] + " compile -b arduino:avr:uno /home/user/Arduino/MySketch",
-		Args:    cobra.MaximumNArgs(1),
-		Run:     run,
+		Use:   "compile",
+		Short: "Compiles Arduino sketches.",
+		Long:  "Compiles Arduino sketches.",
+		Example: "" +
+			"  " + os.Args[0] + " compile -b arduino:avr:uno /home/user/Arduino/MySketch\n" +
+			"  " + os.Args[0] + ` compile -b arduino:avr:uno --build-property "build.extra_flags=\"-DMY_DEFINE=\"hello world\"\"" /home/user/Arduino/MySketch` + "\n" +
+			"  " + os.Args[0] + ` compile -b arduino:avr:uno --build-property "build.extra_flags=-DPIN=2 \"-DMY_DEFINE=\"hello world\"\"" /home/user/Arduino/MySketch` + "\n" +
+			"  " + os.Args[0] + ` compile -b arduino:avr:uno --build-property build.extra_flags=-DPIN=2 --build-property "compiler.cpp.extra_flags=\"-DSSID=\"hello world\"\"" /home/user/Arduino/MySketch` + "\n",
+		Args: cobra.MaximumNArgs(1),
+		Run:  run,
 	}
 
 	command.Flags().StringVarP(&fqbn, "fqbn", "b", "", "Fully Qualified Board Name, e.g.: arduino:avr:uno")
@@ -74,6 +78,8 @@ func NewCommand() *cobra.Command {
 		"Path where to save compiled files. If omitted, a directory will be created in the default temporary path of your OS.")
 	command.Flags().StringSliceVar(&buildProperties, "build-properties", []string{},
 		"List of custom build properties separated by commas. Or can be used multiple times for multiple properties.")
+	command.Flags().StringArrayVar(&buildProperties, "build-property", []string{},
+		"Override a build property with a custom value. Can be used multiple times for multiple properties.")
 	command.Flags().StringVar(&warnings, "warnings", "none",
 		`Optional, can be "none", "default", "more" and "all". Defaults to "none". Used to tell gcc which warning level to use (-W flag).`)
 	command.Flags().BoolVarP(&verbose, "verbose", "v", false, "Optional, turns on verbose mode.")
@@ -95,6 +101,8 @@ func NewCommand() *cobra.Command {
 
 	configuration.Settings.BindPFlag("sketch.always_export_binaries", command.Flags().Lookup("export-binaries"))
 
+	command.Flags().MarkDeprecated("build-properties", "please use --build-property instead.")
+
 	return command
 }
 

go.mod

@@ -40,6 +40,7 @@ require (
 	github.com/sirupsen/logrus v1.4.2
 	github.com/spf13/cobra v1.0.1-0.20200710201246-675ae5f5a98c
 	github.com/spf13/jwalterweatherman v1.0.0
+	github.com/spf13/pflag v1.0.3
 	github.com/spf13/viper v1.6.2
 	github.com/stretchr/testify v1.6.1
 	go.bug.st/cleanup v1.0.0

test/conftest.py

@@ -212,11 +212,14 @@ def detected_boards(run_command):
 
 @pytest.fixture(scope="function")
 def copy_sketch(working_dir):
-    # Copies sketch for testing
-    sketch_path = Path(__file__).parent / "testdata" / "sketch_simple"
-    test_sketch_path = Path(working_dir) / "sketch_simple"
-    shutil.copytree(sketch_path, test_sketch_path)
-    yield str(test_sketch_path)
+    def _copy(sketch_name):
+        # Copies sketch to working directory for testing
+        sketch_path = Path(__file__).parent / "testdata" / sketch_name
+        test_sketch_path = Path(working_dir, sketch_name)
+        shutil.copytree(sketch_path, test_sketch_path)
+        return str(test_sketch_path)
+
+    return _copy
 
 
 @pytest.fixture(scope="function")

test/test_compile.py

@@ -233,6 +233,133 @@ def test_compile_without_precompiled_libraries(run_command, data_dir):
     assert result.ok
 
 
+def test_compile_with_build_properties_flag(run_command, data_dir, copy_sketch):
+    # Init the environment explicitly
+    assert run_command("core update-index")
+
+    # Install Arduino AVR Boards
+    assert run_command("core install arduino:avr@1.8.3")
+
+    sketch_path = copy_sketch("sketch_with_single_string_define")
+    fqbn = "arduino:avr:uno"
+
+    # Compile using a build property with quotes
+    res = run_command(
+        f"compile -b {fqbn} "
+        + '--build-properties="build.extra_flags=\\"-DMY_DEFINE=\\"hello world\\"\\"" '
+        + f"{sketch_path} --verbose --clean"
+    )
+    assert res.failed
+    assert "Flag --build-properties has been deprecated, please use --build-property instead." not in res.stderr
+
+    # Try again with quotes
+    res = run_command(
+        f"compile -b {fqbn} "
+        + '--build-properties="build.extra_flags=-DMY_DEFINE=\\"hello\\"" '
+        + f"{sketch_path} --verbose --clean"
+    )
+    assert res.failed
+    assert "Flag --build-properties has been deprecated, please use --build-property instead." not in res.stderr
+
+    # Try without quotes
+    sketch_path = copy_sketch("sketch_with_single_int_define")
+    res = run_command(
+        f"compile -b {fqbn} "
+        + '--build-properties="build.extra_flags=-DMY_DEFINE=1" '
+        + f"{sketch_path} --verbose --clean"
+    )
+    assert res.ok
+    assert "Flag --build-properties has been deprecated, please use --build-property instead." in res.stderr
+    assert "-DMY_DEFINE=1" in res.stdout
+
+    sketch_path = copy_sketch("sketch_with_multiple_int_defines")
+    res = run_command(
+        f"compile -b {fqbn} "
+        + '--build-properties="build.extra_flags=-DFIRST_PIN=1,compiler.cpp.extra_flags=-DSECOND_PIN=2" '
+        + f"{sketch_path} --verbose --clean"
+    )
+    assert res.ok
+    assert "Flag --build-properties has been deprecated, please use --build-property instead." in res.stderr
+    assert "-DFIRST_PIN=1" in res.stdout
+    assert "-DSECOND_PIN=2" in res.stdout
+
+
+def test_compile_with_build_property_containing_quotes(run_command, data_dir, copy_sketch):
+    # Init the environment explicitly
+    assert run_command("core update-index")
+
+    # Install Arduino AVR Boards
+    assert run_command("core install arduino:avr@1.8.3")
+
+    sketch_path = copy_sketch("sketch_with_single_string_define")
+    fqbn = "arduino:avr:uno"
+
+    # Compile using a build property with quotes
+    res = run_command(
+        f"compile -b {fqbn} "
+        + '--build-property="build.extra_flags=\\"-DMY_DEFINE=\\"hello world\\"\\"" '
+        + f"{sketch_path} --verbose"
+    )
+    assert res.ok
+    assert '-DMY_DEFINE=\\"hello world\\"' in res.stdout
+
+
+def test_compile_with_multiple_build_property_flags(run_command, data_dir, copy_sketch, working_dir):
+    # Init the environment explicitly
+    assert run_command("core update-index")
+
+    # Install Arduino AVR Boards
+    assert run_command("core install arduino:avr@1.8.3")
+
+    sketch_path = copy_sketch("sketch_with_multiple_defines")
+    fqbn = "arduino:avr:uno"
+
+    # Compile using multiple build properties separated by a space
+    res = run_command(
+        f"compile -b {fqbn} "
+        + '--build-property="compiler.cpp.extra_flags=\\"-DPIN=2 -DSSID=\\"This is a String\\"\\"" '
+        + f"{sketch_path} --verbose --clean"
+    )
+    assert res.failed
+
+    # Compile using multiple build properties separated by a space and properly quoted
+    res = run_command(
+        f"compile -b {fqbn} "
+        + '--build-property="compiler.cpp.extra_flags=-DPIN=2 \\"-DSSID=\\"This is a String\\"\\"" '
+        + f"{sketch_path} --verbose --clean"
+    )
+    assert res.ok
+    assert '-DPIN=2 "-DSSID=\\"This is a String\\""' in res.stdout
+
+    # Tries compilation using multiple build properties separated by a comma
+    res = run_command(
+        f"compile -b {fqbn} "
+        + '--build-property="compiler.cpp.extra_flags=\\"-DPIN=2,-DSSID=\\"This is a String\\"\\"\\" '
+        + f"{sketch_path} --verbose --clean"
+    )
+    assert res.failed
+
+    res = run_command(
+        f"compile -b {fqbn} "
+        + '--build-property="compiler.cpp.extra_flags=\\"-DPIN=2\\"" '
+        + '--build-property="compiler.cpp.extra_flags=\\"-DSSID=\\"This is a String\\"\\"" '
+        + f"{sketch_path} --verbose --clean"
+    )
+    assert res.failed
+    assert "-DPIN=2" not in res.stdout
+    assert '-DSSID=\\"This is a String\\"' in res.stdout
+
+    res = run_command(
+        f"compile -b {fqbn} "
+        + '--build-property="compiler.cpp.extra_flags=\\"-DPIN=2\\"" '
+        + '--build-property="build.extra_flags=\\"-DSSID=\\"hello world\\"\\"" '
+        + f"{sketch_path} --verbose --clean"
+    )
+    assert res.ok
+    assert "-DPIN=2" in res.stdout
+    assert '-DSSID=\\"hello world\\"' in res.stdout
+
+
 def test_compile_with_output_dir_flag(run_command, data_dir):
     # Init the environment explicitly
     run_command("core update-index")