Bug 96770

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.

(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;

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>

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

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)) ####