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

rename libmetis -> libmetis-gtsam to avoid collision with system libs #332

Merged
merged 1 commit into from
Jun 2, 2020

Conversation

berndpfrommer
Copy link
Collaborator

@berndpfrommer berndpfrommer commented Jun 1, 2020

At the moment GTSAM builds its own private libmetis.so from the sources in gtsam/3rdparty/metis. This creates a single-architecture libmetis.so file without ABI number, which breaks the building of a multi-architecture Debian package because GTSAM's libmetis.so file collides with the one from the standard libmetis5 Debian package.

The best fix is probably to first make use of 3rdparty/metis optional (see #309 and #292), as was done already for 3rdparty/Eigen, and ultimately directly rely on libmetis5-dev for development of GTSAM.

Until such more elaborate fixes are in place, this PR renames the 3rdparty libmetis to libmetis-gtsam to make Debian/Ubuntu multi-arch builds possible, and avoid accidentally compiling against headers from 3rdparty/metis, but linking against libmetis.so from libmetis5.

Referencing @jlblancoc for review.


This change is Reviewable

@jlblancoc jlblancoc self-requested a review June 2, 2020 08:13
Copy link
Member

@jlblancoc jlblancoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Will probably need to be iterated after testing: we need to test building a gtsam user program built agains the .deb produced by this, to check that none of the cmake exported targets are missing, etc.

In fact, it would be great to add a new Unit Test consisting of invoking cmake to build a simple program using the just-generated gtsam*-config.cmake + gtsam*-targets.cmake, etc. I'll file a separate wish-list issue ticket for it.

@jlblancoc jlblancoc merged commit 65da699 into borglab:develop Jun 2, 2020
@acxz acxz mentioned this pull request Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants