GNOME Bugzilla – Bug 524620
glib 2.16 g_assert moved to gtestutils, patch for pan
Last modified: 2008-07-04 18:04:51 UTC
pan (as of svn 335) won't compile against glib 2.16(.1) as there's no longer a g_assert. g_assert_warning seems to be the closest it gets. FWIW, pan compiled against glib 2.14 seems to run against 2.16 just fine. I suppose that's because I've never hit one of those assertion. Duncan
I forgot to add the error I got, but the same glib/gmessages.h header file (with the note about g_assert) is #included in a few other files as well, so this isn't likely to be the only one. my-tree.cc: In member function 'virtual void pan::DataImpl::MyTree::set_filter(pan::Data::ShowType, const pan::FilterInfo*)': my-tree.cc:99: error: 'g_assert' was not declared in this scope my-tree.cc: In member function 'void pan::DataImpl::MyTree::add_articles(const std::vector<const pan::DataImpl::ArticleNode*, std::allocator<const pan::DataImpl::ArticleNode*> >&)': my-tree.cc:404: error: 'g_assert' was not declared in this scope my-tree.cc:417: error: 'g_assert' was not declared in this scope my-tree.cc:481: error: 'g_assert' was not declared in this scope my-tree.cc:494: error: 'g_assert' was not declared in this scope make[3]: *** [my-tree.o] Error 1 make[3]: Leaving directory `/tmp/portage/net-nntp/pan-9999/work/pan-9999/pan/data-impl'
Created attachment 108109 [details] [review] pan g_assert patch for glib 2.16 OK Daniel Rahn (OpenSuSE) pointed out it just moved. Here's a partial patch. It does the trick for glib 2.16 but will need IFDEFs deciding which version to use, and I don't know how to do that, so... Duncan
Is there some more general glib header that includes both of these? I only have glib-2.14.x, which doesn't even have a gtestutils.h at all, but my glib.h has an #include <glib/gmessages.h>. If glib-2.16 glib.h includes both gmessages.h and gtestutils.h, then #include <glib.h> would solve this for everyone without needing to do any compile-time detection.
(In reply to comment #3) > Is there some more general glib header that includes both of these? I only have > glib-2.14.x, which doesn't even have a gtestutils.h at all, but my glib.h has > an #include <glib/gmessages.h>. If glib-2.16 glib.h includes both gmessages.h > and gtestutils.h, then #include <glib.h> would solve this for everyone without > needing to do any compile-time detection. Well, I have gone down that route ... it will only work going forward from 2.14. glib 2.12 has a completely different design. So only including glib.h will make pan require glib version 2.14 as a minimum. Not sure that's really the way to go as pan will happily work with older versions so far. You could in theory misuse the version defines of glib to do some compile-time detection. Something like that: extern "C" { #include <glib.h> } #if (GLIB_MAJOR_VERSION == 2 && GLIB_MINOR_VERSION < 16) extern "C" { #include <glib/gmessages.h> } #endif
Created attachment 108842 [details] [review] abstract glib API from PAN Ok, here is a patch Duncan and me came up with that does the following: - Move the glib includes/dependencies to a single file pan/general/glib-compat.h - In that file include glib headers in correct order to make PAN more robust against glib API changes
Forgot to mention: patch tested against glib 2.6.3 - 2.16.2
the patch should add glib-compat.h to some Makefile.am rules to make it included in generated tarball (and it will allow make distcheck to pass). Other than that attachment 108842 [details] [review] looks good.
(In reply to comment #7) > the patch should add glib-compat.h to some Makefile.am rules to make it > included in generated tarball (and it will allow make distcheck to pass). Other > than that attachment 108842 [details] [review] [edit] looks good. Gilles, help me out here. I am afraid I don't understand above comment. With the patch glib-compat.h becomes part of the original PAN tarball in the dir pan/general/ and gets included by the SUBDIRS directive, without modification. If I am missing something obvious, please tell me. I'll then go ahead and adjust the patch.
Confirming the bug and changing the severity to blocker. Hope it gets some attention.
fixed in r342. Thanks!
*** Bug 530864 has been marked as a duplicate of this bug. ***