using multiple linking flags when overriding a build property

[artynet]

As described in the object I'd like to override the build properties of my custom boards.txt file leveraging all the benefits of the FreeRTOS samd21 library. For this reason, I have defined an empty build variable :

build.freertos.memory_wrapping_flags=

which is called at runtime with :

arduino-cli compile --fqbn <myboard> --build-properties build.freertos.memory_wrapping_flags="-Wl,--wrap=malloc -Wl,--wrap=free -Wl,--wrap=calloc -Wl,--wrap=realloc" mysketch.ino -vv

but just -Wl is set as its value so all which is located after the first comma is removed, altough enclosed within double quotes. I can see that because if I launch the command with the --show-properties option the output is just :

memory_wrapping_flags=-Wl

any hints about that ?

Environment

• CLI version (output of arduino-cli version): 0.7.0 Commit: 3809fc3
• OS and platform: Linux x64

[artynet]

@per1234 @masci

this quick fix https://github.com/artynet/arduino-cli/commit/e8920fc2895134f9608f0c838ee45a14bb4c1523 seems to solve my problem. Would you mind taking a look at that so, in case, I can process a pull request ?

Thanks in advance

[masci]

@artynet I think StringArrayVar is the way to go here, so +1 from me. This will slightly change (for the better) the way --build-properties works, so we should also add an example in the README file showing how to pass multiple properties at once.

[artynet]

At this very point we should think of writing something like that. Given two sets of custom building flags :

• build.platform_EXTRA_flags="flag1 flag2 flag3"
• build.platform_GENERIC_flags="flag4 flag5 flag6"

we can override the main build properties this way :

--build-properties build.platform_GENERIC_flags="flag4 flag5 flag6 flag1 flag2 flag3"

or

--build-properties build.platform_GENERIC_flags="flag4 flag5 flag6" --build-properties build.platform_EXTRA_flags="flag1 flag2 flag3"

the first being a general override while the second a separated one for single set of flags. Defining :

--build-properties build.platform_GENERIC_flags="flag4 flag5 flag6",build.platform_EXTRA_flags="flag1 flag2 flag3"

would result in this merge :

"flag4 flag5 flag6",build.platform_EXTRA_flags="flag1 flag2 flag3"

where has no sense splitting an array by comma this time...

[artynet]

Hello @masci, any update on this issue ?

[masci]

Ok for me to move on with a PR containing the comment linked above. Not sure I got the final example in your last comment: is that ok from a user perspective? Or is it a counter-example against changing that flag to StringArray?

Sorry I'm not a power Arduino user so I'm a bit slow at getting use cases :stuck_out_tongue:

[artynet]

Basically, we should tell the user to apply the switch for every single set of properties to override them. Comma separate values are not allowed anymore for multiple entries and, if passed that way, they won't be split as it should.

The same issue might apply to the libraries switch just introduced a couple of hours ago....please be aware of that !

[matthijskooijman]

No longer allowing comma-separated values seems like a good solution here. There should usually not be any problem with just supplying the option multiple times, and using a comma to separate without any means to escape literal commas is problematic.

We could just switch the meaning of the current --build-properties flag (as implemented in https://github.com/arduino/arduino-cli/issues/532#issuecomment-569923084), but if we do that, it's a bit weird that --build-properties is plural, while it only allows specifying a single property. It also breaks backward compatibility.

An alternative approach would be to introduce a new --build-property flag, with the new semantics, and keeping the old flag for now, but mark it as deprecated. How is that?

[artynet]

@matthijskooijman that sounds right to me as well