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

[Daniel d'Andrada]

Fixes bug 1080819.

[lp:~dandrader/frame/lp1080819]

104. By Daniel d'Andrada on 2012-11-20

Add frame-x11.pc

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

105. By Daniel d'Andrada on 2012-11-21

Remove stray #include

[Stephen M. Webb (bregma)]

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]

106. By Daniel d'Andrada on 2012-11-26

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 on 2012-11-26

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."

[Daniel d'Andrada (dandrader)]

Updated according to comments.

[Stephen M. Webb (bregma)]

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

[Daniel d'Andrada (dandrader)]

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

=== modified file '.bzrignore'
--- .bzrignore	2012-08-31 19:39:22 +0000
+++ .bzrignore	2012-11-26 20:02:21 +0000
@@ -19,6 +19,7 @@
 coveragereport
 doc/api
 libtool
+include/oif/frame.h
 m4/libtool.m4
 m4/ltoptions.m4
 m4/ltsugar.m4

=== 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 @@
-SUBDIRS = doc src tools test
+SUBDIRS = doc src test
+
+# tools are all X11 based
+if HAVE_XINPUT
+SUBDIRS += tools 
+endif
 
 ACLOCAL_AMFLAGS = -I m4 --install
 
@@ -9,6 +14,10 @@
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = frame.pc
 
+if HAVE_XINPUT
+pkgconfig_DATA +=  frame-x11.pc
+endif
+
 if HAVE_GCOV
   .PHONY: clean-gcda
   clean-gcda:

=== 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 @@
 AC_PROG_CXX
 AC_PROG_INSTALL
 
-PKG_CHECK_MODULES(XINPUT, x11 xext xorg-server [xi >= 1.5.99.1] [inputproto >= 2.1.99.6])
+
+# *** Enable/disable the X11 backend ***
+AC_ARG_ENABLE(x11,
+              AC_HELP_STRING(--disable-x11, Do not build the X11 backend. (default: enable if available)),
+              [],
+              [enable_x11=auto])
+
+
+# Check for X11 as an optional dependency
+AS_IF([test "x$enable_x11" = "xauto"],
+      [
+        PKG_CHECK_MODULES(XINPUT,
+                          x11 xext xorg-server [xi >= 1.5.99.1] [inputproto >= 2.1.99.6],
+                          [have_xinput=yes],
+                          [have_xinput=no])
+      ])
+
+AS_IF([test "x$enable_x11" = "xyes"],
+      [
+        PKG_CHECK_MODULES(XINPUT,
+                          x11 xext xorg-server [xi >= 1.5.99.1] [inputproto >= 2.1.99.6],
+                          [have_xinput=yes],
+                          [
+                            AC_MSG_ERROR([x11 and/or XInput libraries not found!])
+                            have_xinput=no
+                          ])
+      ])
+
+AS_IF([test "x$enable_x11" = "xno"],
+      [have_xinput=no])
+
+AS_IF([test "x$have_xinput" = "xyes"],
+      [
+        FRAME_X11_BACKEND="FRAME_X11_BACKEND"
+        AM_CONDITIONAL(HAVE_XINPUT, true)
+      ],
+      [
+        FRAME_X11_BACKEND="FRAME_NO_X11_BACKEND"
+        AM_CONDITIONAL(HAVE_XINPUT, false)
+      ])
+AC_SUBST(FRAME_X11_BACKEND)
+AC_SUBST(HAVE_XINPUT)
 
 AC_CHECK_PROG([ASCIIDOC], [a2x], [a2x])
 AM_CONDITIONAL([HAVE_DOCTOOLS], [test "x$ASCIIDOC" != "x"])
@@ -47,7 +88,8 @@
 AS_IF([test "x$enable_integration_tests" != xno],
       [CHECK_XORG_GTEST(
           [AC_MSG_NOTICE([xorg-gtest is available, integration tests will be built])],
-          [AC_MSG_WARN([xorg-gtest is not available, tests will not be built])])])
+          [AC_MSG_WARN([xorg-gtest is not available, tests will not be built])])],
+      [have_xorg_gtest=no])
 
 AM_CONDITIONAL([ENABLE_INTEGRATION_TESTS], [test "x$have_xorg_gtest" = xyes])
 
