Review Request 124630: Use SHARE_INSTALL_PREFIX instead of {INSTALL_PREFIX, }share

[Heiko Becker]

> On Aug. 10, 2015, 12:28 nachm., Friedrich W. H. Kossebau wrote:
> > krita/data/profiles/CMakeLists.txt, line 8
> > <https://git.reviewboard.kde.org/r/124630/diff/1/?file=390416#file390416line8>
> >
> >     This is not a proper substitution:
> >     `${DATA_INSTALL_DIR}` points to `share/apps`, while here the files should be installed to `share/color` (so with `color` on the same level as `apps`, like `icons`, `sounds` etc.
> >     
> >     Which results in all files now installed in wrong location `share/apps/color`, instead of `share/color`.
> >     
> >     So breaks things completely.
> >     
> >     Not sure what you are trying to fix exactly:
> >     How does using `${DATA_INSTALL_DIR}` vs. `${CMAKE_INSTALL_PREFIX}` fix the mentioned problem? IIRC (well, just looked up again) `FindKDE4Internal.cmake` derives `DATA_INSTALL_DIR` from `${CMAKE_INSTALL_PREFIX}`.
> 
> Heiko Becker wrote:
>     Perhaps I was carried away by usage of ${DATA_INSTALL_DIR} everywhere else. Sorry for that.
>     
>     And to answer the second question, it works because ${DATA_INSTALL_DIR} derives from ${SHARE_INSTALL_PREFIX}, which itself indeed derives from ${CMAKE_INSTALL_PREFIX}, but one can set it to a different location.
>     
>     `DESTINATION ${SHARE_INSTALL_PREFIX}/color/icc/krita` also looks like a correct fix.
>     
>     Using the variables defined by ecm's KDEInstallDirs would even be a better fix, in my opinion. But seeing that KDEInstallDirs is already included, I guess this isn't done to keep the delta during porting as small as possible?
> 
> Boudewijn Rempt wrote:
>     For now I've reverted the commit: the default profiles were not showing up for our testers. I'm fine with any patch that fixes that, of course :-)
> 
> Friedrich W. H. Kossebau wrote:
>     > ${SHARE_INSTALL_PREFIX}
>     
>     That one is an internal variable, no? At least it is not documented to be "exported", but perhaps it still might be save enough to rely on it.
>     Still, how can one "set it to a different location"?
>     
>     For ECM-based build system (i.e. Calligra frameworks branch), which var would be the one to use?

Sorry for the late update/answer.


>> ${SHARE_INSTALL_PREFIX}

> That one is an internal variable, no? At least it is not documented to be "exported", but perhaps it still might be save enough to rely on it.
> Still, how can one "set it to a different location"?

The cmake magic behind _set_fancy in FindKDE4Internal:
https://quickgit.kde.org/?p=kdelibs.git&a=blob&h=7d54b9b83b999f6a6e8541f8e8b89e0b2a105703&hb=4f61f995f05ce9d3fa1bd43e11278ae216552466&f=cmake%2Fmodules%2FFindKDE4Internal.cmake#l765

And it's already used in calligra.


> For ECM-based build system (i.e. Calligra frameworks branch), which var would be the one to use?

In this case probably KDE_INSTALL_FULL_DATAROOTDIR, but KDEInstallDirs has a bunch of variables for icons, appstream metadata, etc.:

http://api.kde.org/ecm/kde-module/KDEInstallDirs.html


- Heiko


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124630/#review83653
-----------------------------------------------------------


On Sept. 8, 2015, 7:05 nachm., Heiko Becker wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124630/
> -----------------------------------------------------------
> 
> (Updated Sept. 8, 2015, 7:05 nachm.)
> 
> 
> Review request for Calligra.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> Allowing to configure the install location, which is helpful with a
> multiarch layout, where prefix might be something like /usr/<arch> but
> arch independent data should be installed to /usr/share/...
> 
> 
> Diffs
> -----
> 
>   active/CMakeLists.txt 8fa0c6f 
>   krita/data/profiles/CMakeLists.txt a2a997b 
>   krita/data/profiles/elles-icc-profiles/CMakeLists.txt 8aa5a83 
> 
> Diff: https://git.reviewboard.kde.org/r/124630/diff/
> 
> 
> Testing
> -------
> 
> tquiChecked that the affected files get installed into the desired location.
> 
> 
> Thanks,
> 
> Heiko Becker
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20150908/cdac50f3/attachment.htm>