Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit Tests Are Dynamically Linking Against Installed Libraries Rather Than The Newest Libraries #4556

Closed
liangfok opened this issue Dec 20, 2016 · 16 comments

Comments

@liangfok
Copy link
Contributor

liangfok commented Dec 20, 2016

Problem Definition

When running a unit test, we often want it to dynamically linked against the latest version of Drake's libraries (located in drake-distro/build/drake/...) as opposed to the installed version (located in drake-distro/build/install/...) since the installed versions are typically older. A common use case is we want to instrument the Device Under Test (DUT) with temporary debug print statements. By linking against the installed library rather than the most recent library, these print statements will not show up, resulting in confusion.

How to Replicate the Problem

The following was tested on an Ubuntu 14.04 system.

Starting from a clean build environment, execute the following:

cd drake-distro
mkdir build; cd build
cmake .. -G Ninja -DCMAKE_BUILD_TYPE:STRING=Debug 
ninja
cd drake
ctest -VV -R package_map_test

You should see that the unit test package_map_test successfully ran.

Next, open drake-distro/drake/multibody/parsers/package_map.cc and modify method Contains() to be:

bool PackageMap::Contains(const string& package_name) const {
  drake::log()->info("Checking package_name {}.", package_name);
  return map_.find(package_name) != map_.end();
}

Note the newly added log statement, which results in a message being printed to the console.

Save and exit your text editor, then execute the following commands:

cd drake-distro/build/drake
ninja
ctest -VV -R package_map_test

You should see that the modification did not apply (nothing new is printed to the screen). This is presumably because the executable linked against the old (installed) version of the library containing PackageMap (drake-distro/build/install/lib/libdrakeMultibodyParsers.so) instead of the newest non-installed version (drake-distro/build/drake/multibody/parsers/libdrakeMultibodyParsers.so).

To resolve the problem, we need to build at the super-build level:

cd drake-distro/build
ninja
cd drake
ctest -VV -R package_map_test

You should now see that the newly added print statement results in a bunch of text being printed to the console.

@jamiesnape
Copy link
Contributor

Aside: We are almost at a stage where an install should not be required. That would be punting on this issue, of course.

@david-german-tri
Copy link
Contributor

@liangfok The instructions do not guarantee correctness for any build other than the superbuild, so I'm going to downgrade this to medium priority. That said, I agree it's annoying and I don't totally understand why it's happening.

Aside: We are almost at a stage where an install should not be required. That would be punting on this issue, of course.

I'm absolutely on board with removing the install step, and considering that a good-enough resolution. What are the remaining obstacles?

@jamiesnape
Copy link
Contributor

jamiesnape commented Dec 21, 2016

Removing pods.cmake and SWIG, I think. The removing pods.cmake is mostly done (I have a POD-free branch locally). The SWIG part, I have not thought about yet. I doubt it would be difficult.

@david-german-tri
Copy link
Contributor

Right, I believe SWIG belongs to @mwoehlke-kitware on #4486.

@jamiesnape
Copy link
Contributor

Yes, though I think MATLAB would probably have to stay with SWIG and the logic changed not to require an install.

@jamiesnape
Copy link
Contributor

The only caveat, is that for now Director may need an install of Drake. That can be fixed.

@david-german-tri
Copy link
Contributor

Actually, I can't reproduce the original problem. My ninja build is linking the package_map_test against multibody/parsers/libdrakeMultibodyParsers.so, not the one in install/lib.

That said, we've had all sorts of non-hermetic install related nightmares in the past, so this is probably another of those, and all the more reason to burn it.

@liangfok
Copy link
Contributor Author

liangfok commented Dec 21, 2016

After a F2F meeting with @david-german-tri, using ninja -v in drake-distro/build/drake, we isolated the problem to be in the build command's rpath argument.

Here is @david-german-tri's command:

[9/82] : && /usr/bin/clang++-3.9   -Werror=all -Werror=ignored-qualifiers -Werror=overloaded-virtual -Werror=shadow -Werror=inconsistent-missing-override -Werror=sign-compare -DGTEST_DONT_DEFINE_FAIL=1 -DGTEST_DONT_DEFINE_SUCCEED=1 -DGTEST_DONT_DEFINE_TEST=1 -g  -Wl,--no-undefined multibody/parsers/test/package_map_test/CMakeFiles/package_map_test.dir/package_map_test.cc.o  -o multibody/parsers/test/package_map_test/package_map_test  /home/dgerman/drake/drake-distro/build/install/lib/libgtest.so /home/dgerman/drake/drake-distro/build/install/lib/libgtest_main.so multibody/parsers/libdrakeMultibodyParsers.so /home/dgerman/drake/drake-distro/build/install/lib/libgtest.so multibody/libdrakeRBM.so multibody/collision/libdrakeCollision.so /home/dgerman/drake/drake-distro/build/install/lib/libBulletDynamics.so /home/dgerman/drake/drake-distro/build/install/lib/libBulletCollision.so /home/dgerman/drake/drake-distro/build/install/lib/libLinearMath.so /home/dgerman/drake/drake-distro/build/install/lib/libBulletSoftBody.so multibody/shapes/libdrakeShapes.so multibody/joints/libdrakeJoints.so util/libdrakeGeometryUtil.so math/libdrakeMath.so common/libdrakeCommon.so /home/dgerman/drake/drake-distro/build/install/lib/libgflags.so.2.2.0 -lpthread thirdParty/bsd/spruce/libspruce.so thirdParty/zlib/tinyxml2/libtinyxml2.so -Wl,-rpath,/home/dgerman/drake/drake-distro/build/drake/multibody/parsers:/home/dgerman/drake/drake-distro/build/drake/multibody:/home/dgerman/drake/drake-distro/build/drake/multibody/collision:/home/dgerman/drake/drake-distro/build/drake/multibody/shapes:/home/dgerman/drake/drake-distro/build/drake/multibody/joints:/home/dgerman/drake/drake-distro/build/drake/util:/home/dgerman/drake/drake-distro/build/drake/math:/home/dgerman/drake/drake-distro/build/drake/common:/home/dgerman/drake/drake-distro/build/drake/thirdParty/bsd/spruce:/home/dgerman/drake/drake-distro/build/drake/thirdParty/zlib/tinyxml2:/home/dgerman/drake/drake-distro/build/install/lib && :

