After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 757042 - fails to build on armel
fails to build on armel
Status: RESOLVED FIXED
Product: libxml++
Classification: Bindings
Component: Build
2.40.x
Other Linux
: Normal normal
: ---
Assigned To: Christophe de Vienne
Christophe de Vienne
Depends on:
Blocks:
 
 
Reported: 2015-10-24 06:00 UTC by Michael Biebl
Modified: 2015-10-30 10:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (20.79 KB, patch)
2015-10-25 17:51 UTC, Daniel Trebbien
none Details | Review
patch: Add LIBXMLXX_HAVE_EXCEPTION_PTR (6.33 KB, patch)
2015-10-26 17:51 UTC, Kjell Ahlstedt
committed Details | Review
patch: Work around some platforms' lack of support for std::exception_ptr (20.25 KB, patch)
2015-10-26 17:53 UTC, Kjell Ahlstedt
committed Details | Review
build log on armel with the two patches applied (260.20 KB, text/plain)
2015-10-26 18:29 UTC, Michael Biebl
  Details

Description Michael Biebl 2015-10-24 06:00:59 UTC
Version: 2.40.0

The latest version of libxml++2.6 fails to build on armel:
https://buildd.debian.org/status/package.php?p=libxml%2B%2B2.6&suite=sid

Full build log at 
https://buildd.debian.org/status/fetch.php?pkg=libxml%2B%2B2.6&arch=armel&ver=2.40.0-1&stamp=1444323767
Comment 1 Murray Cumming 2015-10-24 08:17:00 UTC
Here is the error from that log:

libtool: compile:  g++ -DHAVE_CONFIG_H -I.. -I.. -I/usr/include/libxml2 -I/usr/include/glibmm-2.4 -I/usr/lib/arm-linux-gnueabi/glibmm-2.4/include -I/usr/include/glib-2.0 -I/usr/lib/arm-linux-gnueabi/glib-2.0/include -I/usr/include/sigc++-2.0 -I/usr/lib/arm-linux-gnueabi/sigc++-2.0/include -DLIBXMLPP_BUILD -D_FORTIFY_SOURCE=2 -Wall -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wall -std=c++11 -c exceptions/wrapped_exception.cc  -fPIC -DPIC -o exceptions/.libs/wrapped_exception.o
In file included from exceptions/wrapped_exception.cc:18:0:
exceptions/wrapped_exception.h:43:49: error: expected ‘)’ before ‘exception_ptr’
   explicit wrapped_exception(std::exception_ptr exception_ptr);
                                                 ^
exceptions/wrapped_exception.h:50:8: error: ‘exception_ptr’ in namespace ‘std’ does not name a type
   std::exception_ptr exception_ptr_;
Comment 2 Murray Cumming 2015-10-24 14:54:44 UTC
Line 43 is this line
  explicit wrapped_exception(std::exception_ptr exception_ptr);
here:

class wrapped_exception : public exception
{
public:
  explicit wrapped_exception(std::exception_ptr exception_ptr);
  ~wrapped_exception() noexcept override;

It looks like std::exception_ptr isn't defined, for some reason:
http://en.cppreference.com/w/cpp/error/exception_ptr
and this seems to be a known g++ problem:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58938

I guess we need to check for it at configure time and #ifdef around it.
Comment 3 Daniel Trebbien 2015-10-25 17:51:55 UTC
Created attachment 314088 [details] [review]
Patch
Comment 4 Kjell Ahlstedt 2015-10-26 07:28:05 UTC
I'll have a closer look at this later (hopefully today).
The basic idea is good, but I think some details can be made nicer.

The lack of std::exception_ptr is even more of a nuisance in libxml++ 2.91.1.
I've used it even more there, and it simplified the exception handling. Now I
will have to revert most of that simplification. Lucky that libxml++ 3.0.x has
not been released yet.
Comment 5 Kjell Ahlstedt 2015-10-26 17:51:54 UTC
Created attachment 314144 [details] [review]
patch: Add LIBXMLXX_HAVE_EXCEPTION_PTR

I've split the patch into two. This is the first one. Both patches are intended
for the libxml++-2-40 branch, but this one can probably be applied also to the
master branch with no or minimal modification.

This is like similar config tests in other packages. The test code is stored in
build/cxx_std.m4.
Comment 6 Kjell Ahlstedt 2015-10-26 17:53:55 UTC
Created attachment 314145 [details] [review]
patch: Work around some platforms' lack of support for std::exception_ptr

This is the second patch. Most of it is identical to Daniel's patch.
The most important difference is Parser::handle_exception() and Validator::
handle_exception(). The rather long list of catch clauses can be kept just
in those two methods. It simplifies especially SaxParser. The same trick is
used in Glib::exception_handlers_invoke() and Glib::glibmm_unexpected_
exception(): Call a method from many exception handlers, and re-throw the
exception.

config.h is not installed. It can be included in test programs. In header
files that application programs can include, use libxml++config.h.

I would be great to have these patches tested in a system without
std::exception_ptr. I've tested it by fiddling with the config test program
in cxx_std.m4, to make it fail, but that's not quite the same as a test on a
system without std::exception_ptr.
Comment 7 Kjell Ahlstedt 2015-10-26 17:54:48 UTC
s/I would be great/It would be great/
Comment 8 Michael Biebl 2015-10-26 17:55:55 UTC
Hi Kjell, I do have access to armel porter boxes, so I can give the patches a try. Thanks for the quick response everyone!
Comment 9 Michael Biebl 2015-10-26 18:28:30 UTC
Seems to build fine on armel now. Test-suite passes as well. Didn't do any more elaborate checks.
Comment 10 Michael Biebl 2015-10-26 18:29:58 UTC
Created attachment 314151 [details]
build log on armel with the two patches applied
Comment 11 Kjell Ahlstedt 2015-10-27 07:48:13 UTC
Thanks for the quick test!
I'll give Daniel a chance to comment. (It was his patch that I modified quite
a bit.) Then I'll push the patches and make a 2.40.1 release.
Comment 12 Daniel Trebbien 2015-10-27 12:22:48 UTC
Hi Kjell,

