Set SYSCONFDIR to /etc when CMAKE_INSTALL_SYSCONFDIR is etc relative to /usr
Needs ReviewPublic

Authored by pwojcik on Dec 27 2019, 5:53 PM.



Marble built with -DCMAKE_INSTALL_PREFIX=/usr installs its data into /usr/etc/xdg/marble.knsrc, what is wrong.
This is because KDEInstallDirs do not handle special case of CMAKE_INSTALL_SYSCONFDIR=etc with CMAKE_INSTALL_PREFIX=/usr.
I am not sure where CMAKE_INSTALL_SYSCONFDIR is set, but certainly not from commandline. Probably from GNUInstallDirs.
GNUInstallDirs already handle special cases: let's use it.

This change complements and make behavior as documented, i.e. :


read-only single-machine data (etc, or /etc if CMAKE_INSTALL_PREFIX is /usr)

Diff Detail

R240 Extra CMake Modules
Lint Skipped
Unit Tests Skipped
pwojcik created this revision.Dec 27 2019, 5:53 PM
Restricted Application added projects: Frameworks, Build System. · View Herald TranscriptDec 27 2019, 5:53 PM
Restricted Application added subscribers: kde-buildsystem, kde-frameworks-devel. · View Herald Transcript
pwojcik requested review of this revision.Dec 27 2019, 5:53 PM
pwojcik retitled this revision from Set SYSCONFDIR to /etc when CMAKE_INSTALL_PREFIX is /usr AND to Set SYSCONFDIR to /etc when CMAKE_INSTALL_SYSCONFDIR is etc relative to /usr.Dec 27 2019, 6:06 PM
pwojcik edited the summary of this revision. (Show Details)
pwojcik added reviewers: kossebau, alexmerry.
apol added a subscriber: apol.Dec 28 2019, 1:12 AM

Not saying that this patch is wrong, would have to look into it more closely.

But for knsrc files you want to use KDE_INSTALL_KNSRCDIR anyway, which will fall into /usr.


Why's /opt special?

pwojcik added inline comments.Dec 28 2019, 10:29 AM

/usr is single special case, but any subdirectory of /opt/ is special by GNUInstallDirs.cmake, following Filesystem Hierarchy Standard.

(Just remember that using KDE_INSTALL_KNSRCDIR though needs at least KNewStuffCore from KF 5.57 (hint was missing in API dox, proposing D26248 to fix that).)

Myself also need to reserve time to look closer at it, hoping to sneak in in the next days or week 1 of 2020

Please see to add a unit test also covering this, so this special case is covered.


Ideally gets a small explaining comment in the code, as this is not directly obvious.

pwojcik updated this revision to Diff 72404.Dec 30 2019, 5:36 PM

Corrected logic, so it is is in line with GNUInstallDirs. Explained in comment. Added tests.

I want to remind about topic.