ros-gbp / cartographer-release

release repository for google cartographer
1 stars 2 forks source link

Melodic cartographer package erroneously installs libgtest.a #8

Closed clalancette closed 6 years ago

clalancette commented 6 years ago

While doing some testing on Melodic, I noticed that if I have the ros-melodic-cartographer package installed, the tests for other packages seem to start failing. The one I noticed this on was urdf, but this may affect other tests as well.

Steps to reproduce:

  1. Setup a bionic machine/container with Melodic: http://wiki.ros.org/melodic/Installation/Ubuntu (note that installing ros-melodic-desktop won't work quite yet, but just install ros-melodic-*)
  2. Setup a catkin workspace with urdf in it (melodic-devel branch):
    $ mkdir -p ~/cart_bug/src
    $ cd ~/cart_bug/src
    $ git clone https://github.com/ros/urdf
    $ cd urdf
    $ git checkout melodic-devel
  3. Comment out temporary workaround line: https://github.com/ros/urdf/blob/melodic-devel/urdf/CMakeLists.txt#L64
  4. Build the workspace, including tests:
    $ cd ~/cart_bug
    $ . /opt/ros/melodic/setup.bash
    $ catkin_make
    $ catkin_make tests

In my build, this ends up with:

[ 44%] Linking CXX executable /home/ubuntu/cart-bug/devel/lib/urdf/urdfdom_compatibility_test
[ 66%] Built target test_model_parser_initxml
[ 88%] Built target test_urdf_parser
/opt/ros/melodic/lib/libgtest.a(gtest-all.cc.o): In function `testing::internal::UnitTestImpl::~UnitTestImpl()':
(.text+0xb65d): undefined reference to `pthread_getspecific'
(.text+0xb676): undefined reference to `pthread_key_delete'
(.text+0xb77a): undefined reference to `pthread_getspecific'
(.text+0xb793): undefined reference to `pthread_key_delete'
/opt/ros/melodic/lib/libgtest.a(gtest-all.cc.o): In function `testing::internal::UnitTestImpl::UnitTestImpl(testing::UnitTest*)':
(.text+0x103fe): undefined reference to `pthread_key_create'
(.text+0x105bb): undefined reference to `pthread_key_create'
/opt/ros/melodic/lib/libgtest.a(gtest-all.cc.o): In function `testing::internal::ThreadLocal<testing::TestPartResultReporterInterface*>::~ThreadLocal()':
(.text._ZN7testing8internal11ThreadLocalIPNS_31TestPartResultReporterInterfaceEED2Ev[_ZN7testing8internal11ThreadLocalIPNS_31TestPartResultReporterInterfaceEED5Ev]+0x1c): undefined reference to `pthread_getspecific'
(.text._ZN7testing8internal11ThreadLocalIPNS_31TestPartResultReporterInterfaceEED2Ev[_ZN7testing8internal11ThreadLocalIPNS_31TestPartResultReporterInterfaceEED5Ev]+0x31): undefined reference to `pthread_key_delete'
/opt/ros/melodic/lib/libgtest.a(gtest-all.cc.o): In function `testing::internal::ThreadLocal<std::vector<testing::internal::TraceInfo, std::allocator<testing::internal::TraceInfo> > >::~ThreadLocal()':
(.text._ZN7testing8internal11ThreadLocalISt6vectorINS0_9TraceInfoESaIS3_EEED2Ev[_ZN7testing8internal11ThreadLocalISt6vectorINS0_9TraceInfoESaIS3_EEED5Ev]+0x1c): undefined reference to `pthread_getspecific'
(.text._ZN7testing8internal11ThreadLocalISt6vectorINS0_9TraceInfoESaIS3_EEED2Ev[_ZN7testing8internal11ThreadLocalISt6vectorINS0_9TraceInfoESaIS3_EEED5Ev]+0x31): undefined reference to `pthread_key_delete'
/opt/ros/melodic/lib/libgtest.a(gtest-all.cc.o): In function `testing::internal::ThreadLocal<std::vector<testing::internal::TraceInfo, std::allocator<testing::internal::TraceInfo> > >::GetOrCreateValue() const':
(.text._ZNK7testing8internal11ThreadLocalISt6vectorINS0_9TraceInfoESaIS3_EEE16GetOrCreateValueEv[_ZNK7testing8internal11ThreadLocalISt6vectorINS0_9TraceInfoESaIS3_EEE16GetOrCreateValueEv]+0x1c): undefined reference to `pthread_getspecific'
(.text._ZNK7testing8internal11ThreadLocalISt6vectorINS0_9TraceInfoESaIS3_EEE16GetOrCreateValueEv[_ZNK7testing8internal11ThreadLocalISt6vectorINS0_9TraceInfoESaIS3_EEE16GetOrCreateValueEv]+0xa1): undefined reference to `pthread_setspecific'
/opt/ros/melodic/lib/libgtest.a(gtest-all.cc.o): In function `testing::internal::ThreadLocal<testing::TestPartResultReporterInterface*>::GetOrCreateValue() const':
(.text._ZNK7testing8internal11ThreadLocalIPNS_31TestPartResultReporterInterfaceEE16GetOrCreateValueEv[_ZNK7testing8internal11ThreadLocalIPNS_31TestPartResultReporterInterfaceEE16GetOrCreateValueEv]+0x1e): undefined reference to `pthread_getspecific'
(.text._ZNK7testing8internal11ThreadLocalIPNS_31TestPartResultReporterInterfaceEE16GetOrCreateValueEv[_ZNK7testing8internal11ThreadLocalIPNS_31TestPartResultReporterInterfaceEE16GetOrCreateValueEv]+0x93): undefined reference to `pthread_setspecific'
collect2: error: ld returned 1 exit status
urdf/urdf/CMakeFiles/urdfdom_compatibility_test.dir/build.make:95: recipe for target '/home/ubuntu/cart-bug/devel/lib/urdf/urdfdom_compatibility_test' failed
make[3]: *** [/home/ubuntu/cart-bug/devel/lib/urdf/urdfdom_compatibility_test] Error 1
CMakeFiles/Makefile2:1170: recipe for target 'urdf/urdf/CMakeFiles/urdfdom_compatibility_test.dir/all' failed
make[2]: *** [urdf/urdf/CMakeFiles/urdfdom_compatibility_test.dir/all] Error 2
CMakeFiles/Makefile2:76: recipe for target 'CMakeFiles/tests.dir/rule' failed
make[1]: *** [CMakeFiles/tests.dir/rule] Error 2
Makefile:175: recipe for target 'tests' failed
make: *** [tests] Error 2

