hughsie / appstream-glib

This library provides objects and helper methods to help reading and writing AppStream metadata.
GNU Lesser General Public License v2.1
65 stars 103 forks source link

misc: Port to libsoup3 #421

Closed mcrha closed 2 years ago

mcrha commented 3 years ago

Build for libsoup3 by default. Use -Dsoup2=true meson option to build against libsoup2 instead. The libsoup API version is exposed in the appstream-glib.pc file as well, thus the library users can check they use the same libsoup API version as the appstream-glib library.

mcrha commented 3 years ago

The default value for the libsoup version use mimics what other projects do, like libgweather or geocode-glib. I guess it's for a reason that the libsoup2 is going to be removed, thus the new builds will not need to carry the "use libsoup3 instead" option ad infinity.

hughsie commented 3 years ago

Ohh wow, @kalev what do you think here? Don't we need to break soname if we change the dep soname?

mcrha commented 3 years ago

Aha, I did not think of it, but yes, it makes sense, to be able to install both versions on a single machine.

mcrha commented 3 years ago

Which would require a change in the .pc file name as well?

hughsie commented 3 years ago

Which would require a change in the .pc file name as well?

Ohh it requires quite a few painful changes, e.g. include file install location binary tool names etc.

mcrha commented 3 years ago

Ohh it requires quite a few painful changes, e.g. include file install location binary tool names etc.

In case of the appstream-glib the libsoup usage is not exposed in the public API, thus there's no problem with the headers, only with the library, which bring in the process the libsoup version it is built from.

It's still painful to do, no doubt. I guess it is the reason why other projects, like geocode-glib do not plan to install the two version sin parallels, at least I do not see any such option here: https://gitlab.gnome.org/GNOME/geocode-glib/-/merge_requests/15

@mcatanzaro Do I recall correctly that there is no plan to install (some of) the libraries in parallel, one for libsoup3 and one for libsoup2? The libsoup itself allows it, but not all already ported libraries do.

ximion commented 3 years ago

Why does this need a feature flag at all? The Meson build definition could autodetect if libsoup3 is present and if so, use it, and otherwise fall back to the old version. Using both in the same process would obviously not work, but having both compiled against different versions would also be odd. Unless there is a good reason to have both Soup versions, getting rid of one should be the goal, right? At least in Debian, we would rebuild all dependencies of the old soup in order, and then remove the older library version once done.

mcatanzaro commented 3 years ago

Do I recall correctly that there is no plan to install (some of) the libraries in parallel, one for libsoup3 and one for libsoup2? The libsoup itself allows it, but not all already ported libraries do.

Totally up to you. libsoup itself is indeed parallel-installable. Whether to do that for appstream-glib really depends on how many reverse dependencies you have using libsoup.

The Meson build definition could autodetect if libsoup3 is present and if so, use it, and otherwise fall back to the old version.

I'm going to suggest avoiding automatic anything, since this tends to lead to unexpected or undesired results. I like what Milan has done: libsoup3 is newer and should be the default, but a fallback to libsoup2 is available on an opt-in basis.

Unless there is a good reason to have both Soup versions, getting rid of one should be the goal, right? At least in Debian, we would rebuild all dependencies of the old soup in order, and then remove the older library version once done.

Well you have to manually port each dependency to libsoup3 first. They will all fail to build until ported. It's very unlikely that a distro as large as Debian could do this with a flag day: it will need two parallel-installed libsoups for a long time to come. And distros will make the switch at different times, hence the feature flag seems practical.

mcrha commented 3 years ago

Whether to do that for appstream-glib really depends on how many reverse dependencies you have using libsoup.

Aha, I see, the it can be fine to not offer to install in parallel both versions. The soname version change (not really only bump) makes sense too, thus the apps already built against the current soname version will eventually break on their execution. They might too, if the libsoup version does not match (as both libsoup 2 and 3 take care of it now).

I like what Milan has done: libsoup3 is newer and should be the default, but a fallback to libsoup2 is available on an opt-in basis.

I just did mimic what had been done in other projects. Including the pkg-config file info, thus the incompatible libsoup API version can be detected in build time, rather than in runtime. Such detection requires changes on the library user code, of course.

