ostree icon indicating copy to clipboard operation
ostree copied to clipboard

[RFC][WIP] port to libsoup3

Open q66 opened this issue 3 years ago • 30 comments

Hello,

this is an initial port of libostree to use libsoup3. The reason for this is:

  1. GNOME and webkit are migrating to libsoup3, and other upstreams should do so as well as the new generation is significantly cleaned up, brings http/2 support and other things
  2. libsoup2 and libsoup3 cannot exist in the same process together
  3. libsoup2 is in maintenance mode and everything is expected to migrate over time

This pull request does several things:

  1. Upgrades glib requirement to at least ~~2.68~~ 2.66 (though it does not yet actually fix up all the warnings that result from this - it only fixes up the bare minimum to get it to build)
  2. changes all usage of SoupURI to GUri (which is needed for libsoup3) - this is why the above is needed
  3. actually ports the fetcher as well as the server
  4. removes the soupuri copypasta from the build (though does not yet remove the actual files)

For now this is rough, as I wanted to get comments first. ~~Notably, I was wondering wthether to attempt some kind of dual-libsoup compatibility. I have already attempted this, but it's fairly complicated and it would take a lot of effort to coerce the code to work both ways (with a lot of stuff being wildly different between the generations).~~ This supports both versions of libsoup now. Additionally, I wanted to find out how the maintainers feel about bumping up the dependency requirements (glib), as that is a significant thing to consider. If this is okay, it would probably be a good idea to start by updating the glib requirement in the codebase on its own first.

It is expected that this rework is buggy for now. I wanted to get this out so that people are aware of it, and so that somebody else doesn't start working on it and waste effort.

In any case, I'm looking forward to this getting a review.

q66 avatar Feb 18 '22 02:02 q66

Hi @q66. Thanks for your PR.

I'm waiting for a ostreedev member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci[bot] avatar Feb 18 '22 02:02 openshift-ci[bot]

/ok-to-test

cgwalters avatar Feb 18 '22 15:02 cgwalters

Upgrades glib requirement to at least 2.68

From the ostree-for-Fedora/RHEL ecosystem...I think we can deal with this. The glib in rhel8 is now quite old, but...we can take the hit of cherry picking patches there instead; that's our burden to carry.

An important point here is that today in Fedora derivatives, ostree uses libcurl, not libsoup. So if we can potentially avoid bumping the glib dependency if libcurl is used, that'd be helpful.

That aside, this looks good to me so far! Thanks for working on this!

cgwalters avatar Feb 18 '22 15:02 cgwalters

The reason the dependency is bumped is the migration to GUri; SoupURI is gone in libsoup3, and the copy-pasted SoupURI inside the codebase appears to be pretty much a workaround. GUri as it is requires at least 2.66, while 2.68 is necessary for G_URI_FLAGS_SCHEME_NORMALIZE.

