arduino-builder incompatible with paths that contain , or ;

[per1234]

Using Arduino IDE Beta Build 61 and Arduino IDE 1.8.6 Hourly Build 2018/05/28 09:33 under Windows 10 64 bit

1. File > Preferences > Sketchbook location > set the sketchbook location to a folder named foo;,bar > OK
2. Sketch > Verify/Compile - Compilation fails:

C:\Program Files (x86)\ArduinoIDE\arduino-PR-beta1.9-BUILD-61\arduino-builder -dump-prefs -logger=machine -hardware C:\Program Files (x86)\ArduinoIDE\arduino-PR-beta1.9-BUILD-61\hardware -hardware C:\Users\per\AppData\Local\Arduino15\packages -tools C:\Program Files (x86)\ArduinoIDE\arduino-PR-beta1.9-BUILD-61\tools-builder -tools C:\Program Files (x86)\ArduinoIDE\arduino-PR-beta1.9-BUILD-61\hardware\tools\avr -tools C:\Users\per\AppData\Local\Arduino15\packages -built-in-libraries C:\Program Files (x86)\ArduinoIDE\arduino-PR-beta1.9-BUILD-61\libraries -libraries E:\foo;,bar\libraries -fqbn=arduino:avr:nano:cpu=atmega328 -ide-version=10900 -build-path C:\Users\per\AppData\Local\Temp\arduino_build_189079 -warnings=all -prefs=build.warn_data_percentage=75 -prefs=runtime.tools.avrdude.path=C:\Users\per\AppData\Local\Arduino15\packages\arduino\tools\avrdude\6.3.0-arduino14 -prefs=runtime.tools.avr-gcc.path=C:\Users\per\AppData\Local\Arduino15\packages\arduino\tools\avr-gcc\5.4.0-atmel3.6.1-arduino2 -prefs=runtime.tools.arduinoOTA.path=C:\Program Files (x86)\ArduinoIDE\arduino-PR-beta1.9-BUILD-61\hardware\tools\avr -verbose C:\Users\per\AppData\Local\Temp\untitled693801815.tmp\sketch_may31a\sketch_may31a.ino
open E:\foo: The system cannot find the file specified.

A similar error occurs when the Arduino IDE is installed under a path that contains ,. The same would surely occur with an installation path that contained ; but the Arduino IDE won't start under this path (arduino/Arduino#7658).

If arduino-builder can't be made to support these characters then I recommend the sketchbook preference reject paths with them in it.

Originally reported at:
http://forum.arduino.cc/index.php?topic=550716

[facchinm]

This looks much less scary than arduino/Arduino#7658 since it's only a sting splitting issue. I'll check it later today

[facchinm]

Forget my words, this is as bad as the other since it involves directly go flags package.
So, unless we are crazy enough to rewrite flags, it would stay there forever 🙁

[matthijskooijman]

You mean the commandline flags parsing package (this one, I guess? https://github.com/jessevdk/go-flags)

I take it you mean that it is already the go-flags package that somehow splits on semicolons?

Any idea if using semicolons/commas inside an option is some kind of feature of that package? Or is this really just a bug in that package?

[facchinm]

No, really the bundled one https://golang.org/pkg/flag/ (flag, not flags, my fault).
My no-brainer test:

diff --git a/arduino-builder/main.go b/arduino-builder/main.go
index 89c31bd..67573b7 100644
--- a/arduino-builder/main.go
+++ b/arduino-builder/main.go
@@ -255,6 +255,9 @@ func main() {
        }
 
        // FLAG_HARDWARE
+
+       fmt.Println(hardwareFoldersFlag)
+
        if hardwareFolders, err := toSliceOfUnquoted(hardwareFoldersFlag); err != nil {
                printCompleteError(err)
        } else if len(hardwareFolders) > 0 {

CLI: ./arduino-builder -compile "-hardware=/foo/temp;,foo" "-hardware=123;456" "-hardware=789,xxx" -fqbn=arduino:avr:uno -tools=/home/foo /tmp/blink.ino

output:

[/foo/temp; foo 123;456 789 xxx]

so it splits the comma , not the semicolon (probably due to the fact that you can specify more than one argument, comma separated)

[matthijskooijman]

I actually believe we implemented this ourselves: https://github.com/arduino/arduino-builder/blob/master/arduino-builder/main.go#L99-L113

Are we actually using this from the IDE (and/or is this a documented API)? I think we should not, instead just pass -hardware multiple times. On the arduino-builder side, we probably still need the custom type for the folder list, to allow specifying an option multple times (which I think flag does not support out of the box.

[facchinm]

Ah right, I totally missed it. We are not using it in any way, just passing multiple times is the way to go (so removing that line should solve the issue).