Merge lp:~hasselmm/appmenu-gtk/gtk3 into lp:appmenu-gtk/0.4

Proposed by Mathias Hasselmann
Status: Merged
Approved by: Michael Terry
Approved revision: 140
Merged at revision: 135
Proposed branch: lp:~hasselmm/appmenu-gtk/gtk3
Merge into: lp:appmenu-gtk/0.4
Diff against target: 154 lines (+43/-17)
5 files modified
80appmenu.in (+1/-1)
Makefile.am (+5/-5)
configure.ac (+27/-6)
src/Makefile.am (+4/-5)
src/bridge.c (+6/-0)
To merge this branch: bzr merge lp:~hasselmm/appmenu-gtk/gtk3
Reviewer Review Type Date Requested Status
Michael Terry Approve
Mathias Hasselmann (community) Needs Resubmitting
Review via email: mp+60326@code.launchpad.net

This proposal supersedes a proposal from 2011-05-08.

Description of the change

This permits building of a gtk3 version of appmenu-gtk.

To post a comment you must log in.
lp:~hasselmm/appmenu-gtk/gtk3 updated
137. By Mathias Hasselmann

Generate proper X session script for both gtk flavors

Revision history for this message
Mathias Hasselmann (hasselmm) wrote :
Revision history for this message
Mathias Hasselmann (hasselmm) wrote :
Revision history for this message
Michael Terry (mterry) wrote :

Looks great. I have two comments:

1) Do we really need --with-gtk3? Seems like --with-gtk2 is enough.

2) You need to add the following in src/bridge.c:

+#if GTK_CHECK_VERSION(3, 0, 0)
+#include <libdbusmenu-gtk3/menuitem.h>
+#include <libdbusmenu-gtk3/parser.h>
+#else
 #include <libdbusmenu-gtk/menuitem.h>
 #include <libdbusmenu-gtk/parser.h>
+#endif

Unfortunately (and this is my fault), the headers use different directories for gtk2/gtk3.

review: Needs Fixing
lp:~hasselmm/appmenu-gtk/gtk3 updated
138. By Mathias Hasselmann

Include proper appmenu-gtk headers.

139. By Mathias Hasselmann

Drop somewhat redundant --with-gtk3 switch.

140. By Mathias Hasselmann

Bump package version

Revision history for this message
Mathias Hasselmann (hasselmm) wrote :

> 1) Do we really need --with-gtk3? Seems like --with-gtk2 is enough.

Dropped.

> 2) You need to add the following in src/bridge.c:

Fixed.

> Unfortunately (and this is my fault), the headers use different directories
> for gtk2/gtk3.

Maybe still can be fixed somehow? The files in that folders seem to be identical.

Revision history for this message
Mathias Hasselmann (hasselmm) :
review: Needs Resubmitting
Revision history for this message
Michael Terry (mterry) wrote :

Looks great! Thanks!

> Maybe still can be fixed somehow? The files in that folders seem to be identical.

A) Can't guarantee that there will never be differences in gtk2/gtk3 versions, though seems more on the 0.1% chance side.

B) We need both libdbusmenu-gtk3-dev and libdbusmenu-gtk-dev to ship the headers so that a package can depend on either one and not get both. So we have to ship two copies of them anyway (or split the headers into some -dev-common package).

C) The normal way of dealing with it is like so:

#include <libdbusmenu-gtk/header.h>
/usr/include/dbusmenu/libdbusmenu-gtk/header.h
/usr/include/dbusmenu3/libdbusmenu-gtk/header.h
And have the .pc file specify which directory to -I