Removing ros-melodic-cartographer and repeating the above allows it to succeed.

It looks to me like the ros-melodic-cartographer package is erroneously installing /opt/ros/melodic/lib/libgtest.a, /opt/ros/melodic/lib/libgtest_main.a, /opt/ros/melodic/lib/libgmock.a, and /opt/ros/melodic/lib/libgmock_main.a, and it seems like it probably should not do that.

mikaelarguedas commented 6 years ago

@clalancette Thanks for reporting this, this is definitely a bug. Looking at your workaround, it seems like the right thing to do permanently, regardless of this bug.

Can you provide a list of packages currently failing because of this? How urgent is the resolution of this? If this is not blocking other packages I may take the time to fix this properly, if it's urgent I may just disable testing completely like we did in ROS 2

clalancette commented 6 years ago

@clalancette Thanks for reporting this, this is definitely a bug. Looking at your workaround, it seems like the right thing to do permanently, regardless of this bug.

Yeah, definitely, you are right. That is the correct thing to do, we had just missed it previously.

Can you provide a list of packages currently failing because of this? How urgent is the resolution of this? If this is not blocking other packages I may take the time to fix this properly, if it's urgent I may just disable testing completely like we did in ROS 2

The only failure I saw was when building urdf locally before the fix in https://github.com/ros/urdf/blob/melodic-devel/urdf/CMakeLists.txt#L64 . Now that that is fixed, and the fact that ros-melodic-cartographer is not in the default install, I would say that it isn't urgent. So feel free to take your time and fix it properly.

