D29136: Use non-deprecated KDEInstallDir

[David Faure]

dfaure added a comment.


  (to remove some confusion: the previous comment had the wrong link and should 
have said "Abandoned in favour of https://phabricator.kde.org/D29299; -- but 
now it's reopened anyway, as an alternative to D29299 
)

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D29136

To: heikobecker
Cc: dfaure, kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, 
ngraham, bruns

D29136: Use non-deprecated KDEInstallDir

[Heiko Becker]

heikobecker reclaimed this revision.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D29136

To: heikobecker
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29136: Use non-deprecated KDEInstallDir

[Heiko Becker]

heikobecker abandoned this revision.
heikobecker added a comment.


  Abandoned in favour of https://phabricator.kde.org/D29136

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D29136

To: heikobecker
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29136: Use non-deprecated KDEInstallDir

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In D29136#660358 , @heikobecker 
wrote:
  
  > > Where would you see "that the macro already used KDEInstallDirs before"? 
When it comes to "LOCALE_INSTALL_DIR", that is set to a default is not set when 
calling the macro. Ideally would be documented though. (my first approach would 
be to also allow a soft dependency here on KDEInstallDirs, checking whether 
KDE_INSTALL_LOCALEDIR is defined and picking its value), similar with 
CMAKE_INSTALL_LOCALEDIR to support GnuInstallDirs automatically).
  >
  > As you indicate, if one calls ki18_install() in a project which includes 
KDEInstallDirs before that call (and KDE_INSTALL_DIRS_NO_DEPRECATED isn't set) 
the value of LOCALE_INSTALL_DIR is used instead of the default "share/locale". 
I extended the same to check K_I_LOCALEDIR (line 99+). Isn't that the soft 
dependency you suggested (minus the GnuInstallDirs part, obviously?)
  
  
  I was more thinking something along D29299 
 (turned my draft code into a proper patch 
for discussing, while at it :) ).
  
  From what I meanwhile found, people using GnuInstallDirs would only have 
CMAKE_INSTALL_LOCALEDIR set, so they can be ignored here then, at least I would 
rather advocate for explicite argument passing as in the alternative patch.

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D29136

To: heikobecker
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29136: Use non-deprecated KDEInstallDir

[Heiko Becker]

heikobecker added a comment.


  > Where would you see "that the macro already used KDEInstallDirs before"? 
When it comes to "LOCALE_INSTALL_DIR", that is set to a default is not set when 
calling the macro. Ideally would be documented though. (my first approach would 
be to also allow a soft dependency here on KDEInstallDirs, checking whether 
KDE_INSTALL_LOCALEDIR is defined and picking its value), similar with 
CMAKE_INSTALL_LOCALEDIR to support GnuInstallDirs automatically).
  
  As you indicate, if one calls ki18_install() in a project which includes 
KDEInstallDirs before that call (and KDE_INSTALL_DIRS_NO_DEPRECATED isn't set) 
the value of LOCALE_INSTALL_DIR is used instead of the default "share/locale". 
I extended the same to check K_I_LOCALEDIR (line 99+). Isn't that the soft 
dependency you suggested (minus the GnuInstallDirs part, obviously?)

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D29136

To: heikobecker
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29136: Use non-deprecated KDEInstallDir

[Friedrich W. H. Kossebau]

kossebau added a comment.


  I understand the desire for pragmatic solution, but I am too old now and have 
seen too many places where being pragmatic at one point resulted in tightly 
coupled systems which later prevented progress. 
  When it comes to build dependencies, forcing users of a product to use the 
same build tools as used for the product is not well received by me in 
principle, as it lowers flexibility and choice as user.
  
  So let me see for now how we can get ahead here without needing to extent 
KI18nConfig with find_dependecy(ECM).
  
  Where would you see "that the macro already used KDEInstallDirs before"? When 
it comes to "LOCALE_INSTALL_DIR", that is set to a default is not set when 
calling the macro. Ideally would be documented though. (my first approach would 
be to also allow a soft dependency here on KDEInstallDirs, checking whether 
KDE_INSTALL_LOCALEDIR is defined and picking its value), similar with 
CMAKE_INSTALL_LOCALEDIR to support GnuInstallDirs automatically).

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D29136

To: heikobecker
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29136: Use non-deprecated KDEInstallDir

[Heiko Becker]

heikobecker added a comment.


  In D29136#660270 , @kossebau wrote:
  
  > using kdeinstalldirs variables needs to ensure that KDEInstallDirs has been 
included before, also introduces ahard  dependency on ECM for any users of 
KI18n. While 99% of apps using KI18n might do this, by design idea of KDE 
Frameworks KI18n as tier1 should not pull in another dependency, even ECM (so 
someone using plain cmake & GnuInstallDirs should be still able to use tier1 
stuff). This needs some more pondering then...
  
  
  Isn't that a mere theoretical concern? At least when building ki18n yourself, 
you need ECM anyway, which is easy to install and almost infinitesimal small in 
size compared to Qt. And btw, where is the "should not pull in another 
dependency, even ECM" documentend? I only know of "Tier 1 Frameworks can depend 
only on Qt official frameworks or other system libraries" [1], which admittedly 
already creates a discrepancy with the ECM (build) requirement of ki18n itself.
  
  I'd also note that the macro already used KDEInstallDirs before, apparently 
without anybody complaining about it, even though I don't want to cargo cult 
this. Furthermore I'm not sure how to solve the bug I encountered differently, 
other than making marble stop using KDE_INSTALL_DIRS_NO_DEPRECATED (or using 
GnuInstallDirs, which possibly might break existing things).
  
  [1] https://community.kde.org/Frameworks/Policies

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D29136

To: heikobecker
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29136: Use non-deprecated KDEInstallDir

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Just came by, have to get back later, but first comment:
  using kdeinstalldirs variables needs to ensure that KDEInstallDirs has been 
included before, also introduces ahard  dependency on ECM for any users of 
KI18n. While 99% of apps using KI18n might do this, by design idea of KDE 
Frameworks KI18n as tier1 should not pull in another dependency, even ECM (so 
someone using plain cmake & GnuInstallDirs should be still able to use tier1 
stuff). This needs some more pondering then...

REPOSITORY
  R249 KI18n

REVISION DETAIL
  https://phabricator.kde.org/D29136

To: heikobecker
Cc: kossebau, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29136: Use non-deprecated KDEInstallDir

[Heiko Becker]

heikobecker updated this revision to Diff 81035.
heikobecker added a comment.


  Added missing parentheses

REPOSITORY
  R249 KI18n

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D29136?vs=81033=81035

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D29136

AFFECTED FILES
  cmake/KF5I18nMacros.cmake.in

To: heikobecker
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29136: Use non-deprecated KDEInstallDir

[Heiko Becker]

heikobecker created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
heikobecker requested review of this revision.

REVISION SUMMARY
  I noticed that when installeding marble, which sets
  KDE_INSTALL_DIRS_NO_DEPRECATED, which then invalidates a possibly
  custom location set via KDEInstallIDirs, causing translations to
  land in a surprising locations.

TEST PLAN
  Marble's translations land in the desired location

REPOSITORY
  R249 KI18n

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D29136

AFFECTED FILES
  cmake/KF5I18nMacros.cmake.in

To: heikobecker
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns