Bug 96770 - include/GL/mesa_glinterop.h:62: error: redefinition of typedef ‘GLXContext’
Summary: include/GL/mesa_glinterop.h:62: error: redefinition of typedef ‘GLXContext’
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: 12.0
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: mesa-dev
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords: bisected, regression
Depends on:
Blocks:
 
Reported: 2016-07-01 20:44 UTC by Vinson Lee
Modified: 2016-10-12 21:40 UTC (History)
3 users (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 Vinson Lee 2016-07-01 20:44:20 UTC
mesa: 07cc838b105dd3f34526db73064f1f21b452240e (master 12.1.0-devel)

Build error with GCC 4.4.

  Compiling src/glx/dri_common_interop.c ...
In file included from src/glx/dri_common_interop.c:33:
include/GL/mesa_glinterop.h:62: error: redefinition of typedef ‘GLXContext’
include/GL/glx.h:165: note: previous declaration of ‘GLXContext’ was here


commit 8472045b16b3e4621553fe451a20a9ba9f0d44b6
Author: Emil Velikov <emil.velikov@collabora.com>
Date:   Tue May 3 12:25:34 2016 +0100

    mesa_glinterop: remove inclusion of GLX header
    
    Since we only need partial information about the GLX symbols we can
    forward declare them and drop the include. Obviously each user of the
    said API will needs more than what's provides, so they'll include the
    GLX header.
    
    If they don't, the compiler will give us a nice warning ;-)
    
    Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
    Reviewed-by: Marek Olšák <marek.olsak@amd.com>
    Tested-by: Tom Stellard <thomas.stellard@amd.com>
Comment 1 Emil Velikov 2016-07-05 11:39:09 UTC
Yay, I broke things ;-)

There's a couple of routes we can take:
 - Bring back the EGL/GLX includes - I would strongly advice against that.
 - guard the typedefs with ifdef macros - fragile, we'll also need to ensure that the header is included after the EGL/GLX ones.
 - opencode/replaces the existing typedefs with the respective original types - a tad nasty, yet it seems like the better option.
 - other ?

Vinson, let me know which one you'd prefer and I'll whip a patch... Unless you beat me to it.
Comment 2 Vinson Lee 2016-09-06 19:15:55 UTC
(In reply to Emil Velikov from comment #1)
> Yay, I broke things ;-)
> 
> There's a couple of routes we can take:
>  - Bring back the EGL/GLX includes - I would strongly advice against that.
>  - guard the typedefs with ifdef macros - fragile, we'll also need to ensure
> that the header is included after the EGL/GLX ones.
>  - opencode/replaces the existing typedefs with the respective original
> types - a tad nasty, yet it seems like the better option.
>  - other ?
> 
> Vinson, let me know which one you'd prefer and I'll whip a patch... Unless
> you beat me to it.

Emil, I don't have a preference but I tested that undoing the changes in 8472045b16b3e4621553fe451a20a9ba9f0d44b6 fixes the build.

diff --git a/include/GL/mesa_glinterop.h b/include/GL/mesa_glinterop.h
index 383d7f9..c6a967e 100644
--- a/include/GL/mesa_glinterop.h
+++ b/include/GL/mesa_glinterop.h
@@ -52,15 +52,12 @@
 
 #include <stddef.h>
 #include <stdint.h>
+#include <GL/glx.h>
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
-/* Forward declarations to avoid inclusion of GL/glx.h */
-typedef struct _XDisplay Display;
-typedef struct __GLXcontextRec *GLXContext;
-
 /* Forward declarations to avoid inclusion of EGL/egl.h */
 typedef void *EGLDisplay;
 typedef void *EGLContext;
Comment 3 Vinson Lee 2016-10-10 22:42:10 UTC
Author: Vinson Lee <vlee@freedesktop.org>
Date:   Mon Oct 3 15:16:30 2016 -0700

    Revert "mesa_glinterop: remove inclusion of GLX header"
    
    This reverts commit 8472045b16b3e4621553fe451a20a9ba9f0d44b6.
    
    Conflicts:
    
        include/GL/mesa_glinterop.h
    
    This patch fixes this build error with GCC 4.4.
    
      Compiling src/glx/dri_common_interop.c ...
    In file included from src/glx/dri_common_interop.c:33:
    include/GL/mesa_glinterop.h:62: error: redefinition of typedef ‘GLXContext’
    include/GL/glx.h:165: note: previous declaration of ‘GLXContext’ was here
    
    Fixes: 8472045b16b3 ("mesa_glinterop: remove inclusion of GLX header")
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96770
    Signed-off-by: Vinson Lee <vlee@freedesktop.org>
Comment 4 Emil Velikov 2016-10-11 10:33:51 UTC
Sigh I'm starting to loose my cool :-\

Vinson why did you opt for the option I _explicitly_ mentioned is a _bad_ idea ?

Yes, this particular workload broken, yet reverting the commit (as you can see in git history) will break a lot more workflows.

Guess I should stop being nice and call people(?)/their work names. Yet I seriously want to avoid that where possible.

/me tests some 2wo local patch
Comment 5 Mauro Rossi 2016-10-12 21:40:58 UTC
Hi,

I need to report that Android build has been broken by Vinson's commit mentioned at Comment #3.

Mauro


In file included from external/mesa/src/gallium/state_trackers/dri/dri2.c:34:
In file included from external/mesa/include/GL/mesa_glinterop.h:55:
external/mesa/include/GL/glx.h:30:10: fatal error: 'X11/Xlib.h' file not found
#include <X11/Xlib.h>
         ^
1 error generated.
[ 30% 588/1946] Gen ES: libmesa_st_mesa <= api_exec.c
ninja: build stopped: subcommand failed.
build/core/ninja.mk:148: recipe for target 'ninja_wrapper' failed
make: *** [ninja_wrapper] Error 1

#### make failed to build some targets (02:08 (mm:ss)) ####


bug/show.html.tmpl processed on Feb 28, 2017 at 05:32:42.
(provided by the Example extension).