frame

Merge lp:~dandrader/frame/lp1080819 into lp:frame

  1. lp1080819
  2. Merge into trunk
Proposed by Daniel d'Andrada
Status: Merged
Merged at revision: 103
Proposed branch: lp:~dandrader/frame/lp1080819
Merge into: lp:frame
Diff against target: 399 lines (+139/-30)
12 files modified
.bzrignore (+1/-0)
Makefile.am (+10/-1)
configure.ac (+61/-2)
frame-x11.pc.in (+9/-0)
include/oif/frame.h.in (+4/-0)
src/Makefile.am (+10/-2)
src/value.h (+0/-2)
test/Makefile.am (+11/-2)
test/regular/Makefile.am (+17/-5)
test/regular/accept-ended-touch.cpp (+2/-2)
test/regular/frame-x11-fixture.cpp (+10/-10)
test/regular/frame-x11-fixture.h (+4/-4)
To merge this branch: bzr merge lp:~dandrader/frame/lp1080819
Reviewer Review Type Date Requested Status
Stephen M. Webb (community) Approve
Review via email: mp+135162@code.launchpad.net

Description of the change

Fixes bug 1080819.

To post a comment you must log in.
lp:~dandrader/frame/lp1080819 updated
104. By Daniel d'Andrada

Add frame-x11.pc

So that configure scripts can know if the installed frame has X11 support.

105. By Daniel d'Andrada

Remove stray #include

Revision history for this message
Stephen M. Webb (bregma) wrote :

I approve of most of these changes in general, with the following exceptions.

(1) as far as I know, it is not an error to mention a symbol name in a GNU linker version script and that symbol is not present. That means there is no need to run the preprocessor on the version script; a single version script should suffice regardless of package build options selected. Less to go wrong.

(2) You should avoid embedding device-specific control in the diagnostic output of configure. The right way to get pretty colour into the diagnostic output is to use the tput program. For example,

  bold_green=$(tput bold)$(tput setf 2)
  bold_white=$(tput bold)$(tput setf 7)
  reset=$(tput sgr0)

You should also use AC_MSG_NOTICE to send notices to the diagnostic output because (a) it send it to all the right places (echo only sends to stdout), and (2) the autoconf tool facilities react properly to command-line options like --quiet.

review: Needs Fixing
lp:~dandrader/frame/lp1080819 updated
106. By Daniel d'Andrada

Improve implementation of the pretty summary of the configure script

- Do not use device-specific terminal commands directly
- Use AC_MSG_NOTICE instead of echo to ensure the messages go to the right places
  and obey configure options such as --quiet

107. By Daniel d'Andrada

No need to configure libframe.ver

Fixed according to review comments from Stephen Webb:
"It is not an error to mention a symbol name in a GNU linker version script and
that symbol is not present. That means there is no need to run the preprocessor
on the version script. A single version script should suffice regardless of
package build options selected."

Revision history for this message
Daniel d'Andrada (dandrader) wrote :

Updated according to comments.

Revision history for this message
Stephen M. Webb (bregma) wrote :

include/oif/frame_x11.h will not get packaged by 'make dist' if not configured to use X11. This is unlikely to be a real problem in production, but if there's a way to work around it that would be good.

Other than that warning, this looks good.

review: Approve
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

