use the feature summary to communicate runtime deps
ClosedPublic

Authored by sitter on Aug 9 2016, 7:54 AM.

Details

Test Plan
  • cmake
  • prints warning
  • can be overridden with CMAKE_DISABLE_FIND_PACKAGE_foo

Diff Detail

Repository
R135 Integration for Qt applications in Plasma
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sitter updated this revision to Diff 5754.Aug 9 2016, 7:54 AM
sitter retitled this revision from to use the feature summary to communicate runtime deps.
sitter updated this object.
sitter edited the test plan for this revision. (Show Details)
Restricted Application added a project: Plasma. · View Herald TranscriptAug 9 2016, 7:54 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript

I like packages stating their dependencies (even runtime ones) because that's where I look first as a packager. But if I'm not missing something (maybe a git add?) this change is a bit misleading because they can never be found. So I think two Find... modules are also needed or "message..." should stay.

sitter added a comment.Aug 9 2016, 9:25 AM

So I think two Find... modules are also needed or "message..." should stay.

Well. No.

Every non-REQUIRED find_package call can be disabled by setting the CMAKE_DISABLE_FIND_PACKAGE_<PackageName> variable to TRUE.

mart added a reviewer: Plasma.Aug 9 2016, 9:59 AM
davidedmundson accepted this revision.Aug 9 2016, 10:05 AM
davidedmundson added a reviewer: davidedmundson.
davidedmundson added a subscriber: davidedmundson.

I don't think we need the comments at the end.

This revision is now accepted and ready to land.Aug 9 2016, 10:05 AM
In D2377#44495, @sitter wrote:

So I think two Find... modules are also needed or "message..." should stay.

Well. No.

Every non-REQUIRED find_package call can be disabled by setting the CMAKE_DISABLE_FIND_PACKAGE_<PackageName> variable to TRUE.

Well, I understand that but I *really* don't think a message should be shown, that always yields the same result, no matter if it can be disabled via CMAKE_D_F_P_Foo or the packages in question are installed or not. What's the difference to message in that case?
Also imagine a user investigating why the two packages don't show up although he has installed them.

Also imagine a user investigating why the two packages don't show up although he has installed them.

In the landing version I have added notes about them not being detected automatically and how to ignore them. That said, if you want to write a reliable font finder that would be pretty cool.

This revision was automatically updated to reflect the committed changes.