Add warning when editing libraries

[facchinm]

Proposal to implement arduino/arduino-cli#1826

3rd commit is totally up to discussion (also the other two of course 😄 )
3rd commit was replaced by much saner #302

@matthijskooijman @PaulStoffregen

[ArduinoBot]

✅ Build completed.

⬇️ Build URL: http://downloads.arduino.cc/PR/arduino-builder/arduino-builder-301.zip

ℹ️ To test this build:

1. Replace arduino-builder binary (you can find it where you installed the IDE) with the provided one

[matthijskooijman]

Why would we recompile a library when it is changed? There is a dependency tracking mechanism that handles recompiling files when needed, so it is counterproductive to recompile everything when a source file changes. If there are problems, we should fix them rather than resorting to recompiling everything...

It could make sense to recompile everything / the entire library when library.properties is changed, even though it would be even better to capture all relevant parts of that in the build properties json, of course.

[facchinm]

@matthijskooijman my reasoning was more like:
if I write a lib and a file is "fine" (it compiles with warnings), every subsequent build will see the file as unchanged (if I didn't also modify its dependencies) so the warnings won't be shown.
Of course a more clever method could be caching the warnings and displaying them even if the file is not being actively recompiled (I think you proposed something like this some time ago) but it's a different story.

[PaulStoffregen]

Human factors matter here. Showing every possible warning on every build is probably the best thing to do on a technical level. But this isn't a primarily technical issue. How people respond is what matters. The goal is to gradually improve the code quality over the whole Arduino ecosystem.

Many times I've worked with other programmers who had sloppy code. Sometimes I've been that guy. When faced with hundreds or even thousands of compiler warnings, the task of fixing them all feels daunting. I recall working with a guy in the mid-1990s who switched from K&R to ANSI C and refused to fix compiler warnings, only because there were far too many (probably over 10,000). But only a dozen or so is an annoyance that feels worthy diverting time to fix.

At least for the first year, I would avoid recompiling other files only to show their warnings.

From a human perspective, especially a library author who maintains their code in spare time, only showing the warnings from the files they're currently editing has the best chance of encouraging those people to invest a small amount of their precious time to clean up those warnings. Especially when the warnings are in the particular code you're mentally considering in the moment, minor issues warnings are easiest to fix.

Humans have a much harder time when warnings show up from other code they're not currently editing or mentally engaging.

Even without this forced rebuilding of all files, eventually developers will almost certainly see the other warnings. Most library authors have at least a few different boards. They probably do occasionally change hardware and recompile everything.

Ultimately compiler warnings are about people. Humans respond much better to moderate numbers of warnings needing a manageable amount of effort to fix. Over time, showing warnings will lead to improvements, but it will be a gradual process. I would avoid pushing too hard, especially all at once.

[matthijskooijman]

@facchinm, ah, warnings being hidden after a "succesful" compile is indeed a problem, but I think this solution is both too unsubtle (people will end up waiting a lot for bigger libraries), and not broad enough - you don't solve the problem for anything other than libraries. If we really want to fix this, it might be a better investment to do it properly and indeed cache warning messages along with the .o file (possibly append them to the .d file the compiler generates?).

As for @PaulStoffregen comment, he has a fair point too, which would suggest to not fix this at all. However, I'd like to point out that while his point holds for an existing library that has a lot of warnings, things are different when a library is correctly maintained and has no existing warnings (only those being introduced currently). In such cases, I've found it annoying that warnings are only shown once, since often they scroll out of view the first time, and when you have the errors sorted out and want to look at any remaining warnings, they are no longer shown. This is especially annoying since the IDE has no "rebuild all" option, other than restarting the IDE. This reasoning holds for the sketch just as well.

Finally, if we add these changes, there should at least be a message "Library was changed, fully recompiling this library with all warnings enabled" to prevent user surprise.

[matthijskooijman]

For reference, here's the issue this is trying to fix: https://github.com/arduino/Arduino/issues/5493

[matthijskooijman]

See also arduino/arduino-ide#1630 for relevant discussion wrt warnings.

One thought to carry over: If warnings are forced to be enabled for libraries being edited, there is no way to disable them for the user if he's so inclined, which might be a nuisance for users (the linked issue has a proposal for a preference for this).

[cwrseck]

Long ago, around arduino 1.0.1, I was trying to debug a problem and needed to check what the library was doing. When I compiled the library with make, not with the IDE, I was pretty horrified by the number of warnings - even compiling with what I thought were the minimal -Wall -Wextra flags.

I couldn't deal with code that rough - any real problems were submerged in trash - so I spent a couple of days going through the standard libraries and clearing stuff out, There were around 1400 warnings, as I recall, hiding three or four actual bugs. I reported this on the Arduino forum (?) and was told that it was deliberate, that beginners were put off by warnings in their code.

There's no way I'd release code in that state, so shortly afterwards I moved to programming directly in C and dropped the Arduino libraries altogether. Whether it's now worth one of the maintainers spending a few days fixing stuff is a matter for them to decide.

Will

[ArduinoBot]

✅ Build completed.

⬇️ Build URL: http://downloads.arduino.cc/PR/arduino-builder/arduino-builder-301.zip

ℹ️ To test this build:

1. Replace arduino-builder binary (you can find it where you installed the IDE) with the provided one