But with libdbusmenu, it sticks libdbusmenu-glib in the same directory with libdbusmenu-gtk, so we either had to duplicate the -glib headers or stick gtk3 headers in the same directory too. I went with the latter, but I'm not convinced it was optimal. :)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file '80appmenu.in'
2--- 80appmenu.in 2011-02-14 15:02:09 +0000
3+++ 80appmenu.in 2011-05-11 19:06:46 +0000
4@@ -1,4 +1,4 @@
5-if [ -f @libdir@/gtk-2.0/2.10.0/menuproxies/libappmenu.so ]
6+if [ -f @moduledir@/menuproxies/libappmenu.so ]
7 then
8 export UBUNTU_MENUPROXY="libappmenu.so"
9 fi
10
11=== modified file 'Makefile.am'
12--- Makefile.am 2011-02-16 22:06:10 +0000
13+++ Makefile.am 2011-05-11 19:06:46 +0000
14@@ -8,17 +8,17 @@
15 build \
16 src
17
18-80appmenu: 80appmenu.in
19- sed -e "s|\@libdir\@|$(libdir)|" $< > $@
20+$(sessionfile): 80appmenu.in
21+ sed -e "s|\@moduledir\@|$(moduledir)|" $< > $@
22
23 xsessiondir = $(sysconfdir)/X11/Xsession.d
24
25-nodist_xsession_DATA = 80appmenu
26+nodist_xsession_DATA = $(sessionfile)
27
28 EXTRA_DIST = $(xsession_DATA) \
29 configure.ac \
30 Makefile.am \
31 autogen.sh \
32- 80appmenu.in
33+ 80appmenu.in
34
35-DISTCLEANFILES = 80appmenu
36+DISTCLEANFILES = $(sessionfile)
37
38=== modified file 'configure.ac'
39--- configure.ac 2011-04-14 20:57:14 +0000
40+++ configure.ac 2011-05-11 19:06:46 +0000
41@@ -1,6 +1,6 @@
42 AC_PREREQ(2.59)
43
44-AC_INIT([appmenu-gtk], [0.2.1], [crussell@canonical.com])
45+AC_INIT([appmenu-gtk], [0.3.0], [crussell@canonical.com])
46 AC_COPYRIGHT([Copyright 2010 Canonical])
47 AC_CONFIG_SRCDIR([src/bridge.c])
48 AC_CONFIG_MACRO_DIR([build/autotools])
49@@ -15,24 +15,44 @@
50 AM_PROG_CC_C_O
51 AC_STDC_HEADERS
52 AC_PROG_LIBTOOL
53+PKG_PROG_PKG_CONFIG
54
55 AC_SUBST(VERSION)
56
57 m4_ifdef([AM_SILENT_RULES],[AM_SILENT_RULES([yes])])
58
59 ###########################
60-# Dependencies - GLib
61+# Dependencies - Gtk
62 ###########################
63
64-GTK_REQUIRED_VERSION=2.18
65+gtk_api=3.0
66+
67+GTK2_REQUIRED_VERSION=2.18
68+GTK3_REQUIRED_VERSION=3.0
69 DBUSMENU_REQUIRED_VERSION=0.3.101
70
71-PKG_CHECK_MODULES(APPMENU, gtk+-2.0 >= $GTK_REQUIRED_VERSION
72- dbusmenu-gtk-0.4 >= $DBUSMENU_REQUIRED_VERSION)
73+AC_ARG_WITH(gtk2, [AS_HELP_STRING([--with-gtk2],[use 2.0 API of GTK+])],[gtk_api=2.0])
74+
75+if test x$gtk_api = x2.0; then
76+ PKG_CHECK_MODULES(APPMENU, gtk+-2.0 >= $GTK2_REQUIRED_VERSION
77+ dbusmenu-gtk-0.4 >= $DBUSMENU_REQUIRED_VERSION)
78+ moduledir='$(libdir)/gtk-2.0/2.10.0/menuproxies'
79+ sessionfile=80appmenu-gtk
80+elif test x$gtk_api = x3.0; then
81+ PKG_CHECK_MODULES(APPMENU, gtk+-3.0 >= $GTK3_REQUIRED_VERSION
82+ dbusmenu-gtk3-0.4 >= $DBUSMENU_REQUIRED_VERSION)
83+ moduledir='$(libdir)/gtk-3.0/3.0.0/menuproxies'
84+ sessionfile=80appmenu-gtk3
85+else
86+ AC_MSG_ERROR([unknown GTK+ API: $gtk_api])
87+fi
88
89 AC_SUBST(APPMENU_CFLAGS)
90 AC_SUBST(APPMENU_LIBS)
91
92+AC_SUBST(moduledir)
93+AC_SUBST(sessionfile)
94+
95 ###########################
96 # Files
97 ###########################
98@@ -58,6 +78,7 @@
99
100 appmenu-gtk configuration:
101
102- Prefix: $prefix
103+ Prefix: $prefix
104+ GTK+ API: $gtk_api
105 ])
106
107
108=== modified file 'src/Makefile.am'
109--- src/Makefile.am 2011-01-04 22:54:15 +0000
110+++ src/Makefile.am 2011-05-11 19:06:46 +0000
111@@ -3,7 +3,6 @@
112 BUILT_SOURCES =
113 EXTRA_DIST=
114
115-moduledir = $(libdir)/gtk-2.0/2.10.0/menuproxies
116 module_LTLIBRARIES = libappmenu.la
117
118 libappmenu_la_SOURCES = \
119@@ -37,13 +36,13 @@
120 $(GCC_FLAGS) \
121 $(MAINTAINER_CFLAGS)
122
123-gen-%.xml.h: %.xml
124+gen-%.xml.h: $(srcdir)/%.xml
125 @echo "Building $@ from $<"
126- @echo "extern const char * _$(subst -,_,$(subst .,_,$(basename $<)));" > $@
127+ @echo "extern const char * _$(subst -,_,$(subst .,_,$(basename $(notdir $<))));" > $@
128
129-gen-%.xml.c: %.xml
130+gen-%.xml.c: $(srcdir)/%.xml
131 @echo "Building $@ from $<"
132- @echo "const char * _$(subst -,_,$(subst .,_,$(basename $<))) = " > $@
133+ @echo "const char * _$(subst -,_,$(subst .,_,$(basename $(notdir $<)))) = " > $@
134 @sed -e "s:\":\\\\\":g" -e s:^:\": -e s:\$$:\\\\n\": $< >> $@
135 @echo ";" >> $@
136
137
138=== modified file 'src/bridge.c'
139--- src/bridge.c 2011-04-20 19:18:42 +0000
140+++ src/bridge.c 2011-05-11 19:06:46 +0000
141@@ -27,8 +27,14 @@
142 #include <gdk/gdkx.h>
143 #include <gio/gio.h>
144
145+#if GTK_CHECK_VERSION(3, 0, 0)
146+#include <libdbusmenu-gtk3/menuitem.h>
147+#include <libdbusmenu-gtk3/parser.h>
148+#else
149 #include <libdbusmenu-gtk/menuitem.h>
150 #include <libdbusmenu-gtk/parser.h>
151+#endif
152+
153 #include <libdbusmenu-glib/menuitem.h>
154 #include <libdbusmenu-glib/server.h>
155

Subscribers

People subscribed via source and target branches