Merge lp:~james-page/ubuntu/precise/zookeeper/arm-ftbfs-fixes into lp:ubuntu/precise/zookeeper

[James Page]

This MP addresses testing on armel/armhf:

1) The current upstream C test code is not compatible with arm; patched to use _sync_* primitives rather than ASM code for atomic operations in the test code. Reported upstream with patch submitted https://issues.apache.org/jira/browse/ZOOKEEPER-1374

2) My panda won't run the Java core test suite through the completion; I'm guessing the same will hold true on the buildds so I've disabled execution on arm based architectures.

Builds OK on my panda locally.

[Clint Byrum (clint-fewbar)]

Excerpts from James Page's message of Tue Jan 24 16:53:30 UTC 2012:
> James Page has proposed merging lp:~james-page/ubuntu/precise/zookeeper/arm-ftbfs-fixes into lp:ubuntu/zookeeper.
>
> Requested reviews:
> Clint Byrum (clint-fewbar)
> Related bugs:
> Bug #920871 in zookeeper (Ubuntu): "zookeeper 3.3.4+dfsg1-2ubuntu2 FTBFS on armel and armhf"
> https://bugs.launchpad.net/ubuntu/+source/zookeeper/+bug/920871
>
> For more details, see:
> https://code.launchpad.net/~james-page/ubuntu/precise/zookeeper/arm-ftbfs-fixes/+merge/89940
>
> This MP addresses testing on armel/armhf:
>
> 1) The current upstream C test code is not compatible with arm; patched to use _sync_* primitives rather than ASM code for atomic operations in the test code. Reported upstream with patch submitted https://issues.apache.org/jira/browse/ZOOKEEPER-1374
>
> 2) My panda won't run the Java core test suite through the completion; I'm guessing the same will hold true on the buildds so I've disabled execution on arm based architectures.
>
> Builds OK on my panda locally.

This looks great, but I don't know that I'm qualified to evaluate it,
having almost no assembly or arm porting experience.

+1, but with the disclaimer that I'm not sure at all why it works. :)

[Matthias Klose (doko)]

looks fine

review: Approve

=== modified file '.pc/applied-patches'
--- .pc/applied-patches	2012-01-06 13:58:23 +0000
+++ .pc/applied-patches	2012-01-24 16:52:20 +0000
@@ -4,3 +4,4 @@
 fixes/ZOOKEEPER-705
 fixes/ZOOKEEPER-1033
 fix-broken-c-client-unittest.patch
+fixes/ZOOKEEPER-1374

=== added directory '.pc/fixes/ZOOKEEPER-1374'
=== added directory '.pc/fixes/ZOOKEEPER-1374/src'
=== added directory '.pc/fixes/ZOOKEEPER-1374/src/c'
=== added directory '.pc/fixes/ZOOKEEPER-1374/src/c/tests'
=== added file '.pc/fixes/ZOOKEEPER-1374/src/c/tests/ThreadingUtil.cc'
--- .pc/fixes/ZOOKEEPER-1374/src/c/tests/ThreadingUtil.cc	1970-01-01 00:00:00 +0000
+++ .pc/fixes/ZOOKEEPER-1374/src/c/tests/ThreadingUtil.cc	2012-01-24 16:52:20 +0000
@@ -0,0 +1,79 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <sys/types.h>
+#include "ThreadingUtil.h"
+#include "LibCSymTable.h"
+
+#ifdef THREADED
+
+// ****************************************************************************
+// Mutex wrapper
+struct Mutex::Impl{
+    Impl(){
+        LIBC_SYMBOLS.pthread_mutex_init(&mut_, 0);        
+    }
+    ~Impl(){
+        LIBC_SYMBOLS.pthread_mutex_destroy(&mut_);        
+    }
+    pthread_mutex_t mut_;
+};
+
+Mutex::Mutex():impl_(new Impl) {}
+Mutex::~Mutex() { delete impl_;}
+void Mutex::acquire() {
+    LIBC_SYMBOLS.pthread_mutex_lock(&impl_->mut_);
+}
+void Mutex::release() {
+    LIBC_SYMBOLS.pthread_mutex_unlock(&impl_->mut_);
+}
+
+// ****************************************************************************
+// Atomics
+int32_t atomic_post_incr(volatile int32_t* operand, int32_t incr)
+{
+    int32_t result;
+    __asm__ __volatile__(
+         "lock xaddl %0,%1\n"
+         : "=r"(result), "=m"(*operand)
+         : "0"(incr)
+         : "memory");
+   return result;
+}
+int32_t atomic_fetch_store(volatile int32_t *ptr, int32_t value)
+{
+    int32_t result;
+    __asm__ __volatile__("lock xchgl %0,%1\n"
+                          : "=r"(result), "=m"(*ptr)
+                          : "0"(value)
+                          : "memory");
+   return result; 
+}
+#else
+int32_t atomic_post_incr(volatile int32_t* operand, int32_t incr){
+    int32_t v=*operand;
+    *operand+=incr;
+    return v;
+}
+int32_t atomic_fetch_store(volatile int32_t *ptr, int32_t value)
+{
+    int32_t result=*ptr;
+    *ptr=value;
+    return result;
+}
+#endif // THREADED