mcrha commented 3 years ago

The commit https://github.com/mcrha/appstream-glib/commit/0e541977329076a94160ad0eb4b0dda9008aa0bf handles the soname version change. I can squash it with the previous commit, I only wanted to make it clearer what the change can be.

ximion commented 3 years ago

Well you have to manually port each dependency to libsoup3 first. They will all fail to build until ported. It's very unlikely that a distro as large as Debian could do this with a flag day: it will need two parallel-installed libsoups for a long time to come. And distros will make the switch at different times, hence the feature flag seems practical.

Sure, supporting both is obviously essential :-) Having an explicit flag is also usually a good idea.

A soname bump and rename and parallel installability feels like a lot of work though. On Debian, there are only two things depending on appstream-glib currently (on the library at least): Flatpak and libmalcontent-ui. The latter could of course be linked to things that use an older libsoup, but at the moment, both its consumer don't do that. Flatpak is an issue, but has its own porting commit to libsoup already. So, @hughsie's call of course, but the impact of being soup3-only seems to be fairly manageable.

mcrha commented 3 years ago

On Debian, there are only two things depending on appstream-glib currently (on the library at least): Flatpak and libmalcontent-ui.

For what it's worth, and unless I ran a wrong dnf command, Fedora has a dependency on the libappstream-glib on Flatpak, malcontent-ui-libs and PackageKit. Neither the malcontent nor the PackageKit depend on the libsoup (thus a simple rebuild against new appstream-glib is fine).

kalev commented 3 years ago

I don't think it's necessary to change the soname but I think it's also fine to do it if you want :) What I would avoid here is making the whole project parallel-installable (changing the .pc file name and headers and so on) as that seems to just lead to more pain on the distro side.

The situation should be roughly the same in Fedora as in Debian that ximion described and we don't have a lot of packages using libappstream-glib.

mwleeds commented 2 years ago

Fedora has a dependency on the libappstream-glib on Flatpak, malcontent-ui-libs and PackageKit.

Flatpak now dropped the libappstream-glib dep on the development branch. It might be better to focus on porting those other two projects from libappstream-glib rather than porting libappstream-glib to soup3?

mcatanzaro commented 2 years ago

Ping maintainers. Please land this. A huge amount of software is using appstream-glib. Porting away seems impractical.

hughsie commented 2 years ago

It's still an API break, no?

nanonyme commented 2 years ago

Decreasing amount of appstream-glib consumers at this point are using the API, majority just use executables out of the project. Bumping soname seems sensible if this is an API break.

hughsie commented 2 years ago

Agreed, the soname has to be bumped too. It probably makes sense to make the next version 0.8.0 too.

ximion commented 2 years ago

Ping maintainers. Please land this. A huge amount of software is using appstream-glib. Porting away seems impractical.

Sorry for the slightly OT question, but which software would that be? On my system, actually nothing depends on appstream-glib (apt-cache rdepends libappstream-glib8 also yields nothing). I like to know what software is using asglib and for what reason they might want to stick with it.

nanonyme commented 2 years ago

@ximion the dependency is typically on appstream-util provided by appstream-glib, not the rest of appstream-glib.

mcatanzaro commented 2 years ago

It's still an API break, no?

Sort of. If the application does not use libsoup, then it doesn't matter. If it does, then yeah distros need to ensure they do not combine libsoup 2 and libsoup 3.

If a lot of software depends on libappstream-glib, then you may wish to bump your API version, so that you have the older API version using libsoup2 and the newer API version using libsoup 3. If not a lot of stuff depends on it, and you don't intend to provide parallel-installability, then I'd say it does not matter.

Sorry for the slightly OT question, but which software would that be?

Quick search of gnome-build-meta:

