Multiple definition of 'main' when including a library with tests

[rosogon]

System

• OS: Linux
• ruby -v: ruby 2.3.1p112
• bundle -v: Bundler version 2.1.4
• bundle info arduino_ci: arduino_ci (0.3.0)
• g++ -v: gcc version 5.4.0 20160609
• URL of your Arduino project: https://github.com/rosogon/arduino_ci_two_libraries

Issue Summary

If libB depends on libA, both with arduino_ci tests, trying to run the unit tests on libB will raise the error "multiple definition of main". This happens because cpp_files_libraries returns the whole tree of the referenced library (libA in the example), not removing the files in test/.

Con: Skipping those files can break libraries containing files in test/ (which are needed to use the library). Weird, but could happen.

Arduino or Unit Test Code, Illustrating the Problem

The following repo illustratres the example: https://github.com/rosogon/arduino_ci_two_libraries

libB $ bundle exec arduino_ci_remote.rb --skip-compilation
Located Arduino binary...                     /home/roman/arduino_ci_ide/arduino
[...]
Requested unittest platform 'uno' is defined in 'platforms' YML...             ✓
Using pre-existing library...                                               libA
Unit testing test.cpp with g++... 

Last command:  $ g++ -std=c++0x -o /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/libB/unittest_test.cpp.bin -DARDUINO=100 -g -O1 -fno-omit-frame-pointer -fno-optimize-sibling-calls -fsanitize=address -D__AVR_ATmega328P__ -I/home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/gems/arduino_ci-0.3.0/cpp/arduino -I/home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/gems/arduino_ci-0.3.0/cpp/unittest -I/home/roman/projects/Arduino/libraries/libA /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/gems/arduino_ci-0.3.0/cpp/arduino/Arduino.cpp /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/gems/arduino_ci-0.3.0/cpp/arduino/Godmode.cpp /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/gems/arduino_ci-0.3.0/cpp/arduino/stdlib.cpp /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/gems/arduino_ci-0.3.0/cpp/unittest/ArduinoUnitTests.cpp /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/libA/test/test.cpp /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/libB/test/test.cpp                                                                                                                         

/tmp/ccFO4mjB.o: In function `std::basic_ostream<char, std::char_traits<char> >& std::flush<char, std::char_traits<char> >(std::basic_ostream<char, std::char_traits<char> >&)':
/home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/gems/arduino_ci-0.3.0/cpp/unittest/ArduinoUnitTests.h:149: multiple definition of `main'
/tmp/ccudL6dM.o:/home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/libA/test/test.cpp:10: first defined here
collect2: error: ld returned 1 exit status
...Unit testing test.cpp with g++                                              ✗
Skipping compilation of examples...                as requested via command line
Failures: 1

[rosogon]

I have modified it like this: rosogon@640201a

If you agree with the approach, I can prepare tests and PR.

[ianfixes]

Hi rosogon, thanks for contributing this! And I'm sorry to hear about this limitation of the library, that's really annoying.

At the moment I'm torn -- I have a 2+ year old feature request for a test/ directory to be officially adopted by the Arduino library specification. I know this is pedantic of me, but I'm hesitant to hard-code a directory to exclude from the compiler (even one that conforms to a convention that I wholeheartedly support) because I can't guarantee that no arduino library anywhere might be using that directory for actual code.

Really there are 2 choices here: 1 is whether there is going to be some official movement toward test/ being "official" in the near future (I just created #169 to track that). If so, I will absolutely adopt the change you suggest. The second-best option would be more cumbersome... adding settings to .arduinoci.yml that allow you to hand-code selection/rejection criteria for each of the dependencies:

• https://github.com/Arduino-CI/arduino_ci/blob/master/REFERENCE.md#control-how-unit-tests-are-compiled-and-run
• https://github.com/Arduino-CI/arduino_ci/blob/master/lib/arduino_ci/ci_config.rb#L301

I saw that @per1234 happened to make some moves on this in the past few days, I'm not sure if there is extra information there.

[rosogon]

Hi @ianfixes ,

Thanks for your detailed answer. I understand your concerns.

While we wait until that test/ is official (let's hope it is), in rosogon@5b0b5bc you have another approach using the exclude_dirs field to reject directories in the included libraries. Another field could be used (exclude_library_dirs?) to make it safer. Let me know your opinion.

[ianfixes]

Yes exactly -- this is the model I would use for the fallback solution. It might make sense to just institute that functionality now, but the question is of how best to do it. What I'm leaning toward right now is a more robust handling of the "libraries" section, from a simple array to a more ambitious hash.

In other words, not only do I think it's important to add what you're suggesting, I think that there are a lot of things that could be improved by configuring the individual libraries. And it would be better to tie that to the rest of the library config instead of the compiler specifically.

If you were looking for a quick fix, I'm sorry to be taking you down this rabbit hole.

[matthijskooijman]

I know this is pedantic of me, but I'm hesitant to hard-code a directory to exclude from the compiler (even one that conforms to a convention that I wholeheartedly support) because I can't guarantee that no arduino library anywhere might be using that directory for actual code.

I actually think that you might be compiling more code than the Arduino IDE currently. Arduino libraries either have their source files contained in the src folder (which is compiled recursively) for modern libraries, or inside the root and utility subfolder (which are compiled non-recursively) for legacy libraries. So I think this is something ArduinoCI could (and should) just match, and which would solve the problem with a test subfolder AFAIU?

See also the Arduino library spec and this issue I filed about unclarity about legacy vs new-style libraries.

[rosogon]

If you were looking for a quick fix, I'm sorry to be taking you down this rabbit hole.

No worries. I have the quick fix running locally :)

I agree that the best approach is what you propose, specially if the idea is to store more information. It was actually my first thought, but discarded because of not being backwards compatible.

Anyway, as @matthijskooijman's comments, there are library files being included to compile that shouldn't.

[ianfixes]

Thanks for filing that @matthijskooijman, I'm now subscribed. I'll implement the logic you suggest here in advance of getting an answer there.

[ianfixes]

Looks like I have the logic partially implemented already, just need to make it conditional: https://github.com/Arduino-CI/arduino_ci/blob/744365d/lib/arduino_ci/cpp_library.rb#L238-L253

[per1234]

You can see Arduino CLI's logic here:
https://github.com/arduino/arduino-cli/blob/master/arduino/libraries/loader.go
This also applies to Arduino IDE

So there are two library layouts:

• recursive (called the "1.5 format" in the specification)
• flat (called the "1.0 format" in the specification)

This is how layout is determined:

+--------------------+    +--------+
|                    | no |        |
| library.properties +--->+ legacy |
|                    |    |        |
+----------+---------+    +---+----+
           |                  |
           | yes              |
           v                  |
   +-------+------+           |
   |              |    no     |
   | "src" folder +---------->+
   |              |           |
   +-------+------+           |
           |                  |
           | yes              |
           v                  v
     +-----+-----+        +---+--+
     |           |        |      |
     | recursive |        | flat |
     |           |        |      |
     +-----------+        +------+

layout	root compiled	src compiled	utility compiled
recursive	no	recursively	no
flat	yes	no	yes

[matthijskooijman]

Thanks for the detailed answer, @per1234!

[rosogon]

Looks like I have the logic partially implemented already, just need to make it conditional: https://github.com/Arduino-CI/arduino_ci/blob/744365d/lib/arduino_ci/cpp_library.rb#L238-L253

Take into account that Find.find at cpp_files_in, recursively enters all directories. As one of the checked dirs is "", whole tree is actually returned. I guess cpp_files_in must receive a recursive parameter, so you can be recursive on /src but not on / (see Find.prune).

[ianfixes]

I'm taking a look at this now