Figure out the GTK version instead of using a hardcoded one
ClosedPublic

Authored by garg on Feb 13 2017, 12:41 PM.

Details

Summary

Find the GTK3 pkg-config file and pick up a version using that.

Test Plan
  • Do a clean rebuild
  • Picks up the right GTK3 version and installs the right CSS file.

Diff Detail

Repository
R98 Breeze for Gtk
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
garg updated this revision to Diff 11286.Feb 13 2017, 12:41 PM
garg retitled this revision from to Figure out the GTK version instead of using a hardcoded one.
garg updated this object.
garg edited the test plan for this revision. (Show Details)
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 13 2017, 12:41 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
garg added a comment.Feb 13 2017, 1:14 PM

This should ideally also be backported for Plasma 5.9

sitter added a subscriber: sitter.Feb 13 2017, 3:50 PM
garg updated this revision to Diff 11296.Feb 13 2017, 4:00 PM

Make finding GTK required, move find_package calls together

lbeltrame added a subscriber: lbeltrame.EditedFeb 13 2017, 4:00 PM

FTR, the reason this was not done at CMake time was to avoid depending on a development package (and associated dependencies like a compiler) to install something that's architecture-independent. My first. local patch I made at the time used pkg_config, but it was discarded for this reason.

Not that I have hard feelings about this, should this be changed, but I just wanted to outline the rationale for it at the time.

garg added a comment.Feb 13 2017, 4:03 PM

FTR, the reason this was not done at CMake time was to avoid depending on a development package (and associated dependencies like a compiler) to install something that's architecture-dependent. My first. local patch I made at the time used pkg_config, but it was discarded for this reason.

Not that I have hard feelings about this, should this be changed, but I just wanted to outline the rationale for it at the time.

Understood, though I fear that hard coding the default leads to broken GTK themeing when the default gtk version in breeze gtk (3.18) is older than what a system might have ( for eg. 3.20 and above ).

This eliminates the ambiguity altogether.

garg updated this revision to Diff 11297.Feb 13 2017, 4:07 PM

Don't cache string, set a CMAKE_MODULE_PATH too

garg updated this revision to Diff 11298.Feb 13 2017, 4:10 PM

Simplify setting up the variable

sitter accepted this revision.Feb 13 2017, 4:10 PM
sitter added a reviewer: sitter.
This revision is now accepted and ready to land.Feb 13 2017, 4:10 PM
apol added a comment.Feb 13 2017, 4:14 PM

You can't drop the CACHE on WITH_GTK3_VERSION. You should keep it or rename the variable.

This changes the build requirements. It should not go into 5.9 IMO.

This revision was automatically updated to reflect the committed changes.
garg added a comment.EditedFeb 13 2017, 4:23 PM

I would argue that breeze-gtk should already have had this change in order to not break on systems where GTK is >= 3.20 and that introducing a new build-dep is the only way of fixing thee issue.

In D4596#86009, @garg wrote:

I would argue that breeze-gtk should already have had this change in order to not break on systems where GTK is >= 3.20 and that introducing a new build-dep is the only way of fixing thee issue.

Please patch it at the distro level until 5.10. I *totally* disagree on behavioral changes on stable releases, even for build requirements.

I have reverted both commits in 5.9.