Bug 1301720 - %cmake RPM macro is not compatible with GNUInstallDirs.cmake
%cmake RPM macro is not compatible with GNUInstallDirs.cmake
Status: NEW
Product: Fedora
Classification: Fedora
Component: cmake (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Orion Poplawski
Fedora Extras Quality Assurance
: FutureFeature
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2016-01-25 14:48 EST by Roman Tsisyk
Modified: 2017-07-14 10:12 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Roman Tsisyk 2016-01-25 14:48:40 EST
Description of problem:

CMake ships with a nice module called GNUInstallDirs.cmake.
This module automatically detects all systems directories in the similar way to autoconf. It has proper support of multilib suffixes and works well on Fedora, Debian, Arch, Gentoo and even FreeBSD. I personally use this module for all my projects.

https://cmake.org/cmake/help/v3.0/module/GNUInstallDirs.html

The problem is that %cmake RPM macro doesn't affect behavior of GNUInstallDirs.cmake:

%cmake RPM macro sets 
        -DINCLUDE_INSTALL_DIR:PATH=/usr/include \
        -DLIB_INSTALL_DIR:PATH=/usr/lib64 \
        -DSYSCONF_INSTALL_DIR:PATH=/etc \
        -DSHARE_INSTALL_PREFIX:PATH=/usr/share \
%if "lib64" == "lib64" 
        -DLIB_SUFFIX=64 \
%endif 

I have to initialize all CMAKE_INSTALL_XXX variables manually:

%cmake . -DCMAKE_INSTALL_BINDIR:PATH=%{_bindir} \
         -DCMAKE_INSTALL_SBINDIR:PATH=%{_sbindir} \
         -DCMAKE_INSTALL_LIBDIR:PATH=%{_libdir} \
         -DCMAKE_INSTALL_LIBEXECDIR:PATH=%{_libexecdir} \
         -DCMAKE_INSTALL_LOCALSTATEDIR:PATH=%{_localstatedir} \
         -DCMAKE_INSTALL_SHAREDSTATEDIR:PATH=%{_sharedstatedir} \
         -DCMAKE_INSTALL_INCLUDEDIR:PATH=%{_includedir} \
         -DCMAKE_INSTALL_INFODIR:PATH=%{_infodir} \
         -DCMAKE_INSTALL_MANDIR:PATH=%{_mandir} \
         -DCMAKE_INSTALL_SYSCONFDIR:PATH=%{_sysconfdir} \

Version-Release number of selected component (if applicable):
3.4.2-1.fc24

Additional info:

Somebody already made a draft:
https://fedoraproject.org/wiki/PackagingDrafts/cmake:

> Proposal: update %cmake rpm macro to set: CMAKE_INSTALL_LIBDIR, INCLUDE_INSTALL_DIR, LIB_INSTALL_DIR, SYSCONF_INSTALL_DIR, SHARE_INSTALL_PREFIX

Please fix either `%cmake` macro or GNUInstallDirs.cmake script.
I hope it isn't so big deal to add my snipplet (see above) to %cmake macro.
Comment 1 Rex Dieter 2016-01-25 15:02:13 EST
May make sense to come up with a new macro that uses these new GNUInstallDirs-style variables/paths, affording the ability to drop legacy cruft used in %%cmake , something like: 

%cmake_gnu
or
%cmake_gnuinstalldirs
(latter certainly is long, but more descriptive)
Comment 2 Orion Poplawski 2016-01-25 21:31:40 EST
We definitely need a new macro - there is a conflict between the common cmake usage of CMAKE_INSTALL_LIBDIR as something that is appended to CMAKE_INSTALL_PREFIX, and GNUInstallDirs' usage as a full path.  We used to define CMAKE_INSTALL_LIBDIR in %cmake but had to remove it because of this.

I've not yet run into a package that needed to define all of these, but I concur that a %cmake_gnuinstalldirs macro could be useful.
Comment 3 Orion Poplawski 2016-01-28 22:35:53 EST
After looking at this closer, it actually appears that the expected usage of the CMAKE_INSTALL_<dir> paths by GNUInstallDirs are the relative paths, although you can set them to absolute paths.  See https://cmake.org/cmake/help/v3.4/module/GNUInstallDirs.html

And with just:

cmake_minimum_required(VERSION 2.8)
include(GNUInstallDirs)

and

cmake -DCMAKE_INSTALL_PREFIX=/usr ..

I end up with:

CMAKE_INSTALL_FULL_BINDIR = /usr/bin
CMAKE_INSTALL_FULL_DATADIR = /usr/share
CMAKE_INSTALL_FULL_DATAROOTDIR = /usr/share
CMAKE_INSTALL_FULL_DOCDIR = /usr/share/doc/Project
CMAKE_INSTALL_FULL_INCLUDEDIR = /usr/include
CMAKE_INSTALL_FULL_INFODIR = /usr/share/info
CMAKE_INSTALL_FULL_LIBDIR = /usr/lib64
CMAKE_INSTALL_FULL_LIBEXECDIR = /usr/libexec
CMAKE_INSTALL_FULL_LOCALEDIR = /usr/share/locale
CMAKE_INSTALL_FULL_LOCALSTATEDIR = /var
CMAKE_INSTALL_FULL_MANDIR = /usr/share/man
CMAKE_INSTALL_FULL_OLDINCLUDEDIR = /usr/include
CMAKE_INSTALL_FULL_SBINDIR = /usr/sbin
CMAKE_INSTALL_FULL_SHAREDSTATEDIR = /usr/com
CMAKE_INSTALL_FULL_SYSCONFDIR = /etc

The only ones that are incorrect are DOCDIR and SHAREDSTATEDIR, so I would be tempted to just add these two to the current %cmake macro.
Comment 4 Rex Dieter 2016-01-29 08:22:37 EST
Possible, but doing it that way means the new macro won't respect other well-known rpm macros like:
%{_bindir}, %{_libdir}, %{_datadir}, %{_localstatedir}, %{_mandir} etc...
Comment 5 Roman Tsisyk 2016-02-01 14:12:45 EST
Orion, I'm sorry. It seems that this bug has already been fixed in rawhide.
Here is my output from CentOS 6, CentOS 7, Fedora 22, Fedora 23 (official Docker images):

```
-- CMAKE_INSTALL_FULL_SYSCONFDIR /usr/etc <!-- Invalid value
-- CMAKE_INSTALL_FULL_BINDIR /usr/bin
-- CMAKE_INSTALL_FULL_SBINDIR /usr/sbin
-- CMAKE_INSTALL_FULL_LIBDIR /usr/lib64
-- CMAKE_INSTALL_FULL_LIBEXECDIR /usr/libexec
-- CMAKE_INSTALL_FULL_LOCALSTATEDIR /usr/var
-- CMAKE_INSTALL_FULL_SHAREDSTATEDIR /usr/com
-- CMAKE_INSTALL_FULL_DATADIR /usr/share
-- CMAKE_INSTALL_FULL_INCLUDEDIR /usr/include
-- CMAKE_INSTALL_FULL_INFODIR /usr/share/info
```

Compare with rawhide:

```
-- CMAKE_INSTALL_FULL_SYSCONFDIR /etc <!--  correct
-- CMAKE_INSTALL_FULL_BINDIR /usr/bin
-- CMAKE_INSTALL_FULL_SBINDIR /usr/sbin
-- CMAKE_INSTALL_FULL_LIBDIR /usr/lib64
-- CMAKE_INSTALL_FULL_LIBEXECDIR /usr/libexec
-- CMAKE_INSTALL_FULL_LOCALSTATEDIR /var
-- CMAKE_INSTALL_FULL_SHAREDSTATEDIR /usr/com
-- CMAKE_INSTALL_FULL_DATADIR /usr/share
-- CMAKE_INSTALL_FULL_INCLUDEDIR /usr/include
-- CMAKE_INSTALL_FULL_INFODIR /usr/share/info
-- CMAKE_INSTALL_FULL_MANDIR /usr/share/man
```

This ticket probably should be closed if nobody wants a proper default value of SYSCONFDIR in Fedora 22, Fedora 23, EPEL 6, EPEL 7, etc. I'll kept only `-DCMAKE_INSTALL_SYSCONFDIR:PATH=%{_sysconfdir}` for compatibility in my specs.
Comment 6 Roman Tsisyk 2016-02-01 14:49:43 EST
Ahh, CMAKE_INSTALL_FULL_LOCALSTATEDIR /usr/var is also incorrect.
Comment 7 Roman Tsisyk 2016-02-01 14:57:17 EST
I see that this problem was fixed in upstream CMake:

https://github.com/Kitware/CMake/blob/master/Modules/GNUInstallDirs.cmake#L323-L353
Comment 8 Denis Fateyev 2016-02-01 15:13:04 EST
(In reply to Roman Tsisyk from comment #5)
> Orion, I'm sorry. It seems that this bug has already been fixed in rawhide.
...
> This ticket probably should be closed if nobody wants a proper default value
> of SYSCONFDIR in Fedora 22, Fedora 23, EPEL 6, EPEL 7, etc.

According the list above, CMAKE_INSTALL_FULL_SHAREDSTATEDIR (and maybe also CMAKE_INSTALL_FULL_DOCDIR) is still invalid in Rawhide.

I believe after the fix it should be ported to epel{6,7} to generalize macros set and avoid all possible discrepancies in EPEL in the future.

> Ahh, CMAKE_INSTALL_FULL_LOCALSTATEDIR /usr/var is also incorrect.

But according your list above it's sane:
-- CMAKE_INSTALL_FULL_LOCALSTATEDIR /var
Comment 9 Roman Tsisyk 2016-02-01 15:28:43 EST
> But according your list above it's sane:
> -- CMAKE_INSTALL_FULL_LOCALSTATEDIR /var

It is sane only on rawhide. Other releases uses /usr/var.
Comment 10 Jan Kurik 2016-02-24 09:20:43 EST
This bug appears to have been reported against 'rawhide' during the Fedora 24 development cycle.
Changing version to '24'.

More information and reason for this action is here:
https://fedoraproject.org/wiki/Fedora_Program_Management/HouseKeeping/Fedora24#Rawhide_Rebase
Comment 11 Denis Fateyev 2016-02-28 07:04:53 EST
Are there any changes here since cmake 3.4.3 / 3.5.0rc landed to rawhide?
As for the fixed macros backporting, I do believe that it would be worth to el6, epel7 and f23 (at least).
Comment 12 Rex Dieter 2017-07-14 10:12:43 EDT
marking FutureFeature, to avoid auto-close

Note You need to log in before you can comment on or make changes to this bug.