=== modified file 'debian/changelog'
--- debian/changelog	2012-01-06 13:58:23 +0000
+++ debian/changelog	2012-01-24 16:52:20 +0000
@@ -1,3 +1,14 @@
+zookeeper (3.3.4+dfsg1-2ubuntu3) precise; urgency=low
+
+  * Fix FTBFS of test suite on armel/armhf (LP: #920871):
+    - d/patches/fixes/ZOOKEEPER-1374: Use __sync_* primitives 
+      instead of ASM code to improve portability across supported 
+      platforms.
+    - d/rules: Don't run full core Java test suite on arm architectures
+      as it breaks due to JVM incompatibilities.
+
+ -- James Page <[e-mail]>  Tue, 24 Jan 2012 15:14:43 +0000
+
 zookeeper (3.3.4+dfsg1-2ubuntu2) precise; urgency=low
 
   [ Clint Byrum ]

=== added file 'debian/patches/fixes/ZOOKEEPER-1374'
--- debian/patches/fixes/ZOOKEEPER-1374	1970-01-01 00:00:00 +0000
+++ debian/patches/fixes/ZOOKEEPER-1374	2012-01-24 16:52:20 +0000
@@ -0,0 +1,38 @@
+Description: Use GCC primitives instead of ASM for better cross
+ platform support.
+Author: James Page <[e-mail]>
+Forwarded: https://issues.apache.org/jira/browse/ZOOKEEPER-1374
+
+Index: zookeeper/src/c/tests/ThreadingUtil.cc
+===================================================================
+--- zookeeper.orig/src/c/tests/ThreadingUtil.cc	2012-01-24 13:13:17.231731255 +0000
++++ zookeeper/src/c/tests/ThreadingUtil.cc	2012-01-24 13:14:45.683733682 +0000
+@@ -45,24 +45,13 @@
+ 
+ // ****************************************************************************
+ // Atomics
+-int32_t atomic_post_incr(volatile int32_t* operand, int32_t incr)
++int32_t atomic_post_incr(volatile int32_t* operand, int32_t incr)
+ {
+-    int32_t result;
+-    __asm__ __volatile__(
+-         "lock xaddl %0,%1\n"
+-         : "=r"(result), "=m"(*operand)
+-         : "0"(incr)
+-         : "memory");
+-   return result;
++   return __sync_fetch_and_add(operand,incr);
+ }
+-int32_t atomic_fetch_store(volatile int32_t *ptr, int32_t value)
++int32_t atomic_fetch_store(volatile int32_t *ptr, int32_t value)
+ {
+-    int32_t result;
+-    __asm__ __volatile__("lock xchgl %0,%1\n"
+-                          : "=r"(result), "=m"(*ptr)
+-                          : "0"(value)
+-                          : "memory");
+-   return result; 
++   return __sync_lock_test_and_set(ptr,value);
+ }
+ #else
+ int32_t atomic_post_incr(volatile int32_t* operand, int32_t incr){

=== modified file 'debian/patches/series'
--- debian/patches/series	2012-01-06 13:58:23 +0000
+++ debian/patches/series	2012-01-24 16:52:20 +0000
@@ -4,3 +4,4 @@
 fixes/ZOOKEEPER-705
 fixes/ZOOKEEPER-1033
 fix-broken-c-client-unittest.patch
+fixes/ZOOKEEPER-1374

=== modified file 'debian/rules'
--- debian/rules	2012-01-06 13:58:23 +0000
+++ debian/rules	2012-01-24 16:52:20 +0000
@@ -63,10 +63,13 @@
 	$(MAKE) -C src/c zktest-mt
 	cd src/c && ./zktest-mt
 
+# Unable to run the full Java test suite on ARM architectures
+ifeq (,$(findstring arm, $(DEB_BUILD_ARCH)))
 override_dh_auto_test-indep:
 	# Run core Java test suite against zookeeper
 	ant -Dversion=$(DEB_UPSTREAM_VERSION) -DlastRevision=-1 test-core-java
 endif
+endif
 
 override_dh_clean:
 	dh_clean --exclude=src/java \

=== modified file 'src/c/tests/ThreadingUtil.cc'
--- src/c/tests/ThreadingUtil.cc	2010-01-28 12:07:38 +0000
+++ src/c/tests/ThreadingUtil.cc	2012-01-24 16:52:20 +0000
@@ -47,22 +47,11 @@
 // Atomics
 int32_t atomic_post_incr(volatile int32_t* operand, int32_t incr)
 {
-    int32_t result;
-    __asm__ __volatile__(
-         "lock xaddl %0,%1\n"
-         : "=r"(result), "=m"(*operand)
-         : "0"(incr)
-         : "memory");
-   return result;
+   return __sync_fetch_and_add(operand,incr);
 }
 int32_t atomic_fetch_store(volatile int32_t *ptr, int32_t value)
 {
-    int32_t result;
-    __asm__ __volatile__("lock xchgl %0,%1\n"
-                          : "=r"(result), "=m"(*ptr)
-                          : "0"(value)
-                          : "memory");
-   return result; 
+   return __sync_lock_test_and_set(ptr,value);
 }
 #else
 int32_t atomic_post_incr(volatile int32_t* operand, int32_t incr){