@@ -65,6 +107,7 @@
 
 AC_CONFIG_FILES([Makefile
                  doc/Makefile
+                 include/oif/frame.h
                  src/Makefile
                  test/Makefile
                  test/gtest/Makefile
@@ -74,6 +117,9 @@
                  tools/Makefile
                  frame.pc])
 
+AS_IF([test "x$have_xinput" = "xyes"],
+      [AC_CONFIG_FILES([frame-x11.pc])])
+
 AC_LANG_PUSH(C++)
 AC_CHECK_DECL(__clang__, CXX_LIBS="-lstdc++")
 AC_SUBST(CXX_LIBS)
@@ -86,3 +132,16 @@
 AC_SUBST(VISIBILITY_CXXFLAGS, "-fvisibility=hidden")
 
 AC_OUTPUT
+
+bold_green=$(tput bold)$(tput setf 2)
+bold_white=$(tput bold)$(tput setf 7)
+reset=$(tput sgr0)
+
+AC_MSG_NOTICE([])
+AC_MSG_NOTICE([${bold_green}Open Input Framework - Frame library - $VERSION${reset}])
+AC_MSG_NOTICE([])
+AC_MSG_NOTICE([   Prefix                  : ${bold_white}${prefix}${reset}])
+AC_MSG_NOTICE([   CFLAGS                  : ${bold_white}${CFLAGS} ${GCC_FLAGS}${reset}])
+AC_MSG_NOTICE([   Build X11 backend       : ${bold_white}${have_xinput}${reset}])
+AC_MSG_NOTICE([   Build integration tests : ${bold_white}${have_xorg_gtest}${reset}])
+AC_MSG_NOTICE([])

=== 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 @@
+prefix=@prefix@
+exec_prefix=@exec_prefix@
+libdir=@libdir@
+includedir=@includedir@
+
+Name: frame-x11
+Description: Touch Frame Library - X11 support
+Version: @PACKAGE_VERSION@
+Requires: frame = @PACKAGE_VERSION@

=== 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 @@
 #endif // __has_feature
 #endif // __has_feature(c_generic_selections)
 
