Improve CMake handling of GNOME libraries
ClosedPublic

Authored by cblack on Mon, Mar 16, 11:45 PM.

Details

Summary

GNOME libraries were being handled in a manner not
as elegant as possible with CMake's built-in pkgconf handling.
This patch uses CMake's built-in pkgconf handling to trim excess
code and improve style (${GTK_LIBRARIES} -> PkgConfig::GTK+3).

Test Plan

See that everything still builds and links as expected.

Diff Detail

Repository
R99 KDE Gtk Configuration Tool
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
cblack created this revision.Mon, Mar 16, 11:45 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMon, Mar 16, 11:45 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
cblack requested review of this revision.Mon, Mar 16, 11:45 PM
apol accepted this revision.Tue, Mar 17, 12:14 AM
apol added a subscriber: apol.

Good stuff!

This revision is now accepted and ready to land.Tue, Mar 17, 12:14 AM

It builds, but why aren't you using that stuff inside of FingGTK3.cmake module? It actually uses it inside already, but with some bugs, that prevents from building last patch.

gikari requested changes to this revision.Tue, Mar 17, 9:00 AM

I reverted the commits ( R99:306e01d86ab9a891ec70219802ce01572b0d4025 and R99:306e01d86ab9a891ec70219802ce01572b0d4025 ) for recoloration module, to fix the CI. I think you need to create a new revision to add recolaration module in a proper way and with proper fixes to GNOME libraries handling.

This revision now requires changes to proceed.Tue, Mar 17, 9:00 AM
cblack updated this revision to Diff 77831.Tue, Mar 17, 2:47 PM

Rebase on master for reverted commits

It builds, but why aren't you using that stuff inside of FingGTK3.cmake module? It actually uses it inside already, but with some bugs, that prevents from building last patch.

It's a lot of CMake code that duplicates what CMake already offers with its pkgconf handling. There's no reason to use it when CMake can wrangle that stuff for you.

gikari accepted this revision.Tue, Mar 17, 2:54 PM

Fine. It builds.

This revision is now accepted and ready to land.Tue, Mar 17, 2:54 PM
This revision was automatically updated to reflect the committed changes.

This has been reverted. @cblack, please take a look at what has happened. Seems like CI is missing some dependency.

gikari reopened this revision.Mon, Mar 23, 10:50 AM
This revision is now accepted and ready to land.Mon, Mar 23, 10:50 AM
gikari requested changes to this revision.Mon, Mar 23, 10:57 PM
This revision now requires changes to proceed.Mon, Mar 23, 10:57 PM
gikari added inline comments.Mon, Mar 23, 11:03 PM
CMakeLists.txt
22

If I understand correctly this line is redundant.

gtkproxies/CMakeLists.txt
7

And PkgConfig::giomm is redundant too.

cblack updated this revision to Diff 78590.Thu, Mar 26, 8:45 PM

Drop giomm

gikari accepted this revision.Fri, Mar 27, 11:59 AM

Ok, let's keep our eyes on CI now. Everything should be fine, since the dependencies didn't change.

This revision is now accepted and ready to land.Fri, Mar 27, 11:59 AM
This revision was automatically updated to reflect the committed changes.

Everything is fine on CI.