Fix binaries not being exported when using settings or env var (#1056)

Author: silvanocerza

cli/compile/compile.go

@@ -87,7 +87,11 @@ func NewCommand() *cobra.Command {
 	command.Flags().BoolVar(&optimizeForDebug, "optimize-for-debug", false, "Optional, optimize compile output for debugging, rather than for release.")
 	command.Flags().StringVarP(&programmer, "programmer", "P", "", "Optional, use the specified programmer to upload.")
 	command.Flags().BoolVar(&clean, "clean", false, "Optional, cleanup the build folder and do not use any cached build.")
-	command.Flags().BoolVarP(&exportBinaries, "export-binaries", "e", false, "If set built binaries will be exported to the sketch folder.")
+	// We must use the following syntax for this flag since it's also bound to settings, we could use the other one too
+	// but it wouldn't make sense since we still must explicitly set the exportBinaries variable by reading from settings.
+	// This must be done because the value is set when the binding is accessed from viper. Accessing from cobra would only
+	// read the value if the flag is set explicitly by the user.
+	command.Flags().BoolP("export-binaries", "e", false, "If set built binaries will be exported to the sketch folder.")
 
 	configuration.Settings.BindPFlag("sketch.always_export_binaries", command.Flags().Lookup("export-binaries"))
 
@@ -107,6 +111,10 @@ func run(cmd *cobra.Command, args []string) {
 	}
 
 	sketchPath := initSketchPath(path)
+	// 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.
+	exportBinaries = configuration.Settings.GetBool("sketch.always_export_binaries")
 
 	_, err = compile.Compile(context.Background(), &rpc.CompileReq{
 		Instance:         inst,

test/test_compile.py

@@ -340,3 +340,74 @@ def test_compile_with_custom_build_path(run_command, data_dir):
     assert not (build_dir / f"{sketch_name}.ino.hex").exists()
     assert not (build_dir / f"{sketch_name}.ino.with_bootloader.bin").exists()
     assert not (build_dir / f"{sketch_name}.ino.with_bootloader.hex").exists()
+
+
+def test_compile_with_export_binaries_env_var(run_command, data_dir, downloads_dir):
+    # Init the environment explicitly
+    run_command("core update-index")
+
+    # Download latest AVR
+    run_command("core install arduino:avr")
+
+    sketch_name = "CompileWithExportBinariesEnvVar"
+    sketch_path = Path(data_dir, sketch_name)
+    fqbn = "arduino:avr:uno"
+
+    # Create a test sketch
+    assert run_command("sketch new {}".format(sketch_path))
+
+    env = {
+        "ARDUINO_DATA_DIR": data_dir,
+        "ARDUINO_DOWNLOADS_DIR": downloads_dir,
+        "ARDUINO_SKETCHBOOK_DIR": data_dir,
+        "ARDUINO_SKETCH_ALWAYS_EXPORT_BINARIES": "true",
+    }
+    # Test compilation with export binaries env var set
+    result = run_command(f"compile -b {fqbn} {sketch_path}", custom_env=env)
+    assert result.ok
+    assert Path(sketch_path, "build").exists()
+    assert Path(sketch_path, "build").is_dir()
+
+    # Verifies binaries are exported when export binaries env var is set
+    assert (sketch_path / "build" / fqbn.replace(":", ".") / f"{sketch_name}.ino.eep").exists()
+    assert (sketch_path / "build" / fqbn.replace(":", ".") / f"{sketch_name}.ino.elf").exists()
+    assert (sketch_path / "build" / fqbn.replace(":", ".") / f"{sketch_name}.ino.hex").exists()
+    assert (sketch_path / "build" / fqbn.replace(":", ".") / f"{sketch_name}.ino.with_bootloader.bin").exists()
+    assert (sketch_path / "build" / fqbn.replace(":", ".") / f"{sketch_name}.ino.with_bootloader.hex").exists()
+
+
+def test_compile_with_export_binaries_config(run_command, data_dir, downloads_dir):
+    # Init the environment explicitly
+    run_command("core update-index")
+
+    # Download latest AVR
+    run_command("core install arduino:avr")
+
+    sketch_name = "CompileWithExportBinariesConfig"
+    sketch_path = Path(data_dir, sketch_name)
+    fqbn = "arduino:avr:uno"
+
+    # Create a test sketch
+    assert run_command("sketch new {}".format(sketch_path))
+
+    # Create settings with export binaries set to true
+    env = {
+        "ARDUINO_DATA_DIR": data_dir,
+        "ARDUINO_DOWNLOADS_DIR": downloads_dir,
+        "ARDUINO_SKETCHBOOK_DIR": data_dir,
+        "ARDUINO_SKETCH_ALWAYS_EXPORT_BINARIES": "true",
+    }
+    assert run_command("config init --dest-dir .", custom_env=env)
+
+    # Test compilation with export binaries env var set
+    result = run_command(f"compile -b {fqbn} {sketch_path}")
+    assert result.ok
+    assert Path(sketch_path, "build").exists()
+    assert Path(sketch_path, "build").is_dir()
+
+    # Verifies binaries are exported when export binaries env var is set
+    assert (sketch_path / "build" / fqbn.replace(":", ".") / f"{sketch_name}.ino.eep").exists()
+    assert (sketch_path / "build" / fqbn.replace(":", ".") / f"{sketch_name}.ino.elf").exists()
+    assert (sketch_path / "build" / fqbn.replace(":", ".") / f"{sketch_name}.ino.hex").exists()
+    assert (sketch_path / "build" / fqbn.replace(":", ".") / f"{sketch_name}.ino.with_bootloader.bin").exists()
+    assert (sketch_path / "build" / fqbn.replace(":", ".") / f"{sketch_name}.ino.with_bootloader.hex").exists()