Fix build with pango 1.44
ClosedPublic

Authored by arojas on Jul 30 2019, 4:34 PM.

Details

Summary

pango>=1.44 headers depend on harfbuzz ones, make sure cmake can find them and pass the right include dir to the compiler

Test Plan

Builds

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.
arojas created this revision.Jul 30 2019, 4:34 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 30 2019, 4:34 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
arojas requested review of this revision.Jul 30 2019, 4:34 PM
sitter edited the summary of this revision. (Show Details)Jul 30 2019, 10:23 PM
sitter added a subscriber: sitter.Jul 30 2019, 10:27 PM

Shouldn't this somehow be handled via pkg-config? IIRC .pc files can indicate dependencies, so I would assume one could get the requirements somehow and resolve those using generic code instead of essentially replicating the dependency information in cmake.

apol added a subscriber: apol.Jul 30 2019, 11:42 PM

Shouldn't this somehow be handled via pkg-config? IIRC .pc files can indicate dependencies, so I would assume one could get the requirements somehow and resolve those using generic code instead of essentially replicating the dependency information in cmake.

Quite possibly, still I'm happy to let this in. FindGTK3.cmake shouldn't be needed altogether and it's a local change to kde-gtk-config.

Shouldn't this somehow be handled via pkg-config? IIRC .pc files can indicate dependencies, so I would assume one could get the requirements somehow and resolve those using generic code instead of essentially replicating the dependency information in cmake.

'pkgconf --cflags gtk+-3.0' and 'pkgconf --libs gtk+-3.0' will recursively return all required cflags and ldflags. But it doesn't look like this is exposed in cmake's pkg_check_modules - this just returns the first-level dependencies.

Anyhow, this is just a hotfix to make the current FindGTK3 work with the latest release of pango. Any further changes towards simplifying it should go in a different commit IMO.

Sure, don't take my bubbling to mean that I object to the diff. I'm just musing on it's long-term necessity

When I dump the variables created by the initial PKG_CHECK_MODULES call in the finder they do have the requirements in the LIBRARIES and INCLUDES. So in point of fact I guess the entire finder is doing it wrong to begin with? ^^
The reason it needs to reimplement the entire dependency lookup is because it uses the variables incorrectly. Out of the retrieved data from pkgconfig it constructs find_ calls of which (I guess) the find_path for includes is entirely moot because PC_GTK3_INCLUDE_DIRS is complete already, and the find_library call should loop over the names in PC_GTK3_LIBRARIES and find them. Actually I'm also right now looking at the FindPkgConfig and it looks like it has a IMPORTED_TARGET since 3.6, so if we were to bump the cmake requirements of plasma the entire finder could be shrunk down to the imported targets option which internally will cause findpkgconfig to do the find_library dance *and* set up neat targets we can use from the outside.

-- PC_GTK3_CFLAGS=-pthread;-I/usr/include/gtk-3.0;-I/usr/include/at-spi2-atk/2.0;-I/usr/include/at-spi-2.0;-I/usr/include/dbus-1.0;-I/usr/lib/x86_64-linux-gnu/dbus-1.0/include;-I/usr/include/gtk-3.0;-I/usr/include/gio-unix-2.0/;-I/usr/include/cairo;-I/usr/include/pango-1.0;-I/usr/include/harfbuzz;-I/usr/include/pango-1.0;-I/usr/include/atk-1.0;-I/usr/include/cairo;-I/usr/include/pixman-1;-I/usr/include/freetype2;-I/usr/include/libpng16;-I/usr/include/freetype2;-I/usr/include/libpng16;-I/usr/include/gdk-pixbuf-2.0;-I/usr/include/libpng16;-I/usr/include/glib-2.0;-I/usr/lib/x86_64-linux-gnu/glib-2.0/include
-- PC_GTK3_CFLAGS_I=
-- PC_GTK3_CFLAGS_OTHER=-pthread
-- PC_GTK3_FOUND=1
-- PC_GTK3_INCLUDEDIR=/usr/include
-- PC_GTK3_INCLUDE_DIRS=/usr/include/gtk-3.0;/usr/include/at-spi2-atk/2.0;/usr/include/at-spi-2.0;/usr/include/dbus-1.0;/usr/lib/x86_64-linux-gnu/dbus-1.0/include;/usr/include/gtk-3.0;/usr/include/gio-unix-2.0/;/usr/include/cairo;/usr/include/pango-1.0;/usr/include/harfbuzz;/usr/include/pango-1.0;/usr/include/atk-1.0;/usr/include/cairo;/usr/include/pixman-1;/usr/include/freetype2;/usr/include/libpng16;/usr/include/freetype2;/usr/include/libpng16;/usr/include/gdk-pixbuf-2.0;/usr/include/libpng16;/usr/include/glib-2.0;/usr/lib/x86_64-linux-gnu/glib-2.0/include
-- PC_GTK3_LDFLAGS=-lgtk-3;-lgdk-3;-lpangocairo-1.0;-lpango-1.0;-latk-1.0;-lcairo-gobject;-lcairo;-lgdk_pixbuf-2.0;-lgio-2.0;-lgobject-2.0;-lglib-2.0
-- PC_GTK3_LDFLAGS_OTHER=
-- PC_GTK3_LIBDIR=/usr/lib/x86_64-linux-gnu
-- PC_GTK3_LIBRARIES=gtk-3;gdk-3;pangocairo-1.0;pango-1.0;atk-1.0;cairo-gobject;cairo;gdk_pixbuf-2.0;gio-2.0;gobject-2.0;glib-2.0
-- PC_GTK3_LIBRARY_DIRS=
-- PC_GTK3_LIBS=
-- PC_GTK3_LIBS_L=
-- PC_GTK3_LIBS_OTHER=
-- PC_GTK3_LIBS_PATHS=
-- PC_GTK3_PREFIX=/usr

That's from the .pc file

prefix=/usr
exec_prefix=${prefix}
libdir=/usr/lib/x86_64-linux-gnu
includedir=${prefix}/include
targets=x11 broadway wayland

gtk_binary_version=3.0.0
gtk_host=x86_64-pc-linux-gnu

Name: GTK+
Description: GTK+ Graphical UI Library
Version: 3.22.30
Requires: gdk-3.0 atk >= 2.15.1 cairo >= 1.14.0 cairo-gobject >= 1.14.0 gdk-pixbuf-2.0 >= 2.30.0 gio-2.0 >= 2.49.4
Requires.private: atk atk-bridge-2.0 wayland-client >= 1.9.91 wayland-protocols >= 1.12 xkbcommon >= 0.2.0 wayland-cursor >= 1.9.91 wayland-egl  epoxy >= 1.0 pangoft2 gio-unix-2.0 >= 2.49.4
Libs: -L${libdir} -lgtk-3 
Cflags: -I${includedir}/gtk-3.0 
~

Ping, can this go in for 5.16.5?

ngraham accepted this revision.Aug 24 2019, 5:24 PM
ngraham added a subscriber: ngraham.

Just ran into the same issue today and prepared a similar patch right before getting a notification email about this! Let's do it for the stable branch for sure.

This revision is now accepted and ready to land.Aug 24 2019, 5:24 PM
This revision was automatically updated to reflect the committed changes.