Not installing the libgtsam-unstable-dev package causes build failures

[mantkiew]

Description

On Ubuntu, when a user installs only libgtsam-dev package and then tries to use the library using CMake, the following error is produced:

CMake Error at /usr/lib/cmake/GTSAM/GTSAM-exports.cmake:100 (message):
  The imported target "gtsam_unstable" references the file
     "/usr/lib/x86_64-linux-gnu/libgtsam_unstable.so.4.0.3"
  but this file does not exist.  Possible reasons include:
  * The file was deleted, renamed, or moved to another location.
  * An install or uninstall procedure did not complete successfully.
  * The installation package was faulty and contained
     "/usr/lib/cmake/GTSAM/GTSAM-exports.cmake"
  but not all the files it references.
Call Stack (most recent call first):
  /usr/lib/cmake/GTSAM/GTSAMConfig.cmake:25 (include)
  CMakeLists.txt:54 (FIND_PACKAGE)

That is, the cmake file references libgtsam_unstable.so.4.0.3 file which belongs to the libgtsam-unstable-dev package.

Consequently, it is not possible to only install libgtsam-dev without installing libgtsam-unstable-dev.

Steps to reproduce

1. apt-get install libgtsam-dev
2. find the library in CMakeLists.txt as usual
3. build and it'll fail with the abovementioned error.

Expected behavior

When the user installs only libgtsam-dev package, the CMake files installed should not refer to the files from the libgtsam-unstable-dev package.

Environment

Ubuntu 18.04 Bionic

[mantkiew]

Maybe this could be added to #824

[ProfFan]

Pinging @jlblancoc who is the expert on packaging.

[varunagrawal]

@mantkiew can you try setting the CMake flag GTSAM_BUILD_UNSTABLE=OFF? It is ON by default which is the reason you're encountering this issue.

[mantkiew]

Thank you, I'll try that. However, I suggest making this option to be OFF by default if the unstable package is not required. Otherwise, the option can stay ON by default if the unstable package becomes installed automatically.

[jlblancoc]

It seems a real CMake exports bug:

CMake Error at /usr/lib/cmake/GTSAM/GTSAM-exports.cmake:100 (message):

There should be two export files, one for gtsam and another for gtsam_unstable.

I have no time for full testing now, but I think the error is the repeated EXPORT GTSAM-exports in both:

• https://github.com/borglab/gtsam/blob/develop/gtsam/CMakeLists.txt#L177-L182
• and https://github.com/borglab/gtsam/blob/develop/gtsam_unstable/CMakeLists.txt#L101-L106

The latter should be EXPORT gtsam-unstable-exports, but probably other minor changes should be made around for consistency...

[varunagrawal]

Finally got down to examining this in detail. @jlblancoc's findings are spot-on, though I am actually wondering if we should have two versions of the GTSAM package, one with GTSAM_BUILD_UNSTABLE=OFF so that gtsam-unstable is not packaged with it and is not included in the CMake exports, and another one with GTSAM_BUILD_UNSTABLE=ON. That way, depending on what package the user installs, they get the files they need without having to change their cmake.

@berndpfrommer thoughts? Should be pretty straightforward I believe?

[varunagrawal]

That being said, I played around with the CMake to get a different export file working called GTSAM_UNSTABLE-exports.cmake, but this will require an explicit find_package(GTSAM_UNSTABLE), which isn't as clean as my prior suggestion IMVHO.

[berndpfrommer]

From what I understand we currently have, due to what looks like a cmake bug, the following actual dependency:
libgtsam-dev depends on libgtsam-unstable
which is not reflected in the packaging scheme.

Yes, I can make two different source packages, one with GTSAM_BUILD_UNSTABLE=ON, the other =OFF, and then have one provide the unstable only, the other the stable only. Getting the packaging to work can be quite tedious though (this will probably take me a full day to get working) and it will make an even greater mess out of an already complicated build process.

I'm also not too keen on doing that because it will now require maintaining at least 3, maybe 4 different packaging repos (with TBB/without TBB) * (with unstable/without unstable). I know of no good way to maintain these 3 or 4 source packages in parallel. I'm also not sure how kindly the Ubuntu folks will look at such work-arounds when we ask them to sponsor gtsam as an official ubuntu package.

So sorry, from a packaging perspective I need to push back.

Alternatively, I could add a package dependency: [libgtsam-dev depends on libgtsam-unstable] until the proper cmake fix is available.

[varunagrawal]

@berndpfrommer that's a fair point, thanks for your comments. 🙂 Can you please wait on the package dependency part? I have the changes ready and will push them first thing in the morning so that you won't have to create and then undo the dependency.

[berndpfrommer]

BTW, under what scenario do you have to do find_package(GTSAM_UNSTABLE)? If it's only when you are actually using libgtsam_unstable, then that's perfectly fine, and in fact the expected behavior.
Except that I'm not sure what the proper convention is. Should it be find_package(GTSAM_UNSTABLE) or find_package(gtsam_unstable) or find_package(GTSAM_unstable)? I think lowercase might be more conventional, i.e. GTSAM_unstable, accepting that GTSAM is an acronym and as such is capitalized, but "unstable" is not.

[varunagrawal]

@berndpfrommer for some reason I can't add you as a reviewer, but can you please take a look at #906?

[berndpfrommer]

OK, will do so Monday. I guess I should test that this PR indeed removes the dependencies such that libgtsam-dev can be used without libgtsam-unstable.

[varunagrawal]

BTW, under what scenario do you have to do find_package(GTSAM_UNSTABLE)? If it's only when you are actually using libgtsam_unstable, then that's perfectly fine, and in fact the expected behavior.

Yup, this should be the expected behavior as per my changes.

Except that I'm not sure what the proper convention is. Should it be find_package(GTSAM_UNSTABLE) or find_package(gtsam_unstable) or find_package(GTSAM_unstable)? I think lowercase might be more conventional, i.e. GTSAM_unstable, accepting that GTSAM is an acronym and as such is capitalized, but "unstable" is not.

I've currently set it GTSAM_UNSTABLE to be consistent with the current GTSAM convention. I believe there is an open issue on CMake to make this case insensitive.