config-parser: export functions to open a config file

Make sure that these will not be exported from libweston.so. The config file handling has been specifically out of scope for libweston and we have removed all the use of weston config structures from it, but the libshared exported functions may still remain, waiting to be cleaned up when someone gets a chance.

I'm also not fond of exporting symbols from weston, because it means the plugins will have to be built with unresolved symbols allowed. It also doesn't help helper clients to begin with.

Third, I don't think Weston project should be in the business of publishing and maintaining a yet another INI file reading library for others to use, so making a new shared library for config reading and installing it would not be nice.

However, I do see your point. We allow these external plugins and helper applications in particular, and they may need configuration. Where do we draw the ABI line...

Wait, how does exporting these functions help you? We do not install libshared. Helper clients do not link to libweston, and they definitely do not link to weston executable.

The config parser code is in libshared (static). The in-tree clients link to that directly. libweston-5 links to libshared. The plugins (e.g. the shells and hmi-controller) link to libweston-5. I assume the the other functions are exported to support this. I'm writing a out of tree controller for the ivi shell and that needs an application that provides the background etc. The easiest way to share the weston.ini was, to do what the in-tree clients do. The only thing missing are these two exports.

libweston was not meant to export the weston_config functions, it is an oversight pending for a fix. weston the executable is probably exporting them too, although ideally it would not, as weston-specific plugins like desktop-shell.so should link libshared directly.

However, you cannot do what in-tree clients do: libshared is a static library that is never installed, so your out-of-tree clients cannot link to it. Plugins OTOH are expected to link to libweston, but then libweston was supposed to be completely free of weston_config stuff.

So we have a problem: there is no recommended way to use weston.ini from external plugins or clients, but it would make sense for weston plugins and their clients to be able to use weston.ini if they are supposed to be used with weston.

Questions to the community:

How should we expose the weston_config API to out-of-tree users? Or should we? The amount of code is so small and without dependencies that it could easily be simply shoved into libweston as optional helpers.

What about other stuff in libshared, should everything be available to out-of-tree users? That might start pulling in Cairo and whatnot and the home-grown command line parsing is probably not the best to be shared, and so on.

Maybe use something like the libinput quirks (which is a purely internal API). The libinput code calls e.g. quirks_get_bool(QUIRK_MODEL_TRACKBALL) and gets the right thing back. Main advantage is: all the parsing errors are in the same location (and I can write tests for it). Oh, and if the requested type is wrong, we can assert on it (requesting an int for what should be a bool).

Obviously you'd have to use a string instead of an enum if you want this to be more generic but all the error checking can still go into the same place. But weston_config_get_bool("General", "SomeKey") and weston_config_get_bool("myplugin", "OtherKey") is not the worst API to have (you can also isolate the plugins from each other this way).

@whot, I wasn't thinking of improving the weston_config API.

My question was more about:

Should weston_config API be exposed at all to external users?
Which binary should expose the API?
- libweston? But that is a compositor library, helper clients should not link to it.
- libweston_config or something else completely new?
Is it enough to expose weston_config API alone, or should we expose more of libshared or libshared-cairo?

My reluctance comes from not wanting to release a yet another INI parsing library and maintain it, but OTOH it probably wouldn't need much maintenance, at least if we clearly declare we do not want any feature additions without in-tree users.

An alternative would be to find a ready-made INI parsing library that eats the same syntax as weston_config, migrate everything to use that, and throw weston_config away.

I suppose the easiest clean solution would be to build a libweston-config that exposes only the weston_config API in full, and install that as a DSO along with libweston. Plus, document that we don't accept feature additions to it unless there is an in-tree user for them. How would that sound?

assigned to @eucan

added IVI shell label

removed IVI shell label

removed assignee

added Core compositor label

added libweston API label and removed Core compositor label

I'd be fine with just exposing it from libweston as-is. I don't want to maintain an INI-like-parsing library, but this is the corner we've painted ourselves into with the weston_config API as is. I understand the logic behind exporting it from a separate DSO, but I'm not sure what it buys us in the practical terms of maintenance overhead, ABI implications, etc.

@pq @derekf We should probably decide this before the release, given that we're changing ABI anyway.

I am still confused on what the aim here is. The MR talks about clients, which implies they do not link to libweston. Is this not about clients after all, but only about plugins?

Also mind, that libweston plugins should not access any weston_config API, that would be a layering violation. weston plugins OTOH can access weston_config API, but they do not really need to access weston_config_parse() because main.c has already called that and can pass the weston_config to the plugin.

In the Weston tree, there are no plugins whatsoever that would call weston_config_parse(), all the callers of that are clients who do not link to libweston.

I still stand by my last set of questions:

I think linking clients to libweston without them being compositors as well is not right.
For plugins, I could go with exporting weston_config_parse() from libweston.
However, the long term plan is really to stop libweston from exporting anything related to weston_config, so we would just rip it out again.
weston executable should also strive to not export symbols, so that plugins can be built with unresolved symbols forbidden.

I guess in the long run, weston executable should be split into an executable and a library again, where the executable would be just a shim loading the library, and weston plugins could link to the library for their needs.

If people want a temporary solution in, I am fine with that if it is also acknowledged that things will change again.

The semantic difference between libweston and weston plugins is that libweston plugins should be usable by all libweston based compositors, also those that do not want weston_config at all. OTOH, weston plugins are specific to Weston and not intended for other libweston based compositors.

So what to do with external clients that want to use weston_config_parse()? Ideally we'd have a new library for that. But if that seems over-engineered, we could use the library to be split out of weston. Yes, it would still be a compositor library, so I could have the same argument against that too.

In the end, I wouldn't like any half-way solution. Either we make weston_config a real proper library to be installed, or we don't and essentially forbid external helper clients from reading weston.ini.

Maybe we should just bite the bullet and decide that weston_config functionality will be put into real properly versioned, installed shared library? It should be mostly one-off effort, and it would clarify the linking structure as well and keep it "clean".

Let me think about what it would mean if libweston exported the full weston_config API on purpose:

it would be a dual-purpose library, one part for compositors, another part for INI processing
we would have to add review rules that libweston itself must not use the weston_config API, so that libweston remains usable for the intended audience, not forcing other compositors to use INI files if they don't want to
the same review rule for libweston plugins in the Weston repository
libweston plugins in external repositories could still make the mistake of using weston_config

Would everyone else agree with this?

If you believe this would be fine, then I'm fine with it too. We just need to document the weston_config part of libweston so that I don't remove it in the future.

To clarify my use-case:

In weston upstream, the desktop-shell plugin uses the weston-desktop-shell application to provide panels etc. And the application reads the configured weston.ini for its configuration.

I want to do the same thing, just out of tree. My helper application links to libweston because libshared-cairo (used by weston-desktop-shell) is not available for out of tree plugins and helper applications.

The rules you listed fit to my use-case, so I obviously like them :-)

Ok, AFAIU, two votes (@daniels and @mol) so far to just export weston_config API from libweston. I suppose if other people had opinions, they would have voiced them by now.

What we are missing right now is some documentation. The exported functions need to be documented if they are not already, and I would like to see a paragraph added to README.md that a second purpose of libweston is to export the weston_config API.

changed milestone to %6.0.0

added 104 commits

76318f0b...91d0f08b - 103 commits from branch wayland:master
2fb0e5e5 - config-parser: export functions to open a config file

Compare with previous version

This is not enough. When built with Meson:

$ nm -g -D libweston/libweston-5.so.0.0.90 | grep section

Nothing. The config API is not exported, as was upstream intention originally.

Admin message

Admin message

config-parser: export functions to open a config file

Merge request reports

Activity