[PATCH wayland v2 3/4] wayland-util: build/ship as separate shared library

[Daniel Stone]

Hi,

On 17 March 2017 at 12:07, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 17 March 2017 at 11:10, Pekka Paalanen <ppaalanen at gmail.com> wrote:
>> On Thu, 16 Mar 2017 15:32:46 +0000
>> Emil Velikov <emil.l.velikov at gmail.com> wrote:
>>>  - managing the compat server/client DSO in any build system will be a pain
>>
>> How so? Whose build system?
>>
>> User projects do not necessarily need any changes, ever.
>>
>> Wayland's build we need to set up once.
>>
>> So, again very much the same as your proposal.
>>
> Not quite - I see three ways to handle the compat server/client DSO:
>  - symlinks
> Distros love these - I've seen cases where they just remove them.
>
>  - hardlinks
> Love again - if you're lucky enough the distro will ship the same file
> X times and/or the debug package will be broken.
> We even had people who explicitly hack them into symlinks even if they
> need to be hardlinks for legitimate reasons.

These don't work for DSOs loaded through ld.so. :\ For modules it's
fine, since you're only asking dlopen() to open a file and use that,
whatever it may be. But if ld.so is trying to satisfy a DT_NEEDED for
libwayland-client.so.1, it will open libwayland-client.so.1, see that
its SONAME is libwayland.so.1, decide it does not satisfy the
dependency, and ignore it. Maybe not the case everywhere, but at least
on Debian jessie on armhf, older Mali binaries which shipped
libEGL.so.1/libGLESv2.so.2/etc as symlinks to libmali.so, I had to
create real ELF objects for each library, with a matching SONAME and a
DT_NEEDED to libmali.so.

>  - ld script, ALA libc.so/libpthread.so
> Not sure how portable those across C runtime(s). No idea how well
> dlopen/dlsym will work with these.
>
>  - symbol forwarding
> It's a lot of fancy code and C runtime specific. You need reasonably
> new glibc and even then there were some bugs - one GLVND dev had a bug
> for 2+yo bug which did not get any input.
>
> Most/all of the above are doable, yet rather icky.

So let's take libwayland.so off the table then, and come back to
libwayland-util.so. Thanks for putting together your testcase, which I
did try, but it doesn't actually work.

Firstly, libwayland-server in this patchset gains a hard Requires on
wayland-util, meaning that -lwayland-util explicitly finds its way
into link lines. So, even if you're only using old ABI, if you build a
binary against a post-split library, you cannot run it in an
environment with a pre-split library. This is a dealbreaker for me,
because it is an ABI change.

I tested by removing wayland-server from the pkg-config requirements
(quite tedious at 17 seconds per autogen run), and having the tests
only link to wayland-client. Failure:
/home/daniels/tmp/wl_link_test/lib.c:12: undefined reference to `wl_list_init'
/home/daniels/tmp/wl_link_test/lib.c:18: undefined reference to `wl_array_init'

I did check that the libwayland-client being used had a dependency on
libwayland-util, but the linker discarded libwayland-client as none of
its symbols were referenced.

If we had good reason to split libwayland-util out - observed
real-world breakage, a desire to grow the wayland-util exported symbol
set - then we could weigh up the least bad way and go for that. But
AFAICT, it causes us no active problems right now, apart from needing
to perhaps be careful when we change the inner workings when people
are determined to have mismatched libwayland-client vs.
libwayland-server versions. Fine, but given the entire implementation
of all those functions is essentially ABI anyway, that's something we
can live with. I hope you understand that I think breaking actual real
users for a hypothetical benefit is not worth it.

>>> With all due respect the whole thing has become a massive bikeshed,
>>> admittedly with some 'help' of my end as well.
>>> The more we continue the less inclined I'm at fixing other issues in Wayland ...
>>
>> Sorry about that.
>>
>>> As a closing thought - I would really appreciate a list of concrete
>>> issues or "actions" where I could work against.
>>> Pretty please ?
>>
>> I have none. I do not have enough knowledge to form an opinion or make
>> a decision to accept/reject. If I can't give an authoritative Acked-by,
>> and no-one of the well-known expert developers did either, then I
>> cannot merge a change that definitely (IMO) needs seconding.
>>
>> Basically I have been waiting for someone besides you who would also
>> know about this topic to give their opinion on the proposal(s), because
>> I am disqualified. I was hoping my replies would have raised more
>> interest by showing I take the suggestion seriously rather than just
>> ignore it to death, but it didn't happen.
>>
> Hypothetical case:
> I'm well established Wayland developer with experience in dealing with
> shared libraries, other.
> The libwayland-util proposal comes around, I skim through the
> suggestion and replies.
>
> [The less hypothetical part]
> I likely won't bother with as the, seemingly:
>  - "I don't know about this stuff, but I don't like it" connotation
>  - negative input, yet lack of concrete issues/tasks to address
>  - huge volume and orthogonal discussions (yes I'm quite guilty here as well)
>
>> Btw. the squashing started as a half-joke, since we seemed to be going
>> nowhere, to try and provoke more participation or points of view, but
>> the more I thought about squashing, the more sense it started to make.
>> And it's a second failed attempt to draw more attention to this
>> question. I got the idea for squashing from systemd libraries, not Mesa.
>>
>> Indeed, that's what I need: Reviewed-by's and Acked-by's from people
>> with proper reputation.
>>
>> Just like for any patch really, the strength of reviews needs to
>> reflect the impact of the patch.
>
> How about we do something like Daniel ?
>  - he pointed out some possible flaws
>  - I dismissed those and provided a simple test case to justify my claim
>
> If every 'issue' is refuted, shout for X weeks slap an Ack and let us
> carry on with out lives ?

NAK from me.

There are real issues this causes, and I'm still not in any way
convinced of the rationale for making this rather dangerous change in
the first place. Much like with the discussion on Mesa's symbol
exports, in a vacuum you are correct: it should have been done this
way in the first place. If there was a compelling rationale to change
it, then we could have a discussion about the best thing to do.

Right now, you have failed to convince people that the upsides of your
change (which are entirely hypothetical AFAICT) are outweigh the
downsides (actually breaking real-world users who exist and would be
seriously pissed off). My NAK is for the downside (this breakage,
another DSO to version), and given the risk of extremely subtle
breakage given everyone's disparate toolchain versions and
configurations, will stand until there is a well-established set of
benefits for doing this which outweighs that risk.

If you want to avoid sinking too much time into this, I suggest you
work on polishing the rationale first.

Cheers,
Daniel