Bug 103225 - Deprecated wl_buffer still mentioned in EGL/eglmesaext.h
Summary: Deprecated wl_buffer still mentioned in EGL/eglmesaext.h
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: EGL/Wayland (show other bugs)
Version: 17.2
Hardware: All FreeBSD
: medium minor
Assignee: Wayland bug list
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-11 16:39 UTC by Greg V
Modified: 2017-11-08 12:11 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments

Comment 1 Pekka Paalanen 2017-10-12 06:43:22 UTC
struct wl_buffer has two different meanings:

- On client side, wl_buffer is a protocol object type, which has not been deprecated. It is still valid, used and necessary. There is no definition for 'struct wl_buffer', it is a purely abstract opaque type. This definition is used by eglCreateWaylandBufferFromImageWL()

- On server side, there used to be a definition for 'struct wl_buffer' which is what you refer to in wayland-server.h, that has since been deprecated. This is never used by any EGL API anymore. It used to be used by eglQueryWaylandBufferWL() until that was changed to use wl_resource instead (a binary compatible change). Server side is supposed to no longer use a struct wl_buffer type at all.

While both server and client side had the same wl_buffer pointer type, the underlying definition has never been the same. The client side thing is actually a struct wl_proxy.

For backwards compatibility, one could not simply remove the definition of struct wl_buffer from wayland-server.h. It was made conditional on not defining WL_HIDE_DEPRECATED. If you #define WL_HIDE_DEPRECATED (and do any code fixes required), you won't get the warning anymore.

However, strictly speaking one cannot define WL_HIDE_DEPRECATED in EGL headers. Programs need to define it themselves as it changes the expectations on how they are written. Defining it in EGL headers could theoretically break someone's build.

I'm not sure what we could do. Simply #including wayland-server.h followed by eglmesaext.h will raise the warning, but the programmer is not doing anything wrong.

Each program can fix this on their own by defining WL_HIDE_DEPRECATED before including any headers. This is also recommended practise since the things are deprecated for a reason. Would this be an acceptable solution for you?
Comment 2 Greg V 2017-10-12 10:09:26 UTC
(In reply to Pekka Paalanen from comment #1)
WL_HIDE_DEPRECATED is what I ended up doing in https://github.com/swaywm/wlroots/pull/252 — the project maintainer told me to report this *because* of WL_HIDE_DEPRECATED :D But yeah I personally think it's fine.
Comment 3 Emil Velikov 2017-10-12 10:33:58 UTC
(In reply to Greg V from comment #2)
> (In reply to Pekka Paalanen from comment #1)
> WL_HIDE_DEPRECATED is what I ended up doing in
> https://github.com/swaywm/wlroots/pull/252 — the project maintainer told me
> to report this *because* of WL_HIDE_DEPRECATED :D But yeah I personally
> think it's fine.
Eek... wlroots. </joking> Have you tried libweston - I doubt people will object any sensible API additions if it doesn't suite your needs.

Back on topic:
 - I'll need to get back to upstreaming that EGL extension
 - A local forward declaration (like the struct wl_resource one) should handle the issue, I'll send a patch in a minute
 - The warning should not block the build, so in the short interim please ignore it.
Comment 4 Emil Velikov 2017-10-12 10:47:52 UTC
Greg there should be a copy of the patch in your inbox. Alternatively you can grab it from patchwork

https://patchwork.freedesktop.org/patch/182020/
Comment 5 Greg V 2017-10-12 11:11:13 UTC
(In reply to Emil Velikov from comment #3)
> Eek... wlroots. </joking> Have you tried libweston - I doubt people will
> object any sensible API additions if it doesn't suite your needs.
My goal was to get wlroots (and by extension, the future version of sway) working on FreeBSD. I'm not writing my own compositor!

>  - A local forward declaration (like the struct wl_resource one) should
> handle the issue, I'll send a patch in a minute
Just tried that. The warning is still there.

I don't actually understand where both headers are included, that's never done explicitly in the source, they just somehow ended up together. (And seemingly, only on FreeBSD?!) wayland-server wasn't included *for* eglmesaext anywhere.

>  - The warning should not block the build, so in the short interim please
> ignore it.
Some people really like -Werror for some reason :( But yeah for now -DWL_HIDE_DEPRECATED is used.
Comment 6 Emil Velikov 2017-10-12 12:48:16 UTC
(In reply to Greg V from comment #5)

> >  - A local forward declaration (like the struct wl_resource one) should
> > handle the issue, I'll send a patch in a minute
> Just tried that. The warning is still there.
> 
Ack. Patch still makes sense on its own.

> I don't actually understand where both headers are included, that's never
> done explicitly in the source, they just somehow ended up together. (And
> seemingly, only on FreeBSD?!) wayland-server wasn't included *for*
> eglmesaext anywhere.
> 
Without a build log it's impossible to tell. From a quick look -> the glapi.h generated in wlroot is including the eglmesaext.h header when it shouldn't need to.


> >  - The warning should not block the build, so in the short interim please
> > ignore it.
> Some people really like -Werror for some reason :( But yeah for now
> -DWL_HIDE_DEPRECATED is used.
Using -Werror in a controlled environment (aka locally) is great. Otherwise it goes badly since different compilers (and versions) produce different warnings.
I hope such people know how to handle/workaround such issues.
Comment 7 Emil Velikov 2017-11-08 12:11:11 UTC
All that said:

The header now provides a local forward declaration for the struct, which is sufficient for all [reasonable] users.

A warning may be present when WL_HIDE_DEPRECATED is not defined.
That's deliberate feature to point out misuse of the structs.


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.