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 661174 - Remove inclusion of Xlib headers from Cogl headers
Remove inclusion of Xlib headers from Cogl headers
Status: RESOLVED FIXED
Product: cogl
Classification: Platform
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: Cogl maintainer(s)
Cogl maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2011-10-07 11:59 UTC by Zan Dobersek
Modified: 2011-11-01 15:59 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (5.33 KB, patch)
2011-10-07 12:26 UTC, Zan Dobersek
none Details | Review
patch #2 (8.32 KB, patch)
2011-10-16 08:13 UTC, Zan Dobersek
none Details | Review
patch #3 (8.77 KB, patch)
2011-10-17 16:58 UTC, Zan Dobersek
none Details | Review

Description Zan Dobersek 2011-10-07 11:59:52 UTC
Cogl headers currently include Xlib headers. The latter define many trivially named objects, which may cause problems in programs/libraries that use cogl.

I recommend Xlib headers are included only in cogl-xlib.h and cogl-xlib-renderer.h headers and that these two headers act as independent, not being included in cogl.h header.
Comment 1 Zan Dobersek 2011-10-07 12:26:03 UTC
Created attachment 198524 [details] [review]
patch

This patch removes most* of the Xlib header includes in cogl headers. cogl-xlib.h and cogl-xlib-renderer.h are made standalone headers. Patch also includes these two headers anywhere they're needed now that they're not included by default.

* The only inclusion left is in cogl-clutter.h. In cogl.h it is noted that the file will eventually be deleted, but until then and in order to fully remove any Xlib headers inclusion a specific header, named cogl-clutter-xlib.h could be created, holding the cogl_clutter_winsys_xlib_get_visual_info function, or the function could be moved to cogl-xlib.h. Until then, the whole problem will not be completely resolved.
Comment 2 Robert Bragg 2011-10-12 14:03:59 UTC
yeah, we've had a few people complain about this issue now.

Originally I was hoping to avoid platform specific headers, but Xlib.h really is a horrible citizen, rudely claiming all kinds of non-namespaced symbols so it seems worth splitting out xlib specific Cogl api at least.

Talking with Neil about this too we would prefer to limit it just to one new standalone cogl-xlib.h header file that developers would have to explicitly include in addition to cogl.h if they need any xlib specific functions.

If you could tweak this so that cogl-xlib-renderer.h remains an internal header but gets included by the standalone cogl-xlib.h that would be really great.

For cogl-clutter.h I think we can do as you suggest and spin out a cogl-clutter-xlib.h but keep it an internal header that gets include in cogl-xlib.h as with cogl-xlib-renderer.h.

thanks for looking at this!
Comment 3 Zan Dobersek 2011-10-16 08:13:52 UTC
Created attachment 199106 [details] [review]
patch #2

This patch should address all the recommendations by Robert.
Comment 4 Zan Dobersek 2011-10-17 16:58:24 UTC
Created attachment 199233 [details] [review]
patch #3

In previous patch the built library defined the 'cogl_clutter_winsys_xlib_get_visual_info' symbol instead of the same symbol with '_CLUTTER' suffix. This patch fixes that.
Comment 5 Robert Bragg 2011-11-01 15:59:57 UTC
thanks I've just rebased your patch and pushed it to master as:

From e71279506a8f6ce09f7f29a7e3af4c8c582a886f Mon Sep 17 00:00:00 2001
From: Zan Dobersek <zandobersek@gmail.com>
Date: Mon, 17 Oct 2011 18:55:35 +0200
Subject: [PATCH] Remove inclusion of Xlib headers in Cogl headers

Xlib headers define many trivially named objects which can later cause name
collision problems when only cogl.h header is included in a program or
library. Xlib headers are now only included through including the standalone
header cogl-xlib.h.