D23552: ECM: remove set_package_properties from FindCanberra

[Friedrich W. H. Kossebau]

kossebau added a comment.


  Compare though all the
  
  > Ideally this is set already directly in the Find-module.
  
  for `DESCRIPTION` and `URL` properties in the docs 
https://cmake.org/cmake/help/latest/module/FeatureSummary.html
  
  Which makes sense to me, at least for FindModules, where one can assume the 
FindX.cmake file exists, so the properties are set, and one only has to add the 
values for the properties `PURPOSE` and `TYPE`, as their are usage dependent.
  So in this case perhaps rather KNotifications should be adapted.

REPOSITORY
  R240 Extra CMake Modules

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

To: dfaure, cgiboudeaux, sitter
Cc: kossebau, kde-buildsystem, kde-frameworks-devel, apol, aacid, LeGast00n, 
GB_2, bencreasy, michaelh, ngraham, bruns

D23552: ECM: remove set_package_properties from FindCanberra

[Harald Sitter]

sitter added a comment.


  No objections from me. This was explicitly suggested during review though, so 
I'd like @cgiboudeaux to approve this.

INLINE COMMENTS

> FindCanberra.cmake:95
>  
>  include(FeatureSummary)
>  

Can be removed as well.

REPOSITORY
  R240 Extra CMake Modules

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

To: dfaure, cgiboudeaux, sitter
Cc: kde-buildsystem, kde-frameworks-devel, apol, aacid, LeGast00n, GB_2, 
bencreasy, michaelh, ngraham, bruns

D23551: CMake config files: use as min dep version the Qt version we built against

[Friedrich W. H. Kossebau]

kossebau added a comment.


  In D23551#521571 , @apol wrote:
  
  > This will only work one-way. If the "wrong" Qt is older, but it's not 
necessarily the case.
  >  Also cmake may be linking against the "right" Qt but then use the "wrong" 
one at runtime.
  
  
  Yes, this will not fix everything, just make things a bit more correct, and 
help at least in some situations. There is nothing lost, besides having to 
maintain one more variable.
  
  Because right now, in some builds the generated CMake config is wrong about 
the min version when building against latest Qt (KIconThemes & KIO at least, 
due to some `#if QT_VERSION >= QT_VERSION_CHECK(5, 12, 0)` dependent API usage).

REPOSITORY
  R263 KXmlGui

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

To: kossebau, #frameworks, aacid, #build_system
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23552: ECM: remove set_package_properties from FindCanberra

[David Faure]

dfaure created this revision.
dfaure added reviewers: cgiboudeaux, sitter.
Herald added projects: Frameworks, Build System.
dfaure requested review of this revision.

REVISION SUMMARY
  The user of this find module has to do it, so that they can specify the
  purpose of using canberra in their application. We can't do it twice,
  it leads to
  
  - Warning: Property DESCRIPTION for package Canberra already set to "Event 
sound library", overriding it with "Library for generating event sounds"
  
  The documentation for FeatureSummary always does find_package
  and then set_package_properties, i.e. it was never meant for the
  finder to do this.

TEST PLAN
  knotifications no longer warns

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

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

AFFECTED FILES
  find-modules/FindCanberra.cmake

To: dfaure, cgiboudeaux, sitter
Cc: kde-buildsystem, kde-frameworks-devel, apol, aacid, LeGast00n, GB_2, 
bencreasy, michaelh, ngraham, bruns

D23551: CMake config files: use as min dep version the Qt version we built against

[Aleix Pol Gonzalez]

apol added a comment.


  This will only work one-way. If the "wrong" Qt is older, but it's not 
necessarily the case.
  Also cmake may be linking against the "right" Qt but then use the "wrong" one 
at runtime.
  
  I'm not convinced.

REPOSITORY
  R263 KXmlGui

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

To: kossebau, #frameworks, aacid, #build_system
Cc: apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23551: CMake config files: use as min dep version the Qt version we built against

[Friedrich W. H. Kossebau]

kossebau added a reviewer: Build System.

REPOSITORY
  R263 KXmlGui

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

To: kossebau, #frameworks, aacid, #build_system
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D23550: CMake config files: use as min dep version the Qt version we built against

[Friedrich W. H. Kossebau]

kossebau added a reviewer: Build System.

REPOSITORY
  R244 KCoreAddons

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

To: kossebau, #frameworks, #build_system
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns