diff mbox series

[RFC,v4,4/9] libgtk3: remove patch to disable atk-bridge support

Message ID 20180614224820.27126-5-joseph.kogut@gmail.com
State Changes Requested
Headers show
Series chromium: new package | expand

Commit Message

Joseph Kogut June 14, 2018, 10:48 p.m. UTC 
Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com>
---
 package/libgtk3/0003-disable-atk-bridge.patch | 45 -------------------
 package/libgtk3/Config.in                     |  1 +
 package/libgtk3/libgtk3.mk                    |  3 +-
 3 files changed, 3 insertions(+), 46 deletions(-)
 delete mode 100644 package/libgtk3/0003-disable-atk-bridge.patch

Comments

Thomas Petazzoni June 15, 2018, 7:53 p.m. UTC | #1 
Hello,

On Thu, 14 Jun 2018 15:48:15 -0700, Joseph Kogut wrote:
> Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com>
> ---
>  package/libgtk3/0003-disable-atk-bridge.patch | 45 -------------------
>  package/libgtk3/Config.in                     |  1 +
>  package/libgtk3/libgtk3.mk                    |  3 +-
>  3 files changed, 3 insertions(+), 46 deletions(-)
>  delete mode 100644 package/libgtk3/0003-disable-atk-bridge.patch
> 
> diff --git a/package/libgtk3/0003-disable-atk-bridge.patch b/package/libgtk3/0003-disable-atk-bridge.patch
> deleted file mode 100644
> index c92174f400..0000000000
> --- a/package/libgtk3/0003-disable-atk-bridge.patch
> +++ /dev/null
> @@ -1,45 +0,0 @@
> -Remove atk-bridge support.
> -
> -atk-bridge doesn't seem useful for now in Buildroot and requires to
> -add two new packages just for it: at-spi2-core and at-spi2-atk.
> -
> -Signed-off-by: Hadrien Boutteville <hadrien.boutteville@gmail.com>

Could we instead keep them optional, by improving this patch ? We
really try to keep dependencies to a reasonable level in Buildroot, so
it would be nice if we could avoid making those mandatory dependencies
of libgtk3.

> -
> ---- a/configure.ac
> -+++ b/configure.ac
> -@@ -1349,11 +1349,7 @@
> - # Check for Accessibility Toolkit flags
> - ########################################
> -
> --if test x$enable_x11_backend = xyes; then
> --   ATK_PACKAGES="atk atk-bridge-2.0"
> --else
> --   ATK_PACKAGES="atk"
> --fi
> -+ATK_PACKAGES="atk"
> -
> - PKG_CHECK_MODULES(ATK, $ATK_PACKAGES)
> -
> ---- a/gtk/a11y/gtkaccessibility.c
> -+++ b/gtk/a11y/gtkaccessibility.c
> -@@ -37,10 +37,6 @@
> - #include <gtk/gtkcombobox.h>
> - #include <gtk/gtkaccessible.h>
> -
> --#ifdef GDK_WINDOWING_X11
> --#include <atk-bridge.h>
> --#endif
> --
> - static gboolean gail_focus_watcher      (GSignalInvocationHint *ihint,
> -                                          guint                  n_param_values,
> -                                          const GValue          *param_values,
> -@@ -987,9 +983,5 @@
> -   _gtk_accessibility_override_atk_util ();
> -   do_window_event_initialization ();
> -
> --#ifdef GDK_WINDOWING_X11
> --  atk_bridge_adaptor_init (NULL, NULL);
> --#endif
> --
> -   atk_misc_instance = g_object_new (GTK_TYPE_MISC_IMPL, NULL);
> - }
> diff --git a/package/libgtk3/Config.in b/package/libgtk3/Config.in
> index 12e64707bd..f14b9deaf8 100644
> --- a/package/libgtk3/Config.in
> +++ b/package/libgtk3/Config.in
> @@ -20,6 +20,7 @@ config BR2_PACKAGE_LIBGTK3
>  	depends on BR2_PACKAGE_HAS_LIBEGL_WAYLAND || \
>  		BR2_PACKAGE_HAS_LIBGL
>  	select BR2_PACKAGE_ATK
> +	select BR2_PACKAGE_AT_SPI2_ATK

Be careful here: AT_SPI2_ATK is only available with X11, so you can't
just select it like this.

>  	select BR2_PACKAGE_CAIRO
>  	select BR2_PACKAGE_CAIRO_PS
>  	select BR2_PACKAGE_CAIRO_PDF
> diff --git a/package/libgtk3/libgtk3.mk b/package/libgtk3/libgtk3.mk
> index 1b85d00aae..e3c3dc5237 100644
> --- a/package/libgtk3/libgtk3.mk
> +++ b/package/libgtk3/libgtk3.mk
> @@ -26,7 +26,8 @@ LIBGTK3_CONF_OPTS = \
>  LIBGTK3_MAKE_OPTS = \
>  	WAYLAND_PROTOCOLS_DATADIR=$(STAGING_DIR)/usr/share/wayland-protocols
>  
> -LIBGTK3_DEPENDENCIES = host-pkgconf host-libgtk3 atk libglib2 cairo pango gdk-pixbuf libepoxy
> +LIBGTK3_DEPENDENCIES = host-pkgconf host-libgtk3 atk at-spi2-atk libglib2 \
> +		       cairo pango gdk-pixbuf libepoxy

And its dependency should only be added..

>  
>  ifeq ($(BR2_PACKAGE_LIBGTK3_X11),y)
>  LIBGTK3_DEPENDENCIES += fontconfig xlib_libX11 xlib_libXext xlib_libXrender xlib_libXi

... when the X11 backend is used.

But again, it would be nicer if the dependency was kept optional.

Best regards,

Thomas
Joseph Kogut June 20, 2018, 6:05 p.m. UTC | #2 
On Fri, Jun 15, 2018 at 12:53 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello,
>
> On Thu, 14 Jun 2018 15:48:15 -0700, Joseph Kogut wrote:
> > Signed-off-by: Joseph Kogut <joseph.kogut@gmail.com>
> > ---
> >  package/libgtk3/0003-disable-atk-bridge.patch | 45 -------------------
> >  package/libgtk3/Config.in                     |  1 +
> >  package/libgtk3/libgtk3.mk                    |  3 +-
> >  3 files changed, 3 insertions(+), 46 deletions(-)
> >  delete mode 100644 package/libgtk3/0003-disable-atk-bridge.patch
> >
> > diff --git a/package/libgtk3/0003-disable-atk-bridge.patch b/package/libgtk3/0003-disable-atk-bridge.patch
> > deleted file mode 100644
> > index c92174f400..0000000000
> > --- a/package/libgtk3/0003-disable-atk-bridge.patch
> > +++ /dev/null
> > @@ -1,45 +0,0 @@
> > -Remove atk-bridge support.
> > -
> > -atk-bridge doesn't seem useful for now in Buildroot and requires to
> > -add two new packages just for it: at-spi2-core and at-spi2-atk.
> > -
> > -Signed-off-by: Hadrien Boutteville <hadrien.boutteville@gmail.com>
>
> Could we instead keep them optional, by improving this patch ? We
> really try to keep dependencies to a reasonable level in Buildroot, so
> it would be nice if we could avoid making those mandatory dependencies
> of libgtk3.
>

That's a great point, and it's something I hadn't thought of. Would
you recommend selectively applying the patch to disable the dependency
on atk-bridge, or some other approach?

Are there any examples of selectively applying patches that I can use
as a reference?

