-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add defines at configure time for wayland protocol interface versions #13993
add defines at configure time for wayland protocol interface versions #13993
Conversation
In wayland, we have the annoying workflow where the protocol code is generated at compile time from xml files and linked statically. This has the assumption that the protocol version that is used to build mpv and the protocol version that is used to build the compositor are the same. They might not be which could lead to the compositor advertising support for some wayland interface that mpv has code to use but the actual linked protocol library doesn't support it. Amazingly, wayland-scanner doesn't generate interface version defines in the includes or anything useful like that (why?). So, give up and do it ourselves during configure time and throw it in config.h.
With the previous commit, we now know what version of xdg_wm_base is supported on the client side. Do the MPMIN check against that instead of blindly on version 6. Fixes mpv-player#13986.
Download the artifacts for this pull request: |
Is it not sufficient to check |
It is possible. But it doesn't solve the fundamental issue which is that we have no way of knowing interface versions at compile time. I think xdg_wm_base is the only thing that currently has this issue, but perhaps something like this would need to be done again in the future for some other protocol. But again, it's kind of overengineering like I said. |
It is possible by calculating the maximum value of all interested |
OK maybe I should have typed "sane way" to be more accurate. I would rather this or hack around in |
FWIW, I'd prefer using |
Maybe you could report it to the wayland-protos issue tracker so at least there's a chance we could get rid of this in the future? |
I already have an open MR for wayland-scanner that adds this version information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks ok, except one nitpick
for line in f: | ||
if "interface name=" in line: | ||
interfaces.append(parse_interface(line)) | ||
sys.stdout.write(','.join(interfaces)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick, but there is mix of "
and '
usage, make it use one of them.
I'm waiting for a response from upstream before I decide what to do with this one. |
We'll just hack around it for now. Hopefully upstream will be receptive to making scanner more useful later on. |
Maybe this is overengineering it but it's kind of dumb to not have this information at configure time in the first place.