Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 1163188
Summary: | FTBFS on s390 | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Dan Horák <dan> | ||||||
Component: | syncevolution | Assignee: | Milan Crha <mcrha> | ||||||
Status: | CLOSED CURRENTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||||
Severity: | unspecified | Docs Contact: | |||||||
Priority: | unspecified | ||||||||
Version: | 21 | CC: | mcrha, patrick.ohly | ||||||
Target Milestone: | --- | ||||||||
Target Release: | --- | ||||||||
Hardware: | s390 | ||||||||
OS: | Unspecified | ||||||||
Whiteboard: | |||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||
Doc Text: | Story Points: | --- | |||||||
Clone Of: | Environment: | ||||||||
Last Closed: | 2015-06-10 12:54:42 UTC | Type: | Bug | ||||||
Regression: | --- | Mount Type: | --- | ||||||
Documentation: | --- | CRM: | |||||||
Verified Versions: | Category: | --- | |||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||
Embargoed: | |||||||||
Bug Depends On: | |||||||||
Bug Blocks: | 467765 | ||||||||
Attachments: |
|
Description
Dan Horák
2014-11-12 13:18:20 UTC
Thanks for a bug report. Patrick, could you have a look, please? In case you'd like to test any proposed patches just let me know and I'll test them in koji for you. The problem is that template specialization does not find a match for integer type size_t = unsigned long. How many bits does that have on S390? gdbus-cxx-bridge.h has cases for all variations (signed vs. unsigned, 16 to 64 bits): #include <stdint.h> ... template<> struct dbus_traits<int16_t> : public basic_marshal< int16_t, VariantTypeInt16 > { static std::string getType() { return "n"; } static std::string getSignature() {return getType(); } static std::string getReply() { return ""; } }; template<> struct dbus_traits<uint16_t> : public basic_marshal< uint16_t, VariantTypeUInt16 > { static std::string getType() { return "q"; } static std::string getSignature() {return getType(); } static std::string getReply() { return ""; } }; template<> struct dbus_traits<int32_t> : public basic_marshal< int32_t, VariantTypeInt32 > { static std::string getType() { return "i"; } static std::string getSignature() {return getType(); } static std::string getReply() { return ""; } }; template<> struct dbus_traits<uint32_t> : public basic_marshal< uint32_t, VariantTypeUInt32 > { static std::string getType() { return "u"; } static std::string getSignature() {return getType(); } static std::string getReply() { return ""; } }; template<> struct dbus_traits<int64_t> : public basic_marshal< int64_t, VariantTypeInt64 > { static std::string getType() { return "x"; } static std::string getSignature() {return getType(); } static std::string getReply() { return ""; } }; template<> struct dbus_traits<uint64_t> : public basic_marshal< uint64_t, VariantTypeUInt64 > { static std::string getType() { return "t"; } static std::string getSignature() {return getType(); } static std::string getReply() { return ""; } }; template<> struct dbus_traits<double> : public basic_marshal< double, VariantTypeDouble > { static std::string getType() { return "d"; } static std::string getSignature() {return getType(); } static std::string getReply() { return ""; } }; I'm a bit surprised that none of these match size_t. If mapping size_t does not work, then LocalTransportAgentChild::sendMsg(const std::string &contentType, size_t offset, size_t len, const LocalTransportChild::ReplyPtr &reply) could be changed to use e.g. uint64_t instead for offset and len. Should work without compiler warnings on all platforms where uint64_t >= size_t, and on platforms where uint64_t < size_t (S390?), the fairly harmless warning about type size reduction can be ignored because the actual message sizes should be much smaller. (In reply to Patrick Ohly from comment #2) > The problem is that template specialization does not find a match for > integer type size_t = unsigned long. How many bits does that have on S390? sizeof(size_t) == 4, I think the problem is that your uint32 is defined as "unsigned int", but it is not compatible with "unsigned long" for the compiler even when they are effectively the same size I see. I guess I was lucky that on other platforms size_t was defined identically to one of the fixed-size types. The following patch replaces the fixed-size types with all combinations of the standard types that may be needed. It works like that on my regular test platforms (32 and 64 bit, different Linux distros). Can you give that a try for S390? diff --git a/src/gdbusxx/gdbus-cxx-bridge.h b/src/gdbusxx/gdbus-cxx-bridge.h index 9b78023..6feaeaa 100644 --- a/src/gdbusxx/gdbus-cxx-bridge.h +++ b/src/gdbusxx/gdbus-cxx-bridge.h @@ -76,6 +76,7 @@ #include <boost/algorithm/string/join.hpp> #include <boost/tuple/tuple.hpp> #include <boost/utility.hpp> +#include <boost/type_traits/is_signed.hpp> /* The SyncEvolution exception handler must integrate into the D-Bus * C++ wrapper. In contrast to the rest of the code, that handler uses @@ -1405,48 +1406,63 @@ template<> struct dbus_traits<int8_t> : dbus_traits<uint8_t> } }; -template<> struct dbus_traits<int16_t> : - public basic_marshal< int16_t, VariantTypeInt16 > +/** runtime detection of integer representation */ +template<typename I, bool issigned, size_t bytes> struct dbus_traits_integer_switch {}; +template<typename I> struct dbus_traits_integer_switch<I, true, 2> : + public basic_marshal< I, VariantTypeInt16 > { static std::string getType() { return "n"; } static std::string getSignature() {return getType(); } static std::string getReply() { return ""; } }; -template<> struct dbus_traits<uint16_t> : - public basic_marshal< uint16_t, VariantTypeUInt16 > +template<typename I> struct dbus_traits_integer_switch<I, false, 2> : + public basic_marshal< I, VariantTypeUInt16 > { static std::string getType() { return "q"; } static std::string getSignature() {return getType(); } static std::string getReply() { return ""; } }; -template<> struct dbus_traits<int32_t> : - public basic_marshal< int32_t, VariantTypeInt32 > +template<typename I> struct dbus_traits_integer_switch<I, true, 4> : + public basic_marshal< I, VariantTypeInt32 > { static std::string getType() { return "i"; } static std::string getSignature() {return getType(); } static std::string getReply() { return ""; } }; -template<> struct dbus_traits<uint32_t> : - public basic_marshal< uint32_t, VariantTypeUInt32 > +template<typename I> struct dbus_traits_integer_switch<I, false, 4> : + public basic_marshal< I, VariantTypeUInt32 > { static std::string getType() { return "u"; } static std::string getSignature() {return getType(); } static std::string getReply() { return ""; } }; -template<> struct dbus_traits<int64_t> : - public basic_marshal< int64_t, VariantTypeInt64 > +template<typename I> struct dbus_traits_integer_switch<I, true, 8> : + public basic_marshal< I, VariantTypeInt64 > { static std::string getType() { return "x"; } static std::string getSignature() {return getType(); } static std::string getReply() { return ""; } }; -template<> struct dbus_traits<uint64_t> : - public basic_marshal< uint64_t, VariantTypeUInt64 > +template<typename I> struct dbus_traits_integer_switch<I, false, 8> : + public basic_marshal< I, VariantTypeUInt64 > { static std::string getType() { return "t"; } static std::string getSignature() {return getType(); } static std::string getReply() { return ""; } }; + +template<typename I> struct dbus_traits_integer : public dbus_traits_integer_switch<I, boost::is_signed<I>::value, sizeof(I)> {}; + +// Some of these types may have the same underlying representation, but they are +// still considered different types by the compiler and thus we must have dbus_traits +// for all of them. +template<> struct dbus_traits<signed short> : public dbus_traits_integer<signed short> {}; +template<> struct dbus_traits<unsigned short> : public dbus_traits_integer<unsigned short> {}; +template<> struct dbus_traits<signed int> : public dbus_traits_integer<signed int> {}; +template<> struct dbus_traits<unsigned int> : public dbus_traits_integer<unsigned int> {}; +template<> struct dbus_traits<signed long> : public dbus_traits_integer<signed long> {}; +template<> struct dbus_traits<unsigned long> : public dbus_traits_integer<unsigned long> {}; + template<> struct dbus_traits<double> : public basic_marshal< double, VariantTypeDouble > { Patrick, could you attach the patch instead, please? Bugzilla wraps the lines and otherwise garbles the letters (for example tabs versus spaces), thus an attached patch will make it easier for testing. Created attachment 958450 [details]
define traits for all standard types
Here's the patch as committed to SyncEvolution master branch.
thanks for the patch, now it fails with ... /bin/sh ./libtool --tag=CXX --mode=compile g++ -DHAVE_CONFIG_H -I. -I/builddir/build/BUILD/syncevolution-1.5/src -I./test -I/builddir/build/BUILD/syncevolution-1.5/src/build-synthesis/src -I/usr/include -I/usr/include/kde4/ -I/usr/include/kde4//KDE -I/usr/include -I/usr/include/QtCore -pthread -I/usr/include/evolution-data-server -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/libsecret-1 -I/usr/include/libsoup-2.4 -I/usr/include/libxml2 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -pthread -I/usr/include/evolution-data-server -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/libsecret-1 -I/usr/include/libsoup-2.4 -I/usr/include/libxml2 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -pthread -I/usr/include/evolution-data-server -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/libsecret-1 -I/usr/include/libsoup-2.4 -I/usr/include/libxml2 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -D_LARGEFILE64_SOURCE -DNE_LFS -I/usr/include/neon -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -pthread -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -D_LARGEFILE64_SOURCE -DNE_LFS -I/usr/include/neon -Wall -Wno-unknown-pragmas -Wno-deprecated-declarations -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m31 -march=z9-109 -mtune=z10 -c -o src/backends/webdav/src_backends_webdav_syncdav_la-CalDAVSource.lo `test -f 'src/backends/webdav/CalDAVSource.cpp' || echo './'`src/backends/webdav/CalDAVSource.cpp libtool: compile: g++ -DHAVE_CONFIG_H -I. -I/builddir/build/BUILD/syncevolution-1.5/src -I./test -I/builddir/build/BUILD/syncevolution-1.5/src/build-synthesis/src -I/usr/include -I/usr/include/kde4/ -I/usr/include/kde4//KDE -I/usr/include -I/usr/include/QtCore -pthread -I/usr/include/evolution-data-server -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/libsecret-1 -I/usr/include/libsoup-2.4 -I/usr/include/libxml2 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -pthread -I/usr/include/evolution-data-server -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/libsecret-1 -I/usr/include/libsoup-2.4 -I/usr/include/libxml2 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -pthread -I/usr/include/evolution-data-server -I/usr/include/nss3 -I/usr/include/nspr4 -I/usr/include/libsecret-1 -I/usr/include/libsoup-2.4 -I/usr/include/libxml2 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -D_LARGEFILE64_SOURCE -DNE_LFS -I/usr/include/neon -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -pthread -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -D_LARGEFILE64_SOURCE -DNE_LFS -I/usr/include/neon -Wall -Wno-unknown-pragmas -Wno-deprecated-declarations -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector-strong --param=ssp-buffer-size=4 -grecord-gcc-switches -m31 -march=z9-109 -mtune=z10 -c src/backends/webdav/CalDAVSource.cpp -fPIC -DPIC -o src/backends/webdav/.libs/src_backends_webdav_syncdav_la-CalDAVSource.o In file included from src/backends/pbap/PbapSyncSource.cpp:45:0: ./src/gdbusxx/gdbus-cxx-bridge.h: In instantiation of 'static void GDBusCXX::dbus_traits<boost::variant<V> >::get(GDBusCXX::ExtractArgs&, GVariantIter&, boost::variant<V>&) [with V = long long int; GVariantIter = _GVariantIter]': ./src/gdbusxx/gdbus-cxx-bridge.h:5584:51: required from 'static void GDBusCXX::SignalWatch3<A1, A2, A3>::internalCallback(GDBusConnection*, const gchar*, const gchar*, const gchar*, const gchar*, GVariant*, gpointer) [with A1 = GDBusCXX::Path_t; A2 = std::basic_string<char>; A3 = boost::variant<long long int>; GDBusConnection = _GDBusConnection; gchar = char; GVariant = _GVariant; gpointer = void*]' ./src/gdbusxx/gdbus-cxx-bridge.h:5599:93: required from 'void GDBusCXX::SignalWatch3<A1, A2, A3>::activate(const Callback_t&) [with A1 = GDBusCXX::Path_t; A2 = std::basic_string<char>; A3 = boost::variant<long long int>; GDBusCXX::SignalWatch3<A1, A2, A3>::Callback_t = boost::function<void(const GDBusCXX::Path_t&, const std::basic_string<char>&, const boost::variant<long long int>&)>]' src/backends/pbap/PbapSyncSource.cpp:635:111: required from here ./src/gdbusxx/gdbus-cxx-bridge.h:2088:42: error: 'getSignature' is not a member of 'GDBusCXX::dbus_traits<long long int>' if (dbus_traits<V>::getSignature() != type) { ^ ./src/gdbusxx/gdbus-cxx-bridge.h:2096:50: error: 'get' is not a member of 'GDBusCXX::dbus_traits<long long int>' dbus_traits<V>::get(context, varIter, val); ^ full logs at http://s390.koji.fedoraproject.org/koji/taskinfo?taskID=1610093 Okay, so there *is* a non-standard 64bit long long lurking in the code (the PBAP backend in this case, where it talks to the non-SyncEvolution obexd D-Bus API). I hadn't thought of that and it didn't surface in my own compile tests - I need to check why. Does anyone know of a contemporary Unix platform where "long long" is not a valid, separate type? The obvious solution would be to add: template<> struct dbus_traits<signed long long> : public dbus_traits_integer<signed long long> {}; template<> struct dbus_traits<unsigned long long> : public dbus_traits_integer<unsigned long long> {}; But that would fail to compile on platforms where a) "long long" is not valid or b) the same as "long", so additional configure and ifdef checks may be needed. Created attachment 967519 [details]
define trait for long long
It turned out that the PBAP backend was accidentally turned off where it should have been turned on.
Anyway, defining traits for [unsigned] long long seems to work unconditionally and fixes this second problem.
seems fixed with syncevolution-1.5.1-1.fc22 (http://s390.koji.fedoraproject.org/koji/buildinfo?buildID=321789) |