> > -
> > ---- a/configure.ac
> > -+++ b/configure.ac
> > -@@ -1349,11 +1349,7 @@
> > - # Check for Accessibility Toolkit flags
> > - ########################################
> > -
> > --if test x$enable_x11_backend = xyes; then
> > --   ATK_PACKAGES="atk atk-bridge-2.0"
> > --else
> > --   ATK_PACKAGES="atk"
> > --fi
> > -+ATK_PACKAGES="atk"
> > -
> > - PKG_CHECK_MODULES(ATK, $ATK_PACKAGES)
> > -
> > ---- a/gtk/a11y/gtkaccessibility.c
> > -+++ b/gtk/a11y/gtkaccessibility.c
> > -@@ -37,10 +37,6 @@
> > - #include <gtk/gtkcombobox.h>
> > - #include <gtk/gtkaccessible.h>
> > -
> > --#ifdef GDK_WINDOWING_X11
> > --#include <atk-bridge.h>
> > --#endif
> > --
> > - static gboolean gail_focus_watcher      (GSignalInvocationHint *ihint,
> > -                                          guint                  n_param_values,
> > -                                          const GValue          *param_values,
> > -@@ -987,9 +983,5 @@
> > -   _gtk_accessibility_override_atk_util ();
> > -   do_window_event_initialization ();
> > -
> > --#ifdef GDK_WINDOWING_X11
> > --  atk_bridge_adaptor_init (NULL, NULL);
> > --#endif
> > --
> > -   atk_misc_instance = g_object_new (GTK_TYPE_MISC_IMPL, NULL);
> > - }
> > diff --git a/package/libgtk3/Config.in b/package/libgtk3/Config.in
> > index 12e64707bd..f14b9deaf8 100644
> > --- a/package/libgtk3/Config.in
> > +++ b/package/libgtk3/Config.in
> > @@ -20,6 +20,7 @@ config BR2_PACKAGE_LIBGTK3
> >       depends on BR2_PACKAGE_HAS_LIBEGL_WAYLAND || \
> >               BR2_PACKAGE_HAS_LIBGL
> >       select BR2_PACKAGE_ATK
> > +     select BR2_PACKAGE_AT_SPI2_ATK
>
> Be careful here: AT_SPI2_ATK is only available with X11, so you can't
> just select it like this.
>
> >       select BR2_PACKAGE_CAIRO
> >       select BR2_PACKAGE_CAIRO_PS
> >       select BR2_PACKAGE_CAIRO_PDF
> > diff --git a/package/libgtk3/libgtk3.mk b/package/libgtk3/libgtk3.mk
> > index 1b85d00aae..e3c3dc5237 100644
> > --- a/package/libgtk3/libgtk3.mk
> > +++ b/package/libgtk3/libgtk3.mk
> > @@ -26,7 +26,8 @@ LIBGTK3_CONF_OPTS = \
> >  LIBGTK3_MAKE_OPTS = \
> >       WAYLAND_PROTOCOLS_DATADIR=$(STAGING_DIR)/usr/share/wayland-protocols
> >
> > -LIBGTK3_DEPENDENCIES = host-pkgconf host-libgtk3 atk libglib2 cairo pango gdk-pixbuf libepoxy
> > +LIBGTK3_DEPENDENCIES = host-pkgconf host-libgtk3 atk at-spi2-atk libglib2 \
> > +                    cairo pango gdk-pixbuf libepoxy
>
> And its dependency should only be added..
>
> >
> >  ifeq ($(BR2_PACKAGE_LIBGTK3_X11),y)
> >  LIBGTK3_DEPENDENCIES += fontconfig xlib_libX11 xlib_libXext xlib_libXrender xlib_libXi
>
> ... when the X11 backend is used.
>
> But again, it would be nicer if the dependency was kept optional.
>
> Best regards,
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> https://bootlin.com

Best,
Joseph
Thomas Petazzoni June 21, 2018, 8:48 a.m. UTC | #3 
Hello,

On Wed, 20 Jun 2018 11:05:03 -0700, Joseph Kogut wrote:

> > Could we instead keep them optional, by improving this patch ? We
> > really try to keep dependencies to a reasonable level in Buildroot, so
> > it would be nice if we could avoid making those mandatory dependencies
> > of libgtk3.
> >  
> 
> That's a great point, and it's something I hadn't thought of. Would
> you recommend selectively applying the patch to disable the dependency
> on atk-bridge, or some other approach?
> 
> Are there any examples of selectively applying patches that I can use
> as a reference?

We don't want to apply patches conditionally. Instead the patch
*itself* should be smarter, and only use atk if available.

> > > ---- a/configure.ac
> > > -+++ b/configure.ac
> > > -@@ -1349,11 +1349,7 @@
> > > - # Check for Accessibility Toolkit flags
> > > - ########################################
> > > -
> > > --if test x$enable_x11_backend = xyes; then
> > > --   ATK_PACKAGES="atk atk-bridge-2.0"
> > > --else
> > > --   ATK_PACKAGES="atk"
> > > --fi
> > > -+ATK_PACKAGES="atk"
> > > -
> > > - PKG_CHECK_MODULES(ATK, $ATK_PACKAGES)

A rough idea is something like this:

PKG_CHECK_MODULES(ATK, atk)

if test x$enable_x11_backend = xyes; then
	PKG_CHECK_MODULES(ATK_BRIDGE, atk-bridge-2.0,
			  AC_DEFINE([HAVE_ATK_BRIDGE], [1]); ATK_BRIDGE_PACKAGE=atk-bridge-2.0)
fi

In configure.ac, add $ATK_BRIDGE_PACKAGE in:

