Wireshark-dev: Re: [Wireshark-dev] Changes to the plugin registration API

From: Bálint Réczey <balint@xxxxxxxxxxxxxxx>
Date: Wed, 20 Dec 2023 19:42:31 +0100
Hi Roland,

Roland Knall <rknall@xxxxxxxxx> ezt írta (időpont: 2023. dec. 4., H, 13:16):
>
> I do not think we need to revert the whole change. I actually like the way the new version check is implemented and think it is beneficial to a lot of users, because it will reduce the number of changes they have to make in order to update their version.

While ABI stability is promised only between minor releases there is
tremendous value in long term API stability because it saves work for
people maintaining software relying on the API. For this reason I
strongly oppose changing the plugin registration API in a backward
incompatible manner unless it is really needed and is asked for by
heavy users of the API.

My recent experience here is limited to libvirt's plugin which I have
to get recompiled for every major Wireshark release. That
recompilation can't easily be avoided, since even the simplest plugins
link to libwsutil:

rbalint@air-m1:~$ otool -L
/Applications/Wireshark.app/Contents//PlugIns/wireshark/4-2/codecs/g711.so
/Applications/Wireshark.app/Contents//PlugIns/wireshark/4-2/codecs/g711.so:
    @rpath/libwsutil.15.dylib (compatibility version 15.0.0, current
version 15.0.0)
    @rpath/libglib-2.0.0.dylib (compatibility version 6801.0.0,
current version 6801.4.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current
version 1319.100.3)

This makes copying plugins across major releases unlikely to work, but
simple recompilations will most likely keep working as long as we
don't change the API much and in the past years we did quite well in
that regard.
To avoid mixing up ABI incompatible plugins the plugins we keep keep
plugins in major+minor release specific subdirs (.../4-2/... above)
and with the ABI stability promise between maintenance releases it is
simple, understandable and reliable.

Changing the API and filesystem layout significantly would also
require changes in many teaching materials and guides in addition to
generating work for every plugin maintained outside of our repository
and should happen only when there are clear benefits outweighing the
work imposed on others.
Adding new optional info to be provided by plugins via defining new
function calls in the plugin API is easy and I don't oppose that.
Plugin compatibility levels are already well defined, understood and
guarded by relying on the shared library ABI versioning.

The plugin registration changes that landed while the conversation
about them was still ongoing led to introduction of new plugin
compatibility versioning changes, triggered filesystem layout changes
and plugin naming changes while adding very little extra to make up
for breaking the pre-existing practice.

I propose returning to what worked well for many years and reverting
the API changes that were not adequately discussed beforehand to avoid
imposing unnecessary work on external plugin maintainers.
I've prepared an MR with the reverts at
https://gitlab.com/wireshark/wireshark/-/merge_requests/13747 .
It is marked as a draft to wait for the conclusion of _this_
discussion. If you would like to return to the well tested API and
filesystem layout and would like to land the revert please give a
thumbs up there.

If you would like to keep the landed changes please elaborate why, and
if you wish leave a thumbs down at the revert.

> But I do think the enforcement of licenses is a too strict reading of GPL v2. Overmore it is a major change, and I think it should be avoided as it will kill a lot of legitimate use-cases for Wireshark. I understand that we need to be as open as possible, but this is actually killing the openess instead of helping it.
>
> As much as it pains us, we must allow corporate users to maintain their own plugin and manage it with future versions of Wireshark. Everything else is not in the best interest of the application.

I personally don't feel much pain about letting corporate users
develop in-house plugins, because most of them are also the
corporations who let their employees contribute to Wireshark on their
paid time and maybe even support the Wireshark Foundation financially.
I see that as mutually beneficial.

I think we resolved the GPL question and there is no pressing need to
check the plugins' license, it was just based on an unfortunate
misunderstanding.

Cheers,
Balint




> Just my 2 cents
>
> Roland
>
> Am Mo., 4. Dez. 2023 um 13:12 Uhr schrieb Bálint Réczey <balint@xxxxxxxxxxxxxxx>:
>>
>> João Valverde <j@xxxxxx> ezt írta (időpont: 2023. dec. 4., H, 12:59):
>> >
>> >
>> >
>> > On 03/12/23 23:25, João Valverde wrote:
>> > > Hi,
>> > >
>> > > There are some changes in progress to the plugin registration API that
>> > > break compatibility and require manual intervention from plugin
>> > > authors maintaining plugins out-of-tree. These changes are rather
>> > > minor and concern only plugin registration, not other APIs accessible
>> > > to plugins.
>> > >
>> > > See MR 13524:
>> > > https://gitlab.com/wireshark/wireshark/-/merge_requests/13524
>> > >
>> > > Changes required are rewriting the registration code (very easy to do
>> > > [1]) and declare (using a C enum) that the plugin is released either
>> > > under GPLv2 or later, or a GPLv2 compatible license. The other changes
>> > > to the ABI version number are
>> >
>> > The choice of the word "released" here was unfortunate, because it may
>> > imply distribution. Please consider "licensed" instead.
>> >
>> > The license declaration field just affirms what was already implicit:
>> > Wireshark plugins must use licensing terms compatible with the GPL
>> > version 2, so there is no policy change there.
>>
>> GPL allows linking and using GPL-licensed software with
>> non-GPL-licensed software locally. This is an important use case of
>> many Wireshark users who do not wish releasing their plugins and your
>> change broke that. Please revert it.
>>
>> >
>> > > currently not relevant to plugin authors (no policy change is
>> > > implied), it just uses less boilerplate with macros.
>> > >
>> > > This should improve the plug-in experience for both developers and
>> > > users and may improve compatibility in the future.
>>
>>
>> > >
>> > > Comments welcome.
>> > >
>> > > Regards,
>> > >
>> > > João
>> > >
>> > > [1]https://gitlab.com/wireshark/wireshark/-/commit/90b16b40921b737aadf9186685d866fd80e37ee6#4a1fe9011e8240918e5fc6230c0bcd2e4d3b9c34
>> > >