That's odd, I've run "./configure --disable-x11" followed by "make dist" and the generated tarball did include frame_x11.h

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file '.bzrignore'
--- .bzrignore 2012-08-31 19:39:22 +0000
+++ .bzrignore 2012-11-26 20:02:21 +0000
@@ -19,6 +19,7 @@
19coveragereport19coveragereport
20doc/api20doc/api
21libtool21libtool
22include/oif/frame.h
22m4/libtool.m423m4/libtool.m4
23m4/ltoptions.m424m4/ltoptions.m4
24m4/ltsugar.m425m4/ltsugar.m4
2526
=== modified file 'Makefile.am'
--- Makefile.am 2012-07-25 22:43:17 +0000
+++ Makefile.am 2012-11-26 20:02:21 +0000
@@ -1,4 +1,9 @@
1SUBDIRS = doc src tools test1SUBDIRS = doc src test
2
3# tools are all X11 based
4if HAVE_XINPUT
5SUBDIRS += tools
6endif
27
3ACLOCAL_AMFLAGS = -I m4 --install8ACLOCAL_AMFLAGS = -I m4 --install
49
@@ -9,6 +14,10 @@
9pkgconfigdir = $(libdir)/pkgconfig14pkgconfigdir = $(libdir)/pkgconfig
10pkgconfig_DATA = frame.pc15pkgconfig_DATA = frame.pc
1116
17if HAVE_XINPUT
18pkgconfig_DATA += frame-x11.pc
19endif
20
12if HAVE_GCOV21if HAVE_GCOV
13 .PHONY: clean-gcda22 .PHONY: clean-gcda
14 clean-gcda:23 clean-gcda:
1524
=== modified file 'configure.ac'
--- configure.ac 2012-11-08 15:55:43 +0000
+++ configure.ac 2012-11-26 20:02:21 +0000
@@ -25,7 +25,48 @@
25AC_PROG_CXX25AC_PROG_CXX
26AC_PROG_INSTALL26AC_PROG_INSTALL
2727
28PKG_CHECK_MODULES(XINPUT, x11 xext xorg-server [xi >= 1.5.99.1] [inputproto >= 2.1.99.6])28
29# *** Enable/disable the X11 backend ***
30AC_ARG_ENABLE(x11,
31 AC_HELP_STRING(--disable-x11, Do not build the X11 backend. (default: enable if available)),
32 [],
33 [enable_x11=auto])
34
35
36# Check for X11 as an optional dependency
37AS_IF([test "x$enable_x11" = "xauto"],
38 [
39 PKG_CHECK_MODULES(XINPUT,
40 x11 xext xorg-server [xi >= 1.5.99.1] [inputproto >= 2.1.99.6],
41 [have_xinput=yes],
42 [have_xinput=no])
43 ])
44
45AS_IF([test "x$enable_x11" = "xyes"],
46 [
47 PKG_CHECK_MODULES(XINPUT,
48 x11 xext xorg-server [xi >= 1.5.99.1] [inputproto >= 2.1.99.6],
49 [have_xinput=yes],
50 [
51 AC_MSG_ERROR([x11 and/or XInput libraries not found!])
52 have_xinput=no
53 ])
54 ])
55
56AS_IF([test "x$enable_x11" = "xno"],
57 [have_xinput=no])
58
59AS_IF([test "x$have_xinput" = "xyes"],
60 [
61 FRAME_X11_BACKEND="FRAME_X11_BACKEND"
62 AM_CONDITIONAL(HAVE_XINPUT, true)
63 ],
64 [
65 FRAME_X11_BACKEND="FRAME_NO_X11_BACKEND"
66 AM_CONDITIONAL(HAVE_XINPUT, false)
67 ])
68AC_SUBST(FRAME_X11_BACKEND)
69AC_SUBST(HAVE_XINPUT)
2970
30AC_CHECK_PROG([ASCIIDOC], [a2x], [a2x])71AC_CHECK_PROG([ASCIIDOC], [a2x], [a2x])
31AM_CONDITIONAL([HAVE_DOCTOOLS], [test "x$ASCIIDOC" != "x"])72AM_CONDITIONAL([HAVE_DOCTOOLS], [test "x$ASCIIDOC" != "x"])
@@ -47,7 +88,8 @@
47AS_IF([test "x$enable_integration_tests" != xno],88AS_IF([test "x$enable_integration_tests" != xno],
48 [CHECK_XORG_GTEST(89 [CHECK_XORG_GTEST(
49 [AC_MSG_NOTICE([xorg-gtest is available, integration tests will be built])],90 [AC_MSG_NOTICE([xorg-gtest is available, integration tests will be built])],
50 [AC_MSG_WARN([xorg-gtest is not available, tests will not be built])])])91 [AC_MSG_WARN([xorg-gtest is not available, tests will not be built])])],
92 [have_xorg_gtest=no])
5193
52AM_CONDITIONAL([ENABLE_INTEGRATION_TESTS], [test "x$have_xorg_gtest" = xyes])94AM_CONDITIONAL([ENABLE_INTEGRATION_TESTS], [test "x$have_xorg_gtest" = xyes])
5395
@@ -65,6 +107,7 @@
65107
66AC_CONFIG_FILES([Makefile108AC_CONFIG_FILES([Makefile
67 doc/Makefile109 doc/Makefile
110 include/oif/frame.h
68 src/Makefile111 src/Makefile
69 test/Makefile112 test/Makefile
70 test/gtest/Makefile113 test/gtest/Makefile
@@ -74,6 +117,9 @@
74 tools/Makefile117 tools/Makefile
75 frame.pc])118 frame.pc])
76119
120AS_IF([test "x$have_xinput" = "xyes"],
121 [AC_CONFIG_FILES([frame-x11.pc])])
122
77AC_LANG_PUSH(C++)123AC_LANG_PUSH(C++)
78AC_CHECK_DECL(__clang__, CXX_LIBS="-lstdc++")124AC_CHECK_DECL(__clang__, CXX_LIBS="-lstdc++")
79AC_SUBST(CXX_LIBS)125AC_SUBST(CXX_LIBS)
@@ -86,3 +132,16 @@
86AC_SUBST(VISIBILITY_CXXFLAGS, "-fvisibility=hidden")132AC_SUBST(VISIBILITY_CXXFLAGS, "-fvisibility=hidden")
87133
88AC_OUTPUT134AC_OUTPUT
135
136bold_green=$(tput bold)$(tput setf 2)
137bold_white=$(tput bold)$(tput setf 7)
138reset=$(tput sgr0)
139
140AC_MSG_NOTICE([])
141AC_MSG_NOTICE([${bold_green}Open Input Framework - Frame library - $VERSION${reset}])
142AC_MSG_NOTICE([])
143AC_MSG_NOTICE([ Prefix : ${bold_white}${prefix}${reset}])
144AC_MSG_NOTICE([ CFLAGS : ${bold_white}${CFLAGS} ${GCC_FLAGS}${reset}])
145AC_MSG_NOTICE([ Build X11 backend : ${bold_white}${have_xinput}${reset}])
146AC_MSG_NOTICE([ Build integration tests : ${bold_white}${have_xorg_gtest}${reset}])
147AC_MSG_NOTICE([])
89148
=== added file 'frame-x11.pc.in'
--- frame-x11.pc.in 1970-01-01 00:00:00 +0000
+++ frame-x11.pc.in 2012-11-26 20:02:21 +0000
@@ -0,0 +1,9 @@
1prefix=@prefix@
2exec_prefix=@exec_prefix@
3libdir=@libdir@
4includedir=@includedir@
5
6Name: frame-x11
7Description: Touch Frame Library - X11 support
8Version: @PACKAGE_VERSION@
9Requires: frame = @PACKAGE_VERSION@
010
=== renamed file 'include/oif/frame.h' => 'include/oif/frame.h.in'
--- include/oif/frame.h 2012-08-31 17:04:43 +0000
+++ include/oif/frame.h.in 2012-11-26 20:02:21 +0000
@@ -52,6 +52,10 @@
52#endif // __has_feature52#endif // __has_feature
53#endif // __has_feature(c_generic_selections)53#endif // __has_feature(c_generic_selections)
5454
55/* Whether the X11 backend (frame_x11.h) is available.
56 FRAME_X11_BACKEND will be defined if it is and FRAME_NO_X11_BACKEND otherwise. */
57#define @FRAME_X11_BACKEND@
58
55#ifdef __cplusplus59#ifdef __cplusplus
56extern "C" {60extern "C" {
57#endif61#endif
5862
=== modified file 'src/Makefile.am'
--- src/Makefile.am 2012-11-08 11:21:24 +0000
+++ src/Makefile.am 2012-11-26 20:02:21 +0000
@@ -31,8 +31,12 @@
31libframeinclude_HEADERS = \31libframeinclude_HEADERS = \
32 $(top_srcdir)/include/oif/frame.h \32 $(top_srcdir)/include/oif/frame.h \
33 $(top_srcdir)/include/oif/frame_backend.h \33 $(top_srcdir)/include/oif/frame_backend.h \
34 $(top_srcdir)/include/oif/frame_internal.h \34 $(top_srcdir)/include/oif/frame_internal.h
35
36if HAVE_XINPUT
37libframeinclude_HEADERS += \
35 $(top_srcdir)/include/oif/frame_x11.h38 $(top_srcdir)/include/oif/frame_x11.h
39endif
3640
37libframe_la_SOURCES = \41libframe_la_SOURCES = \
38 axis.h \42 axis.h \
@@ -52,7 +56,10 @@
52 value.h \56 value.h \
53 value.cpp \57 value.cpp \
54 window.h \58 window.h \
55 window.cpp \59 window.cpp
60
61if HAVE_XINPUT
62libframe_la_SOURCES += \
56 x11/device_x11.h \63 x11/device_x11.h \
57 x11/device_x11.cpp \64 x11/device_x11.cpp \
58 x11/frame_x11.cpp \65 x11/frame_x11.cpp \
@@ -60,5 +67,6 @@
60 x11/handle_x11.cpp \67 x11/handle_x11.cpp \
61 x11/window_x11.h \68 x11/window_x11.h \
62 x11/window_x11.cpp69 x11/window_x11.cpp
70endif
6371
64EXTRA_DIST = $(version_script)72EXTRA_DIST = $(version_script)
6573
=== modified file 'src/value.h'
--- src/value.h 2012-06-21 19:41:40 +0000
+++ src/value.h 2012-11-26 20:02:21 +0000
@@ -25,8 +25,6 @@
2525
26#include <memory>26#include <memory>
2727
28#include <X11/X.h>
29
30#include "oif/frame.h"28#include "oif/frame.h"
31#include "typedefs.h"29#include "typedefs.h"
3230
3331
=== modified file 'test/Makefile.am'
--- test/Makefile.am 2012-07-17 13:57:32 +0000
+++ test/Makefile.am 2012-11-26 20:02:21 +0000
@@ -1,5 +1,14 @@
1SUBDIRS = gtest x11_mocks regular1SUBDIRS = gtest
22
3if HAVE_XINPUT
4SUBDIRS += x11_mocks
5endif
6
7SUBDIRS += regular
8
9# integration tests are all X11 based
10if HAVE_XINPUT
3if ENABLE_INTEGRATION_TESTS11if ENABLE_INTEGRATION_TESTS
4SUBDIRS += integration12SUBDIRS += integration
5endif13endif
14endif
615
=== modified file 'test/regular/Makefile.am'
--- test/regular/Makefile.am 2012-08-24 16:30:05 +0000
+++ test/regular/Makefile.am 2012-11-26 20:02:21 +0000
@@ -1,4 +1,8 @@
1test_targets = check-c-compile check-cxx-compile1test_targets =
2
3if HAVE_XINPUT
4test_targets += check-c-compile check-cxx-compile
5endif
26
3if HAVE_GTEST7if HAVE_GTEST
4test_targets += check-regular8test_targets += check-regular
@@ -37,10 +41,14 @@
37### check-regular41### check-regular
3842
39check_regular_SOURCES = \43check_regular_SOURCES = \
44 backend.cpp
45
46if HAVE_XINPUT
47check_regular_SOURCES += \
40 accept-ended-touch.cpp \48 accept-ended-touch.cpp \
41 backend.cpp \49 frame-x11-fixture.cpp \
42 frame-fixture.cpp \50 frame-x11-fixture.h
43 frame-fixture.h51endif
4452
45#53#
46# Link against the (non-distributed) static lib to pick up the54# Link against the (non-distributed) static lib to pick up the
@@ -50,10 +58,14 @@
50 $(top_builddir)/src/.libs/libframe.a \58 $(top_builddir)/src/.libs/libframe.a \
51 $(top_builddir)/test/gtest/libgtest.a \59 $(top_builddir)/test/gtest/libgtest.a \
52 $(top_builddir)/test/gtest/libgtest_main.a \60 $(top_builddir)/test/gtest/libgtest_main.a \
53 $(top_builddir)/test/x11_mocks/libx11_mocks.a \
54 $(COVERAGE_LIBS) \61 $(COVERAGE_LIBS) \
55 $(GTEST_LIBS)62 $(GTEST_LIBS)
5663
64if HAVE_XINPUT
65check_regular_LDADD += \
66 $(top_builddir)/test/x11_mocks/libx11_mocks.a
67endif
68
57check_regular_CPPFLAGS = \69check_regular_CPPFLAGS = \
58 --std=c++0x \70 --std=c++0x \
59 -I$(top_srcdir)/test/x11_mocks \71 -I$(top_srcdir)/test/x11_mocks \
6072
=== modified file 'test/regular/accept-ended-touch.cpp'
--- test/regular/accept-ended-touch.cpp 2012-07-17 14:27:23 +0000
+++ test/regular/accept-ended-touch.cpp 2012-11-26 20:02:21 +0000
@@ -1,7 +1,7 @@
1#include "frame-fixture.h"1#include "frame-x11-fixture.h"
2#include "x11_mocks.h"2#include "x11_mocks.h"
33
4class AcceptEndedTouch : public FrameFixture4class AcceptEndedTouch : public FrameX11Fixture
5{5{
6};6};
77
88
=== renamed file 'test/regular/frame-fixture.cpp' => 'test/regular/frame-x11-fixture.cpp'
--- test/regular/frame-fixture.cpp 2012-08-31 19:45:35 +0000
+++ test/regular/frame-x11-fixture.cpp 2012-11-26 20:02:21 +0000
@@ -1,23 +1,23 @@
1#include "frame-fixture.h"1#include "frame-x11-fixture.h"
2#include "x11_mocks.h"2#include "x11_mocks.h"
33
4FrameFixture::FrameFixture()4FrameX11Fixture::FrameX11Fixture()
5 : frame_handle(nullptr),5 : frame_handle(nullptr),
6 _xevent_serial_number(1)6 _xevent_serial_number(1)
7{7{
8}8}
99
10void FrameFixture::SetUp()10void FrameX11Fixture::SetUp()
11{11{
12 xmock_touch_acceptance.clear();12 xmock_touch_acceptance.clear();
13}13}
1414
15void FrameFixture::TearDown()15void FrameX11Fixture::TearDown()
16{16{
17 ASSERT_EQ(nullptr, frame_handle);17 ASSERT_EQ(nullptr, frame_handle);
18}18}
1919
20void FrameFixture::CreateXMockTouchScreenDevice()20void FrameX11Fixture::CreateXMockTouchScreenDevice()
21{21{
22 xmock_devices_count = 1;22 xmock_devices_count = 1;
23 xmock_devices = (XIDeviceInfo*) calloc(xmock_devices_count,23 xmock_devices = (XIDeviceInfo*) calloc(xmock_devices_count,
@@ -59,7 +59,7 @@
59 xmock_devices[0].classes = classes;59 xmock_devices[0].classes = classes;
60}60}
6161
62void FrameFixture::DestroyXMockDevices()62void FrameX11Fixture::DestroyXMockDevices()
63{63{
64 for (int i = 0; i < xmock_devices_count; ++i)64 for (int i = 0; i < xmock_devices_count; ++i)
65 {65 {
@@ -70,7 +70,7 @@
70 free(xmock_devices);70 free(xmock_devices);
71}71}
7272
73void FrameFixture::SendTouchEvent(73void FrameX11Fixture::SendTouchEvent(
74 int event_type, int touch_id, float x, float y)74 int event_type, int touch_id, float x, float y)
75{75{
76 UFStatus status;76 UFStatus status;
@@ -112,7 +112,7 @@
112 free(device_event);112 free(device_event);
113}113}
114114
115void FrameFixture::SendTouchOwnershipEvent(int touch_id)115void FrameX11Fixture::SendTouchOwnershipEvent(int touch_id)
116{116{
117 UFStatus status;117 UFStatus status;
118 XGenericEventCookie xcookie;118 XGenericEventCookie xcookie;
@@ -141,7 +141,7 @@
141 free(ownership_event);141 free(ownership_event);
142}142}
143143
144void FrameFixture::FetchDeviceAddedEvent(UFDevice *device)144void FrameX11Fixture::FetchDeviceAddedEvent(UFDevice *device)
145{145{
146 UFEvent event;146 UFEvent event;
147 UFStatus status;147 UFStatus status;
@@ -157,7 +157,7 @@
157 frame_event_unref(event);157 frame_event_unref(event);
158}158}
159159
160void FrameFixture::AssertNoMoreEvents()160void FrameX11Fixture::AssertNoMoreEvents()
161{161{
162 UFEvent event;162 UFEvent event;
163 UFStatus status;163 UFStatus status;
164164
=== renamed file 'test/regular/frame-fixture.h' => 'test/regular/frame-x11-fixture.h'
--- test/regular/frame-fixture.h 2012-07-24 20:47:15 +0000
+++ test/regular/frame-x11-fixture.h 2012-11-26 20:02:21 +0000
@@ -1,14 +1,14 @@
1#ifndef GTEST_FRAME_FIXTURE_H1#ifndef GTEST_FRAME_X11_FIXTURE_H
2#define GTEST_FRAME_FIXTURE_H2#define GTEST_FRAME_X11_FIXTURE_H
33
4#include <gtest/gtest.h>4#include <gtest/gtest.h>
5#include "oif/frame.h"5#include "oif/frame.h"
6#include "oif/frame_x11.h"6#include "oif/frame_x11.h"
77
8class FrameFixture : public ::testing::Test8class FrameX11Fixture : public ::testing::Test
9{9{
10 protected:10 protected:
11 FrameFixture();11 FrameX11Fixture();
1212
13 virtual void SetUp();13 virtual void SetUp();
14 virtual void TearDown();14 virtual void TearDown();

Subscribers

People subscribed via source and target branches