kossebau added a comment.
In D29299#660620 <https://phabricator.kde.org/D29299#660620>, @pino wrote: > In D29299#660614 <https://phabricator.kde.org/D29299#660614>, @kossebau wrote: > > > One does not need to define KDE_INSTALL_LOCALEDIR oneself. One only needs to use find_package(KF5Plasma) or find_package(KF5Package), because some subrepo also does a plasma applet for Plasma integration or have someone cluelessly copied cmake code together because ENOTIMEFORBUILDSYSTEM and copying from the best=KDE and now-it-builds-so-do-not-touch-again. > > > In this case it will be generated automatically, and it will be the same as LOCALE_INSTALL_DIR. Only if nothing else sets this variable. And people do set such variables manually, e.g. after having looked up the docs of the macros they use. Because they have some crazy ideas/needs about installation/deploy layouts. Besides, LOCALE_INSTALL_DIR is also the name used commonly in custom variants of KDEInstallDirs/GnuInstallDirs (e.g. check github search for DefineInstallationPaths.cmake copies or just grep for LOCALE_INSTALL_DIR itself), possibly inspired by KDE's initial KDE4 times list. And this does not blend well, people miss where variables are from and just manually override things or reorder includes until things work. Again, LOCALE_INSTALL_DIR is the variable documented in the dox and the code which people have seen and made sure it is set to a value they want. And it is a variable used in some projects, independent of KDE. KDEInstallDirs messes around with that as well, but projects which have that included knowingly or unknowingly have worked around that. Now if suddenly KDE_INSTALL_LOCALEDIR gets to rule over LOCALE_INSTALL_DIR, it will break things for them. (people could also have unset LOCALE_INSTALL_DIR to enforce the default, and then KDE_INSTALL_LOCALEDIR as first fallback would screw that, ,but I see even lesser chance in that, not a pattern I have come across) > Again: beside what I said earlier, do you see any **actual** potential other scenarios, nor situation where what I proposed would not be safe? I cannot point you to any existing public available code which will fall out over this. But given my experiences on other people's cmake code, I am pretty sure there some using ki18n outside kde which run chance. People do have random "include(KDEInstallDirs)" around, due to helpless cmake code copy&paste. People try to write Plasma applets, thus injecting KDEInstallDirs via find(Plasma) into their otherwise not-ECM-based cmake system. >> Why stop people caring for doing things more safely? :) > > I am not, please do not put thoughts in my mind that were not mine, thanks :) Just telling what thoughts are created at receiver's end about the sending one :) For some context: I care a lot about ECM. After all I am one of the top-2 ECM contributors for the last 2 years. When it comes to KDEInstallDirs, you can find lots of commits from me over all repos which ported code to use non-deprecated KDEInstallDirs variables. And it was me who introduced the use of KDE_INSTALL_DIRS_NO_DEPRECATED with Marble's buildsystem to marry the desire of Marble maintainers to have a Qt-only dependency build variant together with my desire of a full-featured ECM/KF/Plasma one, without cmake variables stepping on each other toes. At least at that time to success. It seems it boils down to: you think no-one writes such broken/fragile cmake code that can be trapped by this. I do believe there are some. Both variants of the patches are correct (right?) and fix things for KDE_INSTALL_DIRS_NO_DEPRECATED. The variant I prefer though increases the chance to not break things for any potential fragile code out there. Can't you just give me the credit my gut feelings are based on experience? Is there any actual reason the variant you prefer has to be used instead? REPOSITORY R249 KI18n REVISION DETAIL https://phabricator.kde.org/D29299 To: kossebau, ilic, heikobecker, #frameworks, aacid, ltoscano Cc: pino, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