Here is my command:

[11/76] : && /usr/bin/clang++-3.9   -Werror=all -Werror=ignored-qualifiers -Werror=overloaded-virtual -Werror=shadow -Werror=inconsistent-missing-override -Werror=sign-compare -DGTEST_DONT_DEFINE_FAIL=1 -DGTEST_DONT_DEFINE_SUCCEED=1 -DGTEST_DONT_DEFINE_TEST=1 -g  -Wl,--no-undefined multibody/parsers/test/package_map_test/CMakeFiles/package_map_test.dir/package_map_test.cc.o  -o multibody/parsers/test/package_map_test/package_map_test  /home/liang/dev/drake-distro-4/build/install/lib/libgtest.so /home/liang/dev/drake-distro-4/build/install/lib/libgtest_main.so multibody/parsers/libdrakeMultibodyParsers.so /home/liang/dev/drake-distro-4/build/install/lib/libgtest.so multibody/libdrakeRBM.so multibody/collision/libdrakeCollision.so /home/liang/dev/drake-distro-4/build/install/lib/libBulletDynamics.so /home/liang/dev/drake-distro-4/build/install/lib/libBulletCollision.so /home/liang/dev/drake-distro-4/build/install/lib/libLinearMath.so /home/liang/dev/drake-distro-4/build/install/lib/libBulletSoftBody.so multibody/shapes/libdrakeShapes.so multibody/joints/libdrakeJoints.so util/libdrakeGeometryUtil.so math/libdrakeMath.so common/libdrakeCommon.so /home/liang/dev/drake-distro-4/build/install/lib/libgflags.so.2.2.0 -lpthread thirdParty/bsd/spruce/libspruce.so thirdParty/zlib/tinyxml2/libtinyxml2.so -Wl,-rpath,/home/liang/dev/drake-distro-4/build/install/lib:/home/liang/dev/drake-distro-4/build/drake/multibody/parsers:/home/liang/dev/drake-distro-4/build/drake/multibody:/home/liang/dev/drake-distro-4/build/drake/multibody/collision:/home/liang/dev/drake-distro-4/build/drake/multibody/shapes:/home/liang/dev/drake-distro-4/build/drake/multibody/joints:/home/liang/dev/drake-distro-4/build/drake/util:/home/liang/dev/drake-distro-4/build/drake/math:/home/liang/dev/drake-distro-4/build/drake/common:/home/liang/dev/drake-distro-4/build/drake/thirdParty/bsd/spruce:/home/liang/dev/drake-distro-4/build/drake/thirdParty/zlib/tinyxml2 && :

Using meld, we can see the differences:

screenshot from 2016-12-21 14 13 25

Basically, the problem is on my computer, /home/liang/dev/drake-distro/build/install/lib appears first in the -rpath argument list whereas on @david-german-tri's computer it appears last.

@david-german-tri
Copy link
Contributor

^ Which is some kind of super weird CMake behavior, I guess?

@jamiesnape
Copy link
Contributor

CMake is ultimately responsible, but the RPATH is apparently being "abused" by PODs.

@jwnimmer-tri
Copy link
Collaborator

With #2482 closed, I wonder if this issue has an updated disposition?

@liangfok
Copy link
Contributor Author

liangfok commented Feb 8, 2017

I will retest tomorrow some time over the next week using my workstation in CAM2, which was used to replicate the test described above.

@liangfok
Copy link
Contributor Author

I retested this on my Ubuntu 14.04 workstation using cf497e8 and was able to replicate the issue. This issue shall thus remain open. 😭

@david-german-tri
Copy link
Contributor

@jamiesnape, I've reassigned this to you, and downgraded the priority to "low" since it's CMake-specific.

@jamiesnape
Copy link
Contributor

@liangfok Now that we use Bazel to build Drake, is this still an issue?

@EricCousineau-TRI
Copy link
Contributor

Closing, as this no longer seems relevant per Jamie's comment.

@jamiesnape jamiesnape removed their assignment Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants