JIC-CSB / kilombo

Kilobot simulator in C
Other
44 stars 16 forks source link

Not installing on Ubuntu 16.04 #35

Closed ccimrie closed 6 years ago

ccimrie commented 8 years ago

I'm trying to install Kilombo and when running the command sudo make install I get these errors:

/usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/libcheck.a(check_log.o): In function `subunit_lfun':
(.text+0x584): undefined reference to `subunit_test_start'
/usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/libcheck.a(check_log.o): In function `subunit_lfun':
(.text+0x63f): undefined reference to `subunit_test_fail'
/usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/libcheck.a(check_log.o): In function `subunit_lfun':
(.text+0x654): undefined reference to `subunit_test_pass'
/usr/lib/gcc/x86_64-linux-gnu/5/../../../x86_64-linux-gnu/libcheck.a(check_log.o): In function `subunit_lfun':
(.text+0x66f): undefined reference to `subunit_test_error'
collect2: error: ld returned 1 exit status
src/tests/CMakeFiles/check_skilobot.dir/build.make:172: recipe for target 'src/tests/check_skilobot' failed
make[2]: *** [src/tests/check_skilobot] Error 1
CMakeFiles/Makefile2:181: recipe for target 'src/tests/CMakeFiles/check_skilobot.dir/all' failed
make[1]: *** [src/tests/CMakeFiles/check_skilobot.dir/all] Error 2
Makefile:138: recipe for target 'all' failed
make: *** [all] Error 2

Not sure if I'm doing something silly or if I'm missing something obvious.

fjansson commented 8 years ago

Thank you for reporting this. We can reproduce the problem. Apparently something changed in the libcheck shipped with Ubuntu 16.04, in the linking flags needed.

https://github.com/libcheck/check/issues/60

AmayaSasaki commented 6 years ago

Hi, I get the same problem when running sudo make install on Debian 4.9.65-3+deb9u2. What should I do?

fjansson commented 6 years ago

How about not building the tests by default? The problem is that libcheck, which we use for tests was changed and this broke the build. We could update our build scripts but then it would probably break for those still having the old version of the library.

My first hunch would be to remove enable_testing() from CMakelists.txt, maybe it builds then. Please let us know if this works.

fjansson commented 6 years ago

There's also pull request https://github.com/JIC-CSB/kilombo/pull/40 which should fix this - We didn't apply it generally since is looks like it would break the build for others.

AmayaSasaki commented 6 years ago

Thanks. However I removed enable_testing() from CMakelists.txt but still get the same error...

Adam01010101F commented 6 years ago

Same here. I am on Ubuntu 17.10

(.text+0x5f4): undefined reference to subunit_test_start' (.text+0x6bf): undefined reference tosubunit_test_fail' (.text+0x6d4): undefined reference to subunit_test_pass' (.text+0x6ef): undefined reference tosubunit_test_error' collect2: error: ld returned 1 exit status src/tests/CMakeFiles/check_skilobot.dir/build.make:172: recipe for target 'src/tests/check_skilobot' failed make[2]: [src/tests/check_skilobot] Error 1 CMakeFiles/Makefile2:181: recipe for target 'src/tests/CMakeFiles/check_skilobot.dir/all' failed make[1]: [src/tests/CMakeFiles/check_skilobot.dir/all] Error 2 Makefile:129: recipe for target 'all' failed make: *** [all] Error 2

Is what I am getting. I have tried removing enable_testing() from the Cmake but that doesn't help and I've also created the Cmake from the pull request #40. Neither of them have helped. I still get errors.

fjansson commented 6 years ago

New attempt for a work-around: edit src/CMakeLists.txt, comment out the final line add_subdirectory(tests).

@Adam01010101F some questions in order to solve this more properly: when you tried the pull request, did you get the same errors or different ones? With the pull request, you need to have the subunit library installed - do you have it? If not, can you install it and try again?

@tjelvar-olsson , @mrmh2 : I think the problem is that on Ubuntu, using check also requires the subunit library, -lsubunit. I think we don't want to enable this flag in general, since it's not required on all systems, and then we would add an extra dependeny. Someone else made cmake search for subunit and add the flag if present: https://github.com/leo-project/libcutil/pull/4 . Would you have time to try this?

Adam01010101F commented 6 years ago

@fjansson The workaround seemed to work. In response to the changes I did last time, the errors remained the same. I only cloned the base project on master, so I am assuming that it wasn't included? I am a noob when it comes to cMake, how would I go about making sure it is installed?

fjansson commented 6 years ago

Subunit is a separate library. It is not included in kilombo, but can be installed with the ubuntu package manager, probably like this (this installs check too, if not already there):

sudo aptitude install subunit check
tjelvar-olsson commented 6 years ago

I have pushed a potential fix to a branch: https://github.com/JIC-CSB/kilombo/tree/issue35-not-installing-on-ubuntu-16.04

I have tested it on ubuntu-14.04 and ubuntu-16.04

@fjansson perhaps you could review it and merge if you think that it is good enough?

Basically it adds libsubunit-dev as a dependancy on Debian based systems. This allows us to add subunit as a target link library to fix the issue in 16.04, whilst being backwards compatible with 14.04.

AmayaSasaki commented 6 years ago

It works fine for me (Debian 4.9). Thanks.

fjansson commented 6 years ago

Looks good to me. I still want to test on OSX and Arch linux, where subunit previously wasn't needed, just to see what cmake does when target_link_libraries adds a library which isn't installed.

On fedora the new branch works. Subunit-devel is a dependency of check-devel, so subunit is installed automatically. I'll add the fedora commands to the readme too.

fjansson commented 6 years ago

It appears the patch would make subunit a requirement on all systems. I'd like to avoid this.

I just committed a different construction, which tries to find subunit, and adds it to the library list only if it was found. So on systems where subunit isn't required by check (like Arch Linux) it is now not a dependency. If someone could test this on Ubuntu, and it works also there, I think we can merge into master.

The subunit-finding script is from here .

tjelvar-olsson commented 6 years ago

So should this bug be marked as fixed with fc742d6?

fjansson commented 6 years ago

Yes, fixed with fc742d6 . Thanks all for reporting and testing!