mikaelarguedas commented 6 years ago

@k-okada @7675t would one of you have time to look into this in the near future? (I expect the same problem to affect lunar and kinetic)

k-okada commented 6 years ago

@mikaelarguedas Unfortunately, this only happens in melodic...

$ dpkg -c ros-kinetic-cartographer_0.2.0-1xenial-20180404-210540-0800_arm64.deb  | grep libg
$ dpkg -c ros-lunar-cartographer_0.2.0-4xenial-20180331-123156-0800_arm64.deb  | grep libg
$ dpkg -c ros-melodic-cartographer_0.3.0-0bionic.20180322.192123_amd64.deb  | grep libg-rw-r--r-- root/root    866224 2018-03-23 04:21 ./opt/ros/melodic/lib/libgmock.a
-rw-r--r-- root/root    868910 2018-03-23 04:21 ./opt/ros/melodic/lib/libgmock_main.a
-rw-r--r-- root/root    606282 2018-03-23 04:21 ./opt/ros/melodic/lib/libgtest.a
-rw-r--r-- root/root      2646 2018-03-23 04:21 ./opt/ros/melodic/lib/libgtest_main.a

@7675t how did you compile latest cartographer on your 16.04 machine?

mikaelarguedas commented 6 years ago

thanks @k-okada for looking into this.

Unfortunately it seems to be tied to the operating system more than the version of the released code.

"old" OS don;t install it: $ dpkg -c ros-lunar-cartographer_0.2.0-4xenial-20180331-122408-0800_amd64.deb | grep libg But more recent OS install these libraries on lunar and melodic (regardless of the cartographer version released, lunar has 0.2.0 and melodic has 0.3.0).

$ dpkg -c ros-lunar-cartographer_0.2.0-4stretch-20180331-122308-0800_amd64.deb | grep libg
-rw-r--r-- root/root    865198 2018-03-31 13:23 ./opt/ros/lunar/lib/libgmock.a
-rw-r--r-- root/root    867884 2018-03-31 13:23 ./opt/ros/lunar/lib/libgmock_main.a
-rw-r--r-- root/root    604120 2018-03-31 13:23 ./opt/ros/lunar/lib/libgtest.a
-rw-r--r-- root/root      2646 2018-03-31 13:23 ./opt/ros/lunar/lib/libgtest_main.a
$ dpkg -c ros-melodic-cartographer_0.3.0-0stretch.20180322.191943_amd64.deb | grep libg
-rw-r--r-- root/root    865198 2018-03-22 12:19 ./opt/ros/melodic/lib/libgmock.a
-rw-r--r-- root/root    867884 2018-03-22 12:19 ./opt/ros/melodic/lib/libgmock_main.a
-rw-r--r-- root/root    604120 2018-03-22 12:19 ./opt/ros/melodic/lib/libgtest.a
-rw-r--r-- root/root      2646 2018-03-22 12:19 ./opt/ros/melodic/lib/libgtest_main.a
mikaelarguedas commented 6 years ago

See potential fix here. I'll see what I can get merged upstream as this seems like a bug that should be fixed there as well

mikaelarguedas commented 6 years ago

PR submitted upstream to get feedback: https://github.com/googlecartographer/cartographer/pull/1041

mikaelarguedas commented 6 years ago

The fix has been approved upstream :tada: so I'm going to propagate it to the lunar and melodic patch folders

mikaelarguedas commented 6 years ago

Fixed in melodic (search for "Install configuration: "None"": http://build.ros.org/view/Mbin_uB64/job/Mbin_uB64__cartographer__ubuntu_bionic_amd64__binary/2/consoleFull

@clalancette FYI

clalancette commented 6 years ago

Fixed in melodic (search for "Install configuration: "None"": http://build.ros.org/view/Mbin_uB64/job/Mbin_uB64__cartographer__ubuntu_bionic_amd64__binary/2/consoleFull

Fantastic! I downloaded the package, installed it, and verified that it no longer installs libgtest* or libgmock*. I also did a quick test with my "failing" case from before, and all is good. Thanks a lot!