So the options are basically:

  1. just switch to GUri as it is right now; that means dropping a bunch of code even for libcurl, and is likely better going forward, as long as you can deal with the newer version
  2. keep the copypasted SoupURI; enable the copypasted SoupURI even for libsoup backend, add internal code for conversion to GUri where libsoup needs it; change the build system to only need newer glib when compiling libsoup (because regardless of how things done, newer libsoup will pull in newer glib requirement because it uses GUri in its public API), and somehow deal with the glib version incompatibilities (which will likely be its own can of worms because it'd need ifdefs or wrappers around the codebase)

q66 avatar Feb 18 '22 16:02 q66

just switch to GUri as it is right now; that means dropping a bunch of code even for libcurl, and is likely better going forward, as long as you can deal with the newer version

Yeah, SGTM. But I don't want to speak for everyone - this definitely has implications for the flatpak ecosystem. There's a somewhat related thread over here: https://github.com/flatpak/flatpak/issues/2241#issuecomment-1044434100

So summoning @smcv

cgwalters avatar Feb 18 '22 16:02 cgwalters

I think the related Flatpak PR is https://github.com/flatpak/flatpak/pull/4582 unless I'm missing something.

mwleeds avatar Feb 18 '22 16:02 mwleeds

GLib 2.68 is going to be available in Ubuntu 22.04 (coming soon) and Debian 12, but a dependency on GLib 2.68 would rule out backporting libostree to Ubuntu 20.04 (the current LTS right now) and Debian 11.

If it's possible to limit the dependency to 2.66, then that would open up the option of backporting to Debian 11, which would be nice. An weak build-time dependency with GLIB_CHECK_VERSION is absolutely fine.

At the moment, all backport and PPA builds of Flatpak use libostree 2020.8, which is the version shipped in Debian 11, and that has been sufficient so far (although there are some annoying bugs for which I might try to backport patches at some point).

However, if Flatpak wants to start depending on new features of libostree 2022+, then a GUri dependency would make that awkward.

The other side of this is that Flatpak itself uses libsoup directly. If Flatpak and libostree are ported to libsoup 3, distros that compile libostree against libsoup will need to be able to switch both of them from libsoup 2 to libsoup 3 as a flag-day, because libsoup doesn't have versioned symbols and so mixing the two libraries will crash.

At the moment, the .deb world is still building libostree against libsoup, because it is not clear to me what the advantages and disadvantages of libsoup vs libcurl are, so I stuck with the status quo instead of making arbitrary changes; and the Ubuntu and PPA packages are derived from my Debian packages, so they inherit that policy. If there are reasons why I should switch to libcurl, that's entirely possible to do.

smcv avatar Feb 18 '22 17:02 smcv

I thought distros in general used libsoup for ostree, but as it turns out Fedora/RHEL does not, so it should be practical for things that do not switch flatpak to libsoup3 to just use the libcurl backend for ostree

I believe 2.66 should be doable, so if that helps, I can attempt that...

q66 avatar Feb 18 '22 17:02 q66

At Endless we use libsoup in ostree and I don't think we'd switch over to libcurl without a lot of testing. Even though I don't have any reasons to doubt libcurl, download failures == you can't upgrade failures. The ostree/flatpak libsoup flag day does not sound fun, though.

dbnicholson avatar Feb 18 '22 19:02 dbnicholson

Notably, I was wondering wthether to attempt some kind of dual-libsoup compatibility. I have already attempted this, but it's fairly complicated and it would take a lot of effort to coerce the code to work both ways (with a lot of stuff being wildly different between the generations)

If the same libostree codebase cannot work with both libsoup 2 and libsoup 3, then that effectively means distros that use libsoup in libostree won't be able to upgrade libostree to a libsoup3 version until Flatpak is ready for it.

smcv avatar Feb 18 '22 23:02 smcv

yes, indeed - i was thinking those could switch to curl backend for the time being, or alternatively carry a patch for flatpak (there is already an open pull request in flatpak for that, it was linked earlier - it appears quite ready, and is a trivial patch, much simpler than the ostree port itself)

of course, the ideal case would be having support in both projects merged, so nobody has to carry any patches...

q66 avatar Feb 19 '22 00:02 q66

I added back support for libsoup2; it can be enabled with --with-soup2. The higher GLib requirement is still in place (I will try to get it down to 2.66 though) as GUri is still used in ostree-fetcher-uri so that the copypasta can be dropped and so that the work becomes easier for soup3 support. The soup2 support is written against relatively modern soup2 API (i.e. all the deprecated conditional stuff is gone, I set the baseline to 2.70.0 which matches Ubuntu 20.04) and to share as much as possible with the soup3 support, it does not use SoupRequest and the likes.

This should address things for distributions that will not be switching to soup3 yet but also don't want to switch to curl.

q66 avatar Feb 20 '22 13:02 q66

changed minimum glib version to 2.66; updated the GUri code to only use the 2.68 scheme normalization when new enough and otherwise it normalizes manually

q66 avatar Feb 20 '22 14:02 q66

Can you split out the GUri change? It seems useful on its own and a necessary prerequisite for the rest of the work.

After that, is there any reason soup3 can't be a separate fetch backend instead of replacing the soup2 one? The hard work of abstracting the fetch backend has already been done, so I don't see a good reason not to leave the soup2 one alone. That would allow ostree and flatpak to migrate in a much cleaner way and let consumers using the soup2 backend continue to do so. At some point in the future the soup2 backend could be dropped.

dbnicholson avatar Feb 20 '22 21:02 dbnicholson

well, at this point soup2 and soup3 share a lot of code; if you prefer that soup2 code remains mostly untouched and soup3 code is made separate, I can do that too

i was planning to split the GUri changes all along, so sure

q66 avatar Feb 20 '22 21:02 q66

I personally think you should leave the soup2 code entirely alone. That ensures there are no regressions introduced and removing the backend later is simpler. At that point any duplication just goes away and there are no leftover abstraction layers from soup2/3 code sharing. Maybe other ostree developers feel differently, though.

dbnicholson avatar Feb 20 '22 22:02 dbnicholson

yeah, sounds good; let's do that

q66 avatar Feb 20 '22 22:02 q66

Dan's suggestion SGTM! It will definitely be easier for distributors to avoid a chained flag day that way.

(Although, have the recursive dependencies of libflatpak been analyzed? e.g. does gnome-software also use libsoup directly? I'd be surprised if it didn't.)

cgwalters avatar Feb 21 '22 19:02 cgwalters

I've previously compiled a more or less complete list of all software that uses libsoup (https://gitlab.gnome.org/GNOME/libsoup/-/issues/218); we're gradually helping upstreams port things over

i'm also working on a linux distribution that currently uses libsoup3 only, so that gets me a testbed to verify things

q66 avatar Feb 21 '22 19:02 q66

ok, first part of the work is at https://github.com/ostreedev/ostree/pull/2551

q66 avatar Feb 21 '22 19:02 q66

this is now rebased on the GUri work; the soup3 backend is added as a separate thing

i adjusted the CI to use soup2, unfortunately it's not available in Debian yet - I should figure out a way to get it tested (as there are probably bugs right now)

the httpd is ported to support both versions of libsoup at once

q66 avatar Mar 14 '22 23:03 q66

i adjusted the CI to use soup2, unfortunately it's not available in Debian yet

I'm hoping to upload libsoup3 to Debian unstable today, which should get it into testing in about a week if all goes well. It will never be available in Debian 11.

smcv avatar Mar 15 '22 13:03 smcv

sure, that's okay, the CI matrix has debian testing already, so we can either switch that one to soup3 or add a new one for testing + soup3, i'll leave that decision to the maintainers here

q66 avatar Mar 15 '22 13:03 q66

IMO, let's start with testing + explicit --with-soup3. I'm less familiar the FCOS parts of the CI, but that might be workable, too.

dbnicholson avatar Mar 15 '22 14:03 dbnicholson

I changed stuff so that --with-soup3 is used to enable libsoup3; I suggest that we wait for Debian testing to pick up libsoup3, then add the CI and fix up any remaining issues (we'll have plenty of time through the gnome 43 development cycle to get this in)

q66 avatar Mar 15 '22 18:03 q66

hm, not really sure what is causing those tests failures...

q66 avatar Mar 15 '22 19:03 q66

(we'll have plenty of time through the gnome 43 development cycle to get this in)

Now is the time to land changes for GNOME 43.

mcatanzaro avatar Jun 07 '22 13:06 mcatanzaro

@q66 Are you planning on picking this up again? Need help with it?

dbnicholson avatar Aug 30 '22 15:08 dbnicholson

the PR should be pretty much ready, there were some CI fails but I wasn't sure how to deal with them (probably would need somebody else to take a look)

q66 avatar Oct 12 '22 02:10 q66

rebased

q66 avatar Oct 12 '22 02:10 q66