$ git grep appstream-glib
elements/core-deps/flatpak.bst:- sdk/appstream-glib.bst
elements/core-deps/malcontent.bst:- sdk/appstream-glib.bst
elements/core/baobab.bst:- sdk/appstream-glib.bst
elements/core/cheese.bst:- sdk/appstream-glib.bst
elements/core/dconf-editor.bst:- sdk/appstream-glib.bst
elements/core/devhelp.bst:- sdk/appstream-glib.bst
elements/core/epiphany.bst:- sdk/appstream-glib.bst
elements/core/evince.bst:- sdk/appstream-glib.bst
elements/core/gnome-calculator.bst:- sdk/appstream-glib.bst
elements/core/gnome-calendar.bst:- sdk/appstream-glib.bst
elements/core/gnome-characters.bst:- sdk/appstream-glib.bst
elements/core/gnome-color-manager.bst:- sdk/appstream-glib.bst
elements/core/gnome-disk-utility.bst:- sdk/appstream-glib.bst
elements/core/gnome-logs.bst:- sdk/appstream-glib.bst
elements/core/gnome-weather.bst:- sdk/appstream-glib.bst
elements/core/totem.bst:- sdk/appstream-glib.bst
elements/sdk-platform.bst:- sdk/appstream-glib.bst
elements/sdk-platform.bst:# appstream-glib, at-spi2-*, atk, fcitx, gcab, gdk-pixbuf, geoclue2, glib-networking,
elements/sdk/appstream-glib.bst:  url: freedesktop_people:~hughsient/appstream-glib/releases/appstream-glib-0.7.18.tar.xz
elements/sdk/appstream-glib.bst:        - '%{libdir}/libappstream-glib.so'
elements/sdk/cantarell-fonts.bst:- sdk/appstream-glib.bst
elements/sdk/os-release.bst:- sdk/appstream-glib.bst
elements/sdk/yelp.bst:- sdk/appstream-glib.bst
elements/vm/os-release-devel.bst:- sdk/appstream-glib.bst
elements/vm/os-release-user.bst:- sdk/appstream-glib.bst
elements/world/accerciser.bst:- sdk/appstream-glib.bst
elements/world/five-or-more.bst:- sdk/appstream-glib.bst
elements/world/four-in-a-row.bst:- sdk/appstream-glib.bst
elements/world/geary.bst:- sdk/appstream-glib.bst
elements/world/gnome-2048.bst:- sdk/appstream-glib.bst
elements/world/gnome-chess.bst:- sdk/appstream-glib.bst
elements/world/gnome-klotski.bst:- sdk/appstream-glib.bst
elements/world/gnome-mahjongg.bst:- sdk/appstream-glib.bst
elements/world/gnome-mines.bst:- sdk/appstream-glib.bst
elements/world/gnome-nibbles.bst:- sdk/appstream-glib.bst
elements/world/gnome-robots.bst:- sdk/appstream-glib.bst
elements/world/gnome-screenshot.bst:- sdk/appstream-glib.bst
elements/world/gnome-sudoku.bst:- sdk/appstream-glib.bst
elements/world/gnome-taquin.bst:- sdk/appstream-glib.bst
elements/world/gnome-todo.bst:- sdk/appstream-glib.bst
elements/world/hitori.bst:- sdk/appstream-glib.bst
elements/world/iagno.bst:- sdk/appstream-glib.bst
elements/world/lightsoff.bst:- sdk/appstream-glib.bst
elements/world/polari.bst:- sdk/appstream-glib.bst
elements/world/quadrapassel.bst:- sdk/appstream-glib.bst
elements/world/swell-foop.bst:- sdk/appstream-glib.bst

I think most of this stuff needs /usr/bin/appstream-util, not libappstream-glib.

ximion commented 2 years ago

I think most of this stuff needs /usr/bin/appstream-util, not libappstream-glib.

Maybe a half-a-day project for me to make them use appstreamcli as soon as we have some idea how to resolve the GNOME versioning clashing with the AppStream vercmp spec... https://gitlab.gnome.org/GNOME/gnome-software/-/merge_requests/1378 Using appstream-util means that the projects currently can't use newer AppStream features or not have the newer stuff validated.

nanonyme commented 2 years ago

The reality is that unless this lands, freedesktop-sdk is going to need to ship libsoup2 in next Fall's release. It is still too early to drop appstream-glib from runtimes. We will most likely delay libsoup3 migration by one more year to reduce risk of apps mixing libsoup2 and libsoup3.

hughsie commented 2 years ago

I can work on this, but not until next week.

hughsie commented 2 years ago

Alternate proposal in https://github.com/hughsie/appstream-glib/pull/441