GTK_PRIVATE_PACKAGES="$ATK_PACKAGES $WAYLAND_PACKAGES $MIR_PACKAGES epoxy >= epoxy_required_version"

And in gtkaccessibility.c, use #ifdef HAVE_ATK_BRIDGE instead of #ifdef
GDK_WINDOWING_X11 to guard the include of <atk-bridge.h> and the call
to atk_bridge_adaptor_init().

Of course, those are rough guidelines, you will probably need to adjust
stuff to actually make it work :-)

Best regards,

Thomas Petazzoni
diff mbox series

Patch

diff --git a/package/libgtk3/0003-disable-atk-bridge.patch b/package/libgtk3/0003-disable-atk-bridge.patch
deleted file mode 100644
index c92174f400..0000000000
--- a/package/libgtk3/0003-disable-atk-bridge.patch
+++ /dev/null
@@ -1,45 +0,0 @@ 
-Remove atk-bridge support.
-
-atk-bridge doesn't seem useful for now in Buildroot and requires to
-add two new packages just for it: at-spi2-core and at-spi2-atk.
-
-Signed-off-by: Hadrien Boutteville <hadrien.boutteville@gmail.com>
-
---- a/configure.ac
-+++ b/configure.ac
-@@ -1349,11 +1349,7 @@
- # Check for Accessibility Toolkit flags
- ########################################
-
--if test x$enable_x11_backend = xyes; then
--   ATK_PACKAGES="atk atk-bridge-2.0"
--else
--   ATK_PACKAGES="atk"
--fi
-+ATK_PACKAGES="atk"
-
- PKG_CHECK_MODULES(ATK, $ATK_PACKAGES)
-
---- a/gtk/a11y/gtkaccessibility.c
-+++ b/gtk/a11y/gtkaccessibility.c
-@@ -37,10 +37,6 @@
- #include <gtk/gtkcombobox.h>
- #include <gtk/gtkaccessible.h>
-
--#ifdef GDK_WINDOWING_X11
--#include <atk-bridge.h>
--#endif
--
- static gboolean gail_focus_watcher      (GSignalInvocationHint *ihint,
-                                          guint                  n_param_values,
-                                          const GValue          *param_values,
-@@ -987,9 +983,5 @@
-   _gtk_accessibility_override_atk_util ();
-   do_window_event_initialization ();
-
--#ifdef GDK_WINDOWING_X11
--  atk_bridge_adaptor_init (NULL, NULL);
--#endif
--
-   atk_misc_instance = g_object_new (GTK_TYPE_MISC_IMPL, NULL);
- }
diff --git a/package/libgtk3/Config.in b/package/libgtk3/Config.in
index 12e64707bd..f14b9deaf8 100644
--- a/package/libgtk3/Config.in
+++ b/package/libgtk3/Config.in
@@ -20,6 +20,7 @@  config BR2_PACKAGE_LIBGTK3
 	depends on BR2_PACKAGE_HAS_LIBEGL_WAYLAND || \
 		BR2_PACKAGE_HAS_LIBGL
 	select BR2_PACKAGE_ATK
+	select BR2_PACKAGE_AT_SPI2_ATK
 	select BR2_PACKAGE_CAIRO
 	select BR2_PACKAGE_CAIRO_PS
 	select BR2_PACKAGE_CAIRO_PDF
diff --git a/package/libgtk3/libgtk3.mk b/package/libgtk3/libgtk3.mk
index 1b85d00aae..e3c3dc5237 100644
--- a/package/libgtk3/libgtk3.mk
+++ b/package/libgtk3/libgtk3.mk
@@ -26,7 +26,8 @@  LIBGTK3_CONF_OPTS = \
 LIBGTK3_MAKE_OPTS = \
 	WAYLAND_PROTOCOLS_DATADIR=$(STAGING_DIR)/usr/share/wayland-protocols
 
-LIBGTK3_DEPENDENCIES = host-pkgconf host-libgtk3 atk libglib2 cairo pango gdk-pixbuf libepoxy
+LIBGTK3_DEPENDENCIES = host-pkgconf host-libgtk3 atk at-spi2-atk libglib2 \
+		       cairo pango gdk-pixbuf libepoxy
 
 ifeq ($(BR2_PACKAGE_LIBGTK3_X11),y)
 LIBGTK3_DEPENDENCIES += fontconfig xlib_libX11 xlib_libXext xlib_libXrender xlib_libXi