arduino / arduino-cli

Arduino command line tool
https://arduino.github.io/arduino-cli/latest/
GNU General Public License v3.0
4.33k stars 375 forks source link

When list of included libraries changes, all code should be recompiled #970

Open matthijskooijman opened 4 years ago

matthijskooijman commented 4 years ago

Bug Report

Current behavior

There is some code in place to force a recompile of everything when the compilation options change (e.g. when selecting a different board or board options). However, when the list of included libraries changes, this also changes the compiler flags, but currently this does not force a recompile of everything, meaning that some files might have been compiled with different include flags than others.

This is usually not a problem, since e.g. when removing an include from your sketch, which is not used by any other source files, this should not change behavior (and if the file is used, then the missing include will trigger the library to be included anyway).

However, this is not generally true. In particular, when multiple libraries provide the same include filename, then changing includes in e.g. your .ino file, might end up choosing a different one of these multiple libraries. If so, any other files that included that file must now be recompiled, since they were previously compiled with the other library in the include path.

See below for a complete example.

Expected behavior

It might be possible to pick out specific files to be recompiled based on the contents of the .d files, but it's probably safer and easier to just recompile everything when the list of included libraries changes.

Environment

Additional context

Here's an example sketch with two libraries showing the problem: IncludeChanges.zip

All contents are also shown at the bottom of this issue, for easier review. This contains two libraries A and B, both of which define a Common.h that has a func function. One accepts an int, one accepts a char, making them source compatible, but binary incompatible (i.e. any code that is recompiled will work against both, but code compiled for one will not link against the other). The sketch simply includes either library in the .ino file and has a separate .cpp file that includes Common.h and calls func(1). As you can see below, changing from library A to B does not cause a recompile of test.cpp and breaks the build.

IncludeChanges$ cat IncludeChanges.ino
#include <A.h>

void loop() { };
IncludeChanges$ arduino-cli compile --libraries libraries -v | grep  previously.*sketch/

... Changed include using editor ...

IncludeChanges$ cat IncludeChanges.ino
#include <B.h>

void loop() { };
IncludeChanges$ arduino-cli compile --libraries libraries -v | grep  previously.*sketch/
Using previously compiled file: /tmp/arduino-sketch-E34A9943CF606AE3061B946D36F3A4EE/sketch/test.cpp.o
/tmp/ccaQVdvp.ltrans0.ltrans.o: In function `setup':
/tmp/arduino-sketch-E34A9943CF606AE3061B946D36F3A4EE/sketch/test.cpp:5: undefined reference to `func(int)'
collect2: error: ld returned 1 exit status
Error during build: exit status 1
IncludeChanges$ tree
.
├── IncludeChanges.ino
├── libraries
│   ├── A
│   │   ├── A.h
│   │   ├── Common.cpp
│   │   └── Common.h
│   └── B
│       ├── B.h
│       ├── Common.cpp
│       └── Common.h
├── sketch.json
└── test.cpp

3 directories, 9 files
IncludeChanges$ for f in $(find -type f); do echo $f; cat $f | sed 's/^/  /'; echo; done
./test.cpp
  #include <Arduino.h>
  #include <Common.h>

  void setup() {
      func(1);
  }

./libraries/A/Common.cpp
  void func(int) { }

./libraries/A/A.h

./libraries/A/Common.h
  void func(int);

./libraries/B/Common.cpp
  void func(char) { }

./libraries/B/B.h

./libraries/B/Common.h
  void func(char);

./IncludeChanges.ino
  #include <B.h>

  void loop() { };

./sketch.json
  {
    "cpu": {
      "fqbn": "arduino:avr:uno",
      "port": ""
    }
  }
ubidefeo commented 4 years ago

@cmaglie This happened to me while messing around with a TFT Display (ST7735), when some libraries were included but shared headers with other libraries I had installed. Once compiled with the wrong, old library (Adafruit_ST7735) I installed the new library to use (Adafruit_ST7735_and_ST7789) but the include was the same and it didn't pick up the new library until I deleted the build