Bug 30398 - incorporate OpenBSD patches or make them unnecessary
Summary: incorporate OpenBSD patches or make them unnecessary
Status: NEW
Alias: None
Product: Telepathy
Classification: Unclassified
Component: gabble (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords: NEEDINFO
Depends on:
Blocks:
 
Reported: 2010-09-27 08:25 UTC by Simon McVittie
Modified: 2010-09-27 15:14 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Simon McVittie 2010-09-27 08:25:46 UTC
OpenBSD seems to apply some patches to telepathy-gabble. In an ideal world, distributors wouldn't need to do that.

Tracked elsewhere
-----------------

http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-src_util_c?rev=1.3 seems related to Bug #22972 so it's not tracked here.

Header ordering
---------------

According to Bug #30347, on OpenBSD you have to put "system headers" before "userland headers". Is there somewhere we can find a more detailed explanation?

This is going to be awkward on Darwin: according to info autoconf, sys/socket.h requires stdlib.h to be included first, and stdlib.h itself requires stdio.h, so there may be no way to keep everyone happy.

Hopefully one day we can use GIO to make the whole mess someone else's problem...

This accounts for:
http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-tests_twisted_test-resolver_c?rev=1.1
http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-src_tube-stream_c?rev=1.3
http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-lib_gibber_gibber-util_c?rev=1.1

NULL
----

http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-lib_ext_wocky_examples_register_c?rev=1.2
> $OpenBSD: patch-lib_ext_wocky_examples_register_c,v 1.2 2010/09/23 13:37:12 jasper Exp $
> --- lib/ext/wocky/examples/register.c.orig	Tue Sep 21 19:59:47 2010
> +++ lib/ext/wocky/examples/register.c	Tue Sep 21 20:00:23 2010
> @@ -73,7 +73,7 @@ main (int argc,
>          "password"   , pass,
>          "xmpp-server", host, NULL);
>  
> -  g_object_set (G_OBJECT (wcon), "email", email, NULL);
> +  g_object_set (G_OBJECT (wcon), "email", email, (void *)0);
>    wocky_connector_register_async (wcon, NULL, connector_callback, NULL);
>    g_main_loop_run (mainloop);

Why does this need changing?

Warnings
--------

http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-lib_ext_wocky_wocky_Makefile_in?rev=1.1
http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-lib_ext_wocky_tests_Makefile_in?rev=1.2

Sorry, we're not going to take patches that turn off warnings. If you insist on turning them off during build, you can use "make ERROR_CFLAGS=".

pkg-config
----------

http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-lib_ext_wocky_wocky_wocky-uninstalled_pc_in?rev=1.2
> $OpenBSD: patch-lib_ext_wocky_wocky_wocky-uninstalled_pc_in,v 1.2 2010/09/23 13:37:12 jasper Exp $
> --- lib/ext/wocky/wocky/wocky-uninstalled.pc.in.orig	Tue Sep 21 19:58:49 2010
> +++ lib/ext/wocky/wocky/wocky-uninstalled.pc.in	Tue Sep 21 19:58:52 2010
> @@ -6,7 +6,6 @@ abs_top_builddir=@abs_top_builddir@
>  Name: Wocky (uninstalled copy)
>  Description: XMPP library
>  Version: @VERSION@
> -Requires: pkg-config >= 0.21
>  Requires.private: glib-2.0 >= 2.16, gobject-2.0 >= 2.16, gio-2.0
>  Libs: ${abs_top_builddir}/wocky/libwocky.la
>  Cflags: -I${abs_top_srcdir} -I${abs_top_builddir} -I${abs_top_builddir}/wocky

Why? As noted in configure.ac, we really do require pkg-config 0.21 or later for correct behaviour of Requires.private; pkg-config provides this pseudo-package internally.
Comment 1 Jasper Lievisse Adriaanse 2010-09-27 08:35:43 UTC
(In reply to comment #0)
> OpenBSD seems to apply some patches to telepathy-gabble. In an ideal world,
> distributors wouldn't need to do that.
> 
> Tracked elsewhere
> -----------------
> 
> http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-src_util_c?rev=1.3
> seems related to Bug #22972 so it's not tracked here.
> 
> Header ordering
> ---------------
> 
> According to Bug #30347, on OpenBSD you have to put "system headers" before
> "userland headers". Is there somewhere we can find a more detailed explanation?
It's good style within OpenBSD to have system headers before userland. Also we don't try to have one mega header that includes everything, that should be up to the source files. If you propose a patch that works for Darwin, Ill be happy to check it on OpenBSD.

> This is going to be awkward on Darwin: according to info autoconf, sys/socket.h
> requires stdlib.h to be included first, and stdlib.h itself requires stdio.h,
> so there may be no way to keep everyone happy.
> 
> Hopefully one day we can use GIO to make the whole mess someone else's
> problem...
> 
> This accounts for:
> http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-tests_twisted_test-resolver_c?rev=1.1
> http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-src_tube-stream_c?rev=1.3
> http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-lib_gibber_gibber-util_c?rev=1.1
> 
> NULL
> ----
> 
> http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-lib_ext_wocky_examples_register_c?rev=1.2
> > $OpenBSD: patch-lib_ext_wocky_examples_register_c,v 1.2 2010/09/23 13:37:12 jasper Exp $
> > --- lib/ext/wocky/examples/register.c.orig	Tue Sep 21 19:59:47 2010
> > +++ lib/ext/wocky/examples/register.c	Tue Sep 21 20:00:23 2010
> > @@ -73,7 +73,7 @@ main (int argc,
> >          "password"   , pass,
> >          "xmpp-server", host, NULL);
> >  
> > -  g_object_set (G_OBJECT (wcon), "email", email, NULL);
> > +  g_object_set (G_OBJECT (wcon), "email", email, (void *)0);
> >    wocky_connector_register_async (wcon, NULL, connector_callback, NULL);
> >    g_main_loop_run (mainloop);
> 
> Why does this need changing?
Our gcc4 would error out with -Wall about missing sentinel in function call.
This fixes it.

> Warnings
> --------
> 
> http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-lib_ext_wocky_wocky_Makefile_in?rev=1.1
> http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-lib_ext_wocky_tests_Makefile_in?rev=1.2
> 
> Sorry, we're not going to take patches that turn off warnings. If you insist on
> turning them off during build, you can use "make ERROR_CFLAGS=".
Ill turn it off that way, as our GCC4 insists on those sentinels being present in the function call.

> 
> pkg-config
> ----------
> 
> http://www.openbsd.org/cgi-bin/cvsweb/ports/net/telepathy/telepathy-gabble/patches/patch-lib_ext_wocky_wocky_wocky-uninstalled_pc_in?rev=1.2
> > $OpenBSD: patch-lib_ext_wocky_wocky_wocky-uninstalled_pc_in,v 1.2 2010/09/23 13:37:12 jasper Exp $
> > --- lib/ext/wocky/wocky/wocky-uninstalled.pc.in.orig	Tue Sep 21 19:58:49 2010
> > +++ lib/ext/wocky/wocky/wocky-uninstalled.pc.in	Tue Sep 21 19:58:52 2010
> > @@ -6,7 +6,6 @@ abs_top_builddir=@abs_top_builddir@
> >  Name: Wocky (uninstalled copy)
> >  Description: XMPP library
> >  Version: @VERSION@
> > -Requires: pkg-config >= 0.21
> >  Requires.private: glib-2.0 >= 2.16, gobject-2.0 >= 2.16, gio-2.0
> >  Libs: ${abs_top_builddir}/wocky/libwocky.la
> >  Cflags: -I${abs_top_srcdir} -I${abs_top_builddir} -I${abs_top_builddir}/wocky
> 
> Why? As noted in configure.ac, we really do require pkg-config 0.21 or later
> for correct behaviour of Requires.private; pkg-config provides this
> pseudo-package internally.
We wrote our own pkg-config in perl and it resides in the base system. We could change it to provide such a file.
Comment 2 Simon McVittie 2010-09-27 09:39:01 UTC
(In reply to comment #1)
> Our gcc4 would error out with -Wall about missing sentinel in function call.
> This fixes it.

I don't think this is appropriate to apply, then; info gcc says:

     A valid `NULL' in this context is defined as zero with any pointer
     type.  If your system defines the `NULL' macro with an integer type
     then you need to add an explicit cast.  GCC replaces `stddef.h'
     with a copy that redefines NULL appropriately.

so if OpenBSD's gcc packaging breaks this, please complain to your gcc maintainer.

> > pkg-config
> We wrote our own pkg-config in perl and it resides in the base system. We could
> change it to provide such a file.

If you really can't use the normal pkg-config then you should make sure your reimplementation is compatible, I think. In the C implementation, there is actually no pkg-config.pc - pkg-config inserts equivalent information into its internal data structures before reading any .pc files. I'd recommend doing the same.

Note that the 0.21 dependency is to support a fairly subtle feature of Requires.private (adding private dependencies' CFLAGS), so you may need to check that your pkg-config reimplementation behaves the same.
Comment 3 Jasper Lievisse Adriaanse 2010-09-27 15:14:02 UTC
(In reply to comment #2)
> > > pkg-config
> > We wrote our own pkg-config in perl and it resides in the base system. We could
> > change it to provide such a file.
> 
> If you really can't use the normal pkg-config then you should make sure your
> reimplementation is compatible, I think. In the C implementation, there is
> actually no pkg-config.pc - pkg-config inserts equivalent information into its
> internal data structures before reading any .pc files. I'd recommend doing the
> same.
We try to have our pkg-config implementation as compatible as possible with the "normal" one, I've added the missing feature our pkg-config, and the diff is awaiting review and further testing. So the pkg-config are not needed anymore, thanks for pointing it out.

> Note that the 0.21 dependency is to support a fairly subtle feature of
> Requires.private (adding private dependencies' CFLAGS), so you may need to
> check that your pkg-config reimplementation behaves the same.
Indeed.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.