Avoid depending on libwayland 1.20 unnecessarily

[smcv]

Description

When using shared linking (linking in the normal way with
-lwayland-client) rather than loading Wayland libraries dynamically at
runtime, listing symbols that don't exist in the current version results
in a build failure. We don't actually call wl_proxy_marshal_flags() or
wl_proxy_marshal_array_flags() directly; the reason we need them is
that they're called by the code generated by wayland-scanner >= 1.20.

If we're building against an older Wayland library, then we'll have its
corresponding version of wayland-scanner (mismatched versions are not
supported), so we won't need those two symbols, and can avoid generating
a dependency on them.

Conversely, if we're building against a newer Wayland library, the
generated code will call them unconditionally, so we cannot treat them as
optional and gracefully fall back: that would result in a crash. Instead,
treat them as a mandatory part of the Wayland library, so that if they
are not found at runtime, we can fall back to X11 without crashing.

libwayland 1.18 is in several LTS distributions (Ubuntu 20.04,
Debian 11, RHEL 8) so avoiding a hard dependency on 1.20 is quite
useful.

Existing Issue(s)

Resolves: #5376

Helps: #4636

[smcv]

This worked when I applied it while packaging SDL 2.0.20 for Debian unstable (which had libwayland 1.18 at the time), but I haven't re-verified it with current git, hence marking as draft until I get a chance to do that (hopefully tomorrow). Feel free to apply this despite the draft status if you've tested it yourself.

[smcv]

I haven't re-verified it with current git, hence marking as draft until I get a chance to do that

Re-tested on Debian 11 (Wayland 1.18.0) based on 8df045c, still seems to work as expected.

[sezero]

Helps: #4636

Just tried, it did not.

[smcv]

Helps: #4636

Just tried, it did not.

I didn't claim that this fully resolves #4636, but combining this with #5429 should?

[sezero]

I didn't claim that this fully resolves #4636, but combining this with #5429 should?

#5429 will refuse to build wayland in for me, so it should.

[smcv]

With this and #5429, I would expect that:

• your build described in build error with --disable-wayland-shared #4636 (with arbitrarily old Wayland) should fail (cleanly) with --disable-wayland-shared, but succeed with --enable-wayland-shared;
• building on e.g. Debian 11 or Ubuntu 20.04 (with Wayland 1.18, wl_proxy_marshal_flags causing build to fail #5376) should succeed either way, which is an improvement on the current situation where only --enable-wayland-shared works;
• building on e.g. Debian testing (with Wayland 1.20) should continue to succeed either way

[flibitijibibo]

Here's a fun question... let's say I'm on a setup with the 1.20 headers, and I want to make a build that simply does not use the new exports at all - what exactly is preventing us from doing this via something like a define? The fact that the breakage is in a header almost comes off as an ABI break since you physically cannot build for compatibility without macro'ing out the contents of client-protocol.h, since the change exists in a static inline function and therefore can't be ignored even if you use old functions?

It seems like the real solution to this is to just unconditionally macro the new _flags functions to call the old functions instead. This does a number of things once 1.18 is enforced:

• Fixes the shared build for 1.18 because we won't have any references to the new functions except in macro form (which will go unused)
• Fixes the shared build for 1.20 because builds will not use the new functions, so 1.18 won't break immediately on load
• Fixes the dynamic build for 1.18 because the macro will ultimately do nothing
• Fixes the dynamic build for 1.20 because we're no longer using new functions that are being forced via static inline functions, so 1.18 will still be okay

[flibitijibibo]

Pushed the 1.18 enforcement to simpify all these patches coming in... introduces a conflict but hopefully trivial to patch up.

[smcv]

let's say I'm on a setup with the 1.20 headers, and I want to make a build that simply does not use the new exports at all

I don't think wayland-scanner aims to be forward-compatible: it assumes that the code generated by wayland-scanner will be compiled with Wayland headers of equal version, and run with a Wayland library of equal or greater version. In general, shared libraries are designed to be backwards-compatible but not forwards-compatible, so if you will distribute binaries, you generally have to compile with the oldest dependency version your binaries aim to support.

It seems like the real solution to this is to just unconditionally macro the new _flags functions to call the old functions instead

Doesn't that result in the WL_MARSHAL_FLAG_DESTROY flag (and any flags added in future) being ignored completely? That seems bad, because the semantics of WL_MARSHAL_FLAG_DESTROY are "... and then atomically destroy the object", replacing a separate call to wl_proxy_destroy() that was done under a separate lock.

[smcv]

If wayland-scanner had an equivalent of GLib's gdbus-codegen --glib-max-allowed=2.70, then that would provide more forward-compatibility, but it currently doesn't have that.

[flibitijibibo]

Hm, that might be something we should ask for... weirdos like me can just install exactly 1.18 but this could prove to be a really nasty, near-undetectable bug for someone building on something newer.

[flibitijibibo]

The change to wayland-scanner is smaller than I was expecting:

https://gitlab.freedesktop.org/wayland/wayland/-/commit/0e0274af0c9f60d2759713df136f4294054c9096

I'm guessing a full --max-version would be a lot more involved across all versions but for 19->20 this doesn't look like it'd be too invasive?

[smcv]

3cc3bcd has the change @flibitijibibo suggested, and a correspondingly adjusted commit message.

[fooishbar]

I don't think wayland-scanner aims to be forward-compatible: it assumes that the code generated by wayland-scanner will be compiled with Wayland headers of equal version, and run with a Wayland library of equal or greater version. In general, shared libraries are designed to be backwards-compatible but not forwards-compatible, so if you will distribute binaries, you generally have to compile with the oldest dependency version your binaries aim to support.

A bit late to this, but yep, this is exactly it. The rule is to build with the same version of wayland-scanner and libwayland-client, and if you want that to be something that will work on older systems, then (same as glibc etc) be building against that version or older.