+/* Whether the X11 backend (frame_x11.h) is available.
+   FRAME_X11_BACKEND will be defined if it is and FRAME_NO_X11_BACKEND otherwise. */
+#define @FRAME_X11_BACKEND@
+
 #ifdef __cplusplus
 extern "C" {
 #endif

=== 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 @@
 libframeinclude_HEADERS = \
 	$(top_srcdir)/include/oif/frame.h \
 	$(top_srcdir)/include/oif/frame_backend.h \
-	$(top_srcdir)/include/oif/frame_internal.h \
+	$(top_srcdir)/include/oif/frame_internal.h
+
+if HAVE_XINPUT
+libframeinclude_HEADERS += \
 	$(top_srcdir)/include/oif/frame_x11.h
+endif
 
 libframe_la_SOURCES = \
 	axis.h \
@@ -52,7 +56,10 @@
 	value.h \
 	value.cpp \
 	window.h \
-	window.cpp \
+	window.cpp
+
+if HAVE_XINPUT
+libframe_la_SOURCES += \
 	x11/device_x11.h \
 	x11/device_x11.cpp \
 	x11/frame_x11.cpp \
@@ -60,5 +67,6 @@
 	x11/handle_x11.cpp \
 	x11/window_x11.h \
 	x11/window_x11.cpp
+endif
 
 EXTRA_DIST = $(version_script)

=== 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 @@
 
 #include <memory>
 
-#include <X11/X.h>
-
 #include "oif/frame.h"
 #include "typedefs.h"
 

=== 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 @@
-SUBDIRS = gtest x11_mocks regular
-
+SUBDIRS = gtest
+
+if HAVE_XINPUT
+SUBDIRS += x11_mocks
+endif
+
+SUBDIRS += regular
+
+# integration tests are all X11 based
+if HAVE_XINPUT
 if ENABLE_INTEGRATION_TESTS
 SUBDIRS += integration
 endif
+endif

=== 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 @@
-test_targets = check-c-compile check-cxx-compile
+test_targets =
+
+if HAVE_XINPUT
+test_targets += check-c-compile check-cxx-compile
+endif
 
 if HAVE_GTEST
 test_targets += check-regular
@@ -37,10 +41,14 @@
 ### check-regular
 
 check_regular_SOURCES = \
+	backend.cpp
+
+if HAVE_XINPUT
+check_regular_SOURCES += \
 	accept-ended-touch.cpp \
-	backend.cpp \
-	frame-fixture.cpp \
-	frame-fixture.h
+	frame-x11-fixture.cpp \
+	frame-x11-fixture.h
+endif
 
 #
 # Link against the (non-distributed) static lib to pick up the
@@ -50,10 +58,14 @@
 	$(top_builddir)/src/.libs/libframe.a \
 	$(top_builddir)/test/gtest/libgtest.a \
 	$(top_builddir)/test/gtest/libgtest_main.a \
-	$(top_builddir)/test/x11_mocks/libx11_mocks.a \
 	$(COVERAGE_LIBS) \
 	$(GTEST_LIBS)
 
+if HAVE_XINPUT
+check_regular_LDADD += \
+	$(top_builddir)/test/x11_mocks/libx11_mocks.a
+endif
+
 check_regular_CPPFLAGS = \
 	--std=c++0x \
 	-I$(top_srcdir)/test/x11_mocks \

=== 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 @@
-#include "frame-fixture.h"
+#include "frame-x11-fixture.h"
 #include "x11_mocks.h"
 
-class AcceptEndedTouch : public FrameFixture
+class AcceptEndedTouch : public FrameX11Fixture
 {
 };
 

=== 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 @@
-#include "frame-fixture.h"
+#include "frame-x11-fixture.h"
 #include "x11_mocks.h"
 
-FrameFixture::FrameFixture()
+FrameX11Fixture::FrameX11Fixture()
   : frame_handle(nullptr),
     _xevent_serial_number(1)
 {
 }
 
-void FrameFixture::SetUp()
+void FrameX11Fixture::SetUp()
 {
   xmock_touch_acceptance.clear();
 }
 
-void FrameFixture::TearDown()
+void FrameX11Fixture::TearDown()
 {
   ASSERT_EQ(nullptr, frame_handle);
 }
 
-void FrameFixture::CreateXMockTouchScreenDevice()
+void FrameX11Fixture::CreateXMockTouchScreenDevice()
 {
   xmock_devices_count = 1;
   xmock_devices = (XIDeviceInfo*) calloc(xmock_devices_count,
@@ -59,7 +59,7 @@
   xmock_devices[0].classes = classes;
 }
 
-void FrameFixture::DestroyXMockDevices()
+void FrameX11Fixture::DestroyXMockDevices()
 {
   for (int i = 0; i < xmock_devices_count; ++i)
   {
@@ -70,7 +70,7 @@
   free(xmock_devices);
 }
 
-void FrameFixture::SendTouchEvent(
+void FrameX11Fixture::SendTouchEvent(
     int event_type, int touch_id, float x, float y)
 {
   UFStatus status;
@@ -112,7 +112,7 @@
   free(device_event);
 }
 
-void FrameFixture::SendTouchOwnershipEvent(int touch_id)
+void FrameX11Fixture::SendTouchOwnershipEvent(int touch_id)
 {
   UFStatus status;
   XGenericEventCookie xcookie;
@@ -141,7 +141,7 @@
   free(ownership_event);
 }
 
-void FrameFixture::FetchDeviceAddedEvent(UFDevice *device)
+void FrameX11Fixture::FetchDeviceAddedEvent(UFDevice *device)
 {
   UFEvent event;
   UFStatus status;
@@ -157,7 +157,7 @@
   frame_event_unref(event);
 }
 
-void FrameFixture::AssertNoMoreEvents()
+void FrameX11Fixture::AssertNoMoreEvents()
 {
   UFEvent event;
   UFStatus status;

=== 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 @@
-#ifndef GTEST_FRAME_FIXTURE_H
-#define GTEST_FRAME_FIXTURE_H
+#ifndef GTEST_FRAME_X11_FIXTURE_H
+#define GTEST_FRAME_X11_FIXTURE_H
 
 #include <gtest/gtest.h>
 #include "oif/frame.h"
 #include "oif/frame_x11.h"
 
-class FrameFixture : public ::testing::Test
+class FrameX11Fixture : public ::testing::Test
 {
  protected:
-  FrameFixture();
+  FrameX11Fixture();
 
   virtual void SetUp();
   virtual void TearDown();