I think your changes look good.  I like that you created an Autoconf macro to test for std::exception_ptr support.  Also, I learned something new about exception handling in C++ (namely that you can call a handler function within the catch block and the exception object remains valid until the exception is handled).  Your refactoring to contain the list of catch blocks to a single handler, thus eliminating duplicated code, is an excellent idea.

One possible issue:  Does adding the handle_exception() member to `xmlpp::Parser' and `xmlpp::Validator' technically break ABI compatibility in this case?  The new member is declared within a block of protected members.  C++98 specifies layout compatibility for POD-struct types, but seemingly nothing else.  The latest C++ working draft, n4527, now talks about "standard-layout class" types, but `xmlpp::Parser' and `xmlpp::Validator' are non-standard-layout types because of the virtual functions.
Comment 13 Kjell Ahlstedt 2015-10-27 15:09:18 UTC
> One possible issue:  Does adding the handle_exception() member to
> `xmlpp::Parser' and `xmlpp::Validator' technically break ABI compatibility in
> this case?

We add non-virtual methods to classes all the time in other packages, such as
gtkmm, trying to keep up with changes in the underlying C package. It has been
done before in libxml++ too. I would be surprised if it would cause a problem
in this case. There are some restrictions, though: Don't add virtual methods,
don't add data members, don't add base classes.
Comment 14 Kjell Ahlstedt 2015-10-27 15:33:30 UTC
I have pushed the patches in comments 5 and 6, plus a patch that adds a short
description of the exception handling to the documentation of SaxParser.
Those patches were added to the libxml++-2-40 branch. It remains to make similar
changes to the master branch. I leave this bug open until that's fixed.

What about a 2.40.1 release? Shall I do that right away, or wait for more
problems to be detected and fixed?
Comment 15 Michael Biebl 2015-10-27 18:09:15 UTC
A 2.40.1 release sounds great. Should new problems turn up, we can always release a 2.40.1.1 or 2.40.2.
Comment 16 Kjell Ahlstedt 2015-10-28 07:25:15 UTC
Perhaps there is a minor problem with the new handle_exception() methods after
all. I'm still convinced that they don't break ABI, but I don't know if all
protected methods are considered API. New API is not allowed between 2.40.0 and
2.40.1. I've pushed a patch that makes handle_exception() private. Then they are
certainly not part of the API.
Comment 17 Murray Cumming 2015-10-28 08:57:24 UTC
< I don't know if all protected methods are considered API.

Yes, protected methods are API, because builds would break if those methods were used and the methods then disappeared.

> New API is not allowed between 2.40.0 and 2.40.1.

Ideally, not, yes.

> I've pushed a patch that makes handle_exception() private. Then they are
> certainly not part of the API.

Would it be particularly useful as API? If so, we can make it protected for 2.42.

Thanks.
Comment 18 Kjell Ahlstedt 2015-10-28 14:24:03 UTC
I have released libxml++ 2.40.1.

> Would it be particularly useful as API?

I doubt that.

> If so, we can make it protected for 2.42.

If there will ever be a 2.42 release. Most of future development will be on
3.x, I hope.

Anyway, there are protected handle_exception() methods in 2.91.1 in the master
branch. I will have to modify them now. They can't use std::exception_ptr
unconditionally.
Comment 19 Michael Biebl 2015-10-29 00:19:03 UTC
https://buildd.debian.org/status/package.php?p=libxml%2B%2B2.6&suite=sid

Looking good.

Closing this bug report.
Thanks everyone.
Comment 20 Michael Biebl 2015-10-29 00:20:30 UTC
https://buildd.debian.org/status/logs.php?pkg=libxml%2B%2B2.6&ver=2.40.1-1

that's the link specific for version 2.40.1
Comment 21 Kjell Ahlstedt 2015-10-29 09:18:51 UTC
Now I've updated the master branch, too.
Perhaps time for another new release? 2.91.2 or 2.93.1 or what?
ABI is not identical to 2.91.1. Luckily the ABI of libxml++-3.0. has not been
frozen yet.
Comment 22 Murray Cumming 2015-10-29 09:36:29 UTC
Sure.
Comment 23 Kjell Ahlstedt 2015-10-30 10:23:20 UTC
I have released libxml++ 2.91.2.