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

rosogon · 2020-10-03T18:06:38Z

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

The text was updated successfully, but these errors were encountered:

rosogon · 2020-10-03T22:08:54Z

I have modified it like this: rosogon@640201a

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

ianfixes · 2020-10-04T00:53:25Z

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:

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.

to avoid multiple definition of main if included library contains tests. Example of .arduino-ci.yaml: ``` unittest: libraries: - libA platforms: - uno exclude_dirs: - libA/test ``` See issue Arduino-CI#168

rosogon · 2020-10-04T15:57:52Z

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 · 2020-10-05T17:36:33Z

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 · 2020-10-06T06:20:54Z

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 · 2020-10-06T20:02:56Z

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 · 2020-10-07T01:34:28Z

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 · 2020-10-07T01:39:37Z

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 · 2020-10-07T04:09:03Z

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 · 2020-10-07T08:01:15Z

Thanks for the detailed answer, @per1234!

rosogon · 2020-10-07T20:27:30Z

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 · 2020-10-16T20:40:59Z

I'm taking a look at this now

ianfixes · 2020-10-19T03:37:01Z

@rosogon can you try my branch from #183 and see if this fixes the issue? You will need to update the dependency in your Gemfile as follows:

gem 'arduino_ci', git: 'https://github.com/ianfixes/arduino_ci.git', branch: '2020-10-16_suggestions'

rosogon · 2020-10-21T20:59:24Z

Hi @ianfixes . I get this error when running bundle exec arduino_ci_remote.rb --skip-compilation:

Requested unittest platform 'uno' is defined in 'platforms' YML...             ✓
Using pre-existing library...                                               libA
Unit testing test.cpp with g++... 
/home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/lib/arduino_ci/cpp_library.rb:291:in `block in arduino_library_src_dirs': undefined method `new' for #<ArduinoCI::CppLibrary:0x00000002ee0988> (NoMethodError)
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/lib/arduino_ci/cpp_library.rb:291:in `map'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/lib/arduino_ci/cpp_library.rb:291:in `arduino_library_src_dirs'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/lib/arduino_ci/cpp_library.rb:298:in `include_args'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/lib/arduino_ci/cpp_library.rb:345:in `test_args'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/lib/arduino_ci/cpp_library.rb:375:in `build_for_test_with_configuration'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/exe/arduino_ci_remote.rb:236:in `block (4 levels) in perform_unit_tests'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/exe/arduino_ci_remote.rb:101:in `perform_action'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/exe/arduino_ci_remote.rb:122:in `attempt_multiline'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/exe/arduino_ci_remote.rb:235:in `block (3 levels) in perform_unit_tests'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/exe/arduino_ci_remote.rb:234:in `each'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/exe/arduino_ci_remote.rb:234:in `block (2 levels) in perform_unit_tests'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/exe/arduino_ci_remote.rb:232:in `each'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/exe/arduino_ci_remote.rb:232:in `block in perform_unit_tests'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/exe/arduino_ci_remote.rb:231:in `each'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/exe/arduino_ci_remote.rb:231:in `perform_unit_tests'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bundler/gems/arduino_ci-6196faa19e2e/exe/arduino_ci_remote.rb:396:in `<top (required)>'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bin/arduino_ci_remote.rb:23:in `load'
        from /home/roman/romanfiles/projects/Arduino/arduino_ci_two_libraries/.bundle/ruby/2.3.0/bin/arduino_ci_remote.rb:23:in `<main>'

ianfixes · 2020-10-26T05:02:28Z

You're right.. and I meant to respond a few days ago that (1) the test code didn't exercise this the way I thought it would and (2) there's a small rabbit hole here with getting library specs to load recursively. And possibly an inconsistency (library name vs path on disk) in the current code.

But this is in progress.

ianfixes · 2020-10-27T02:20:05Z

OK, I think this is ready for you to give it another try. At any rate, I was able to confirm what I believe is the desired behavior via unit testing.

This fixes a host of issues you didn't raise, including being able to recursively bring in library dependencies by looking at library.properties. I expect this will work fine for local testing, but I haven't bubbled up the "give me all the libraries" functionality to the CI test scripts. That can be future work if this satisfies your local testing. Please let me know!

rosogon · 2020-11-02T01:30:21Z

Hi @ianfixes . Just tested and it works for my cases (both with 1.0 libraries). Thanks!

ianfixes added enhancement New feature or request unittest libs The assert / assure / unittest reporting apparatus is affected labels Oct 4, 2020

ianfixes mentioned this issue Oct 19, 2020

Suggested fixes #183

Merged

ianfixes mentioned this issue Oct 27, 2020

Do not compile tests from included libraries #188

Closed

ianfixes mentioned this issue Nov 4, 2020

Arduino CI seems to not handle dependencies between imported Arduino Libraries #192

Closed

ianfixes closed this as completed in #183 Nov 13, 2020

per1234 mentioned this issue Nov 16, 2020

Fix compile command stuck in a loop when including multiple custom libraries arduino/arduino-cli#1066

Merged

4 tasks

per1234 mentioned this issue Nov 30, 2020

Documentation: Unclear when a library is compiled as "new-style" or "legacy" arduino/arduino-cli#1013

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

rosogon commented Oct 3, 2020 •

edited

Loading

rosogon commented Oct 3, 2020

ianfixes commented Oct 4, 2020

rosogon commented Oct 4, 2020

ianfixes commented Oct 5, 2020

matthijskooijman commented Oct 6, 2020

rosogon commented Oct 6, 2020

ianfixes commented Oct 7, 2020

ianfixes commented Oct 7, 2020

per1234 commented Oct 7, 2020 •

edited

Loading

matthijskooijman commented Oct 7, 2020

rosogon commented Oct 7, 2020

ianfixes commented Oct 16, 2020

ianfixes commented Oct 19, 2020

rosogon commented Oct 21, 2020

ianfixes commented Oct 26, 2020

ianfixes commented Oct 27, 2020

rosogon commented Nov 2, 2020

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

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

Comments

rosogon commented Oct 3, 2020 • edited Loading

System

Issue Summary

Arduino or Unit Test Code, Illustrating the Problem

rosogon commented Oct 3, 2020

ianfixes commented Oct 4, 2020

rosogon commented Oct 4, 2020

ianfixes commented Oct 5, 2020

matthijskooijman commented Oct 6, 2020

rosogon commented Oct 6, 2020

ianfixes commented Oct 7, 2020

ianfixes commented Oct 7, 2020

per1234 commented Oct 7, 2020 • edited Loading

matthijskooijman commented Oct 7, 2020

rosogon commented Oct 7, 2020

ianfixes commented Oct 16, 2020

ianfixes commented Oct 19, 2020

rosogon commented Oct 21, 2020

ianfixes commented Oct 26, 2020

ianfixes commented Oct 27, 2020

rosogon commented Nov 2, 2020

rosogon commented Oct 3, 2020 •

edited

Loading

per1234 commented Oct 7, 2020 •

edited

Loading