Fix library install directories #2148
Conversation
|
Thx you for your contribution.
|
|
For Windows, |
|
Thx you for confirming! |
|
Otherwise, the unversioned and more or less openMVG specific IMHO longterm this stlplus should be replace with std::filesystem (C++17). |
| install(EXPORT openMVG-targets | ||
| FILE OpenMVGTargets.cmake | ||
| NAMESPACE OpenMVG:: | ||
| DESTINATION share/openMVG/cmake | ||
| DESTINATION ${CMAKE_INSTALL_LIBDIR}/openMVG/cmake |
There was a problem hiding this comment.
I would still keep the share here (see cmake doc https://cmake.org/cmake/help/book/mastering-cmake/chapter/Install.html)
There was a problem hiding this comment.
That does not mention specifics for install(EXPORT).
On the other hand, https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-search-procedure lists both lib* and share as searched paths (with various, identical pre- and postfixes).
/usr/share/ is reserved for architecture independent files, but the .cmake files contain architecture dependent paths.
There was a problem hiding this comment.
So lib/share would be an appropriate path from your knowledge. We can check the behavior on a local folder install and see if consistent with the doc https://github.com/openMVG/openMVG/blob/develop/BUILD.md#using-openmvg-as-a-third-party-library-dependency-with-cmake
There was a problem hiding this comment.
If you meant lib or share, then yes, but not lib/share (unless lib/ is actually part of the INSTALL_PREFIX).
(| in the link above is part of a regex and means "or").
But share is only allowed for architecture independent packages (e.g. header only libraries).
There was a problem hiding this comment.
Convinced this is correct and does not cause any breakage?
There was a problem hiding this comment.
In the documentation, <userprefix>/share/cmake should become <userprefix>/lib/cmake.
(In case CMAKE_INSTALL_PREFIX is not one of the standard system prefixes, e.g. /usr/ or /usr/local/, CMake defaults to just lib.)
stlplus is a fairly small helper library, and the used subset is even smaller, so it makes sense to link it statically to the openMVG libraries.
The OpenMVGTargets-<buildtype>.cmake contains paths based on CMAKE_INSTALL_LIBDIR, and thus should go to the same, architecture dependent directory. Remove the duplicate installation of the target files as `openMVG-targets*.cmake`.
|
@StefanBruens Thx you for your contribution. Merged! |
No description provided.