Switch to GNUInstallDirs [v2] #1424
Conversation
Distributions are given standard variables for already existing hooks. Multiarch libdirs is taken care of automagically. Raises minimum cmake version by a little.
CMAKE_INSTALL_FULL_LIBDIR contains absolute path with CMAKE_INSTALL_PREFIX
CMAKE_INSTALL_FULL_INCLUDEDIR contains absolute path with CMAKE_INSTALL_PREFIX
This changes the default doc installdir to DATAROOTDIR/doc/PROJECT_NAME
Should be ported to CMakePackageConfigHelpers in the future, but this will do for now.
These files are not typically installed by the build system. Instead, distributions may or may not manually add them to their packaging (wrt standard licenses, it makes no sense to duplicate them all over the FS). CHANGES has not existed since 2012.
As PROJECT_NAME = OPENJPEG the generated value for CMAKE_INSTALL_DOCDIR becomes $DATAROOTDIR/doc/OPENJPEG. Fix by lowercasing PROJECT_NAME immediately before including GNUInstallDirs.
|
cc @a17r @thesamesam |
|
@rouault ping, just to follow up on this, are you interested in this change? |
yes, but there are likely subtelies. In other projects where I'm involved with, it has been raised that sometimes CMAKE_INSTALL_{foo} is an absolute directory and not a relative directory (cf OSGeo/gdal#6004 or OSGeo/PROJ#3206), so I suspect that in your pull request CMAKE_INSTALL_FULL_{foo} should be used, at least for .pc file generation |
Sure, I'll look into it - it would make sense, in Gentoo's patching to use GNUInstallDirs the values for Would you prefer that |
yes, my understanding is that it is the correct way |
OK, thanks for pointing it out. I'll make those changes (tomorrow probably, getting late here). |
actually taking inspiration from OSGeo/PROJ@ab25e4b which tests if CMAKE_INSTALL_{foo} is relative or not might better. The idea is to still use ${prefix}/${CMAKE_INSTALL_{foo}} when it is relative, so that packages can be relocated |
Thanks @rouault - this is primarily for .pc file generation, right? |
|
The reason I ask is that I'm considering one of two approaches:
I'm leaning towards the latter approach as it's less work. But I'd like to know which you prefer. |
I'm not completely sure to have understood your first option. But IMHO ${libdir} shouldn't be used outside of .pc generation. ${CMAKE_INSTALL_LIBDIR} is idiomatic in all other places such as install() CMake function. |
Excellent, thanks! I'll go with that then. |
In some cases the CMAKE_INSTAL_{BIN,MAN,DOC,LIB,INCLUDE}DIR variables
may turn out to be absolute paths in which case prepending ${prefix} in
the pkg-config .pc files will result in incorrect values.
For .pc file generation, figure out if these variables are absolute and
omit the prefix in the configured file when so.
See: OSGeo/PROJ@ab25e4b
|
@rouault Sorry for taking a while to get back to this, I've implemented it now, let me know what you think. I had to use |
|
@rouault Gentle ping on this, I think I've implemented what you requested. |
|
merged. thanks! |
|
Yay! Thank you! |
|
This broke cross-compilation of downstream projects (poppler) for me, I've submitted #1439 to fix this. |
|
How does this break cross-compilation? |
I am installing using CMAKE_INSTALL_PREFIX=/usr/local and DESTDIR=/path/to/my/sysroot. Without the relocatable change, downstream projects will add |
|
That is correct, which is why cross-compilation generally expects |
Yes, I set all of those, and that works fine for almost all projects (the ones that don't include those paths in the Config.cmake file, and instead use |
|
wouldn't it then make more sense to move to modern CMake where the user has to use |
|
This did not replace all usages of openjpeg/src/lib/openjp2/CMakeLists.txt Line 108 in 2d60670 |
|
Please file a new bug then. |
| if(NOT OPENJPEG_INSTALL_INCLUDE_DIR) | ||
| set(OPENJPEG_INSTALL_INCLUDE_DIR "include/${OPENJPEG_INSTALL_SUBDIR}") | ||
| endif() |
There was a problem hiding this comment.
This change removes the ability to customize the include dir location in v2.5.1. This affects the vcpkg port. (I wouldn't care, but I learned that Visual Studio users feel less comfortable with versioned include paths if not using cmake.)
There was a problem hiding this comment.
why? just pass -DCMAKE_INSTALL_INCLUDEDIR=foo to cmake?
There was a problem hiding this comment.
This would change the prefix (include -> foo) but not the suffix (OPENJPEG_INSTALL_SUBDIR) because it hardcoded everywhere.
For the vcpkg port I decided to not add a patch (i.e. openjpeg decides), but write two forwarding headers into vcpkg's old location, marked as "legacy".
So probably no further change needed here - but a breaking change nevertheless.
There was a problem hiding this comment.
@dg0yt If you want to contribute a pull request to fix that, that's a good moment to do because I'll likely have to issue a 2.5.2 because of #1514 (comment)
Rebase of #1165 on the current master, plus an attempt at moving docs
$DATAROOTDIR/doc/{OPENJPEG -> openjpeg}. I considered alternatively to mangle CMAKE_INSTALL_DOCDIR directly, but I think that could cause other problems.