[vlc-devel] [PATCH] plugins: do not capture set of null-capability modules

[Lyndon Brown]

On Sat, 2020-09-26 at 22:10 +0300, Rémi Denis-Courmont wrote:
> Le lauantaina 26. syyskuuta 2020, 21.58.29 EEST Lyndon Brown a écrit
> :
> > attached. preview below.
> > 
> > i can move the discussion of the reasoning for skipping null-cap
> > modules from the comment block to the commit log if preferred.
> > similarly i'll move the minor documentation enhancements i slipped
> > in
> > here to a separate commit if preferred.
> 
> This is semantically wrong. Notionally we want to store all modules.
> It's just 
> so that currently the store operation only manages capabilities.
> 
> And even then, there are no particular reasons not to keep track of
> modules 
> with "none" capabilities. For all I know, some GUI could want to list
> modules 
> sorted by caps.

The "store" function indeed only registers a module into a capability
list. I see absolutely no useful purpose to capturing null/none/no
capability modules in doing this. They serve absolutely no purpose, and
to do so is going against both the intentions of plugin authors and the
purpose of building capability lists, which is obviously to create
lists of modules that can be used to perform some specific function.
Nothing is going to say "give me the list of all null-cap/no-cap
modules so I can try them and put one to use", since by definition they
are incapable of doing anything, and so there is no purpose to building
such a list.

If something wants to output a list of modules sorted by caps, there is
nothing stopping them, in fact as an extension to later work relating
to capabilities, I've revamped the --list-verbose output to do exactly
that, and there is absolutely no purpose in providing the null-cap/no-
cap set in that output. The purpose of the output is to help users
discover what modules are available for each *functional* capability,
and what shortcuts they have. The null-cap/no-cap ones being listed
would only serve to confuse people by exposing a plugin implementation
detail which isn't meant to be exposed like that.

There are three in-tree plugins making an utterly pointless
add_submodule() call at the start of their descriptors, for which
patches will be submitted later on. In all other cases of
add_submodule() being called before a capability is specified, labels
are been specified beforehand (i.e. <labels>, <addsubmod>,
<labels+cap>). It seems in these cases that the author has done this
deliberately in order to effectively set some plugin level attributes.
The attributes of the very first module in a plugin of course are used
in certain help output, and thus by doing this, not assigning any
capability to the first module, the first module is re-purposed for
controlling the (plugin) labels used in that help output, when the
labels of the first real functional submodule would not be suitable.

That is the only reason for which these null-cap/no-cap modules exist,
as a "hack" by some plugin authors to control help output, because they
can't specify the labels on the plugin object itself.

We do not need to capture a set of these, for which nothing is ever
going to query...

Furthermore, if this is not merged, if a no-cap module list is
legitimised in this way, then this has a knock on effect for all of the
capability related work in my tree built on top of this patch, which
I'd have to spend time reviewing for necessary adjustments, not having
expected any push back on this. :/

If your objection is more in terms of some later intention to modify
the "store" function to do more than just record a capability into a
capability set, and thus in that situation the function rejecting a
module with no capability would be an issue, then I can understand
that, and adjustments would have to be made accordingly.

We could of course make changes to the descriptor design to eliminate
these null-cap/no-cap modules by allowing plugin authors to set labels
on plugins, but hopefully we can agree to put off any such
consideration until later, since I've done a lot of work already across
the entire set of descriptors, tidying and fixing issues and such, to
submit later once earlier work gets merged that it's sitting on.

Regards,
Lyndon