Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial work to support multiple http libraries #4943

Merged
merged 16 commits into from Jun 16, 2022

Conversation

alexlarsson
Copy link
Member

As discussed in #4582 we might want to support libcurl in addition to soup2, and maybe also soup3. This is some preparatory work for this that centralizes all the current code using soup into a single file where it can easily be made switchable.

Copy link
Collaborator

@smcv smcv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the general idea of loosening the dependency like this.

common/flatpak-utils-private.h Outdated Show resolved Hide resolved
common/flatpak-utils.c Outdated Show resolved Hide resolved
This now finds the correct error message in the redhat oci repo for rhel.
As discussed in flatpak#4582 we
want ot use GUri for soup3, and if we want to use libcurl we might
as well also use it to avoid complex ifdefs, as we're linking to it
already via glib.

This imports a subset of GUri for older versions of glib.
Its still just a SoupSession, but now the implementation is more
centralized and can be something else down the line.
This allows:
 * getting http status
 * getting www-authenticate header
 * Doing HEAD instead of get

This is needed by the OCI registry code for authentication
This copies and simplifies some helpers from soup:
 * Encoding url queries
 * Parsing simple http header parameter lists

The goal is to use mostly GUri and a few extra helpers for the flatpak
internals, and then pass raw string uris to the http functions which
could then be backed by any kind of http implementation.
This will allow us to make the soup dependency optional.
@alexlarsson
Copy link
Member Author

Pushed what is essentially just a rebase with a slightly better commit layout, but it also has the GUri backport in a different file.

This miniminzes the soup implementation by moving it out of the
highlevel multiple-retry entry points and simplifying the
lower level part to use only one shared helper.

This will also make it easier to replace the soup specific
parts.
If build with curl (--with-curl, which is default) then we use libcurl
instead of libsoup as the http backend.
@mwleeds
Copy link
Collaborator

mwleeds commented Jun 15, 2022

I just built and installed this branch, confirmed that it's linking against libcurl according to ldd, and installed a flatpak with it, so I can confirm it works for me on Silverblue 36.

@mwleeds mwleeds closed this Jun 15, 2022
@mwleeds mwleeds reopened this Jun 15, 2022
This adds a separate, more modern CI build running on ubuntu 22.04
using curl, and leaves the old one around building against soup.

In addition, the modern one uses the system bwrap and dbus-proxy so
that we test these configurations too (and because the modern system
has good versions of these).

I also enabled running parallel make check again, hoping that
whatever made this hang is now fixed. We'll see.
@alexlarsson
Copy link
Member Author

Gah, it failes the new CI build due to that appstream bug...

@alexlarsson
Copy link
Member Author

Yeah, we need 0.15.3...

@alexlarsson
Copy link
Member Author

@ximion Is there an appstream 0.15.3 or later for ubuntu 22.04 somewhere?

@alexlarsson alexlarsson force-pushed the http-libs branch 3 times, most recently from 1483bab to e14261b Compare June 16, 2022 10:12
@alexlarsson alexlarsson force-pushed the http-libs branch 2 times, most recently from b42d23c to 2f0f1e0 Compare June 16, 2022 10:16
We need appstream >= 0.15.3 to get this fix:
  ximion/appstream#384

Without it the test-suite fails.
@alexlarsson
Copy link
Member Author

I've done basic testing of this and it seems to do the right things. Plus it passes the tests and we have CI of both codepaths, so I'm gonna merge this.

However, there could be second degree issues where thing work or don't work with particular webservers or whatnot, so we should get an unstable release out to get some testing of this.

@alexlarsson alexlarsson merged commit 4247e61 into flatpak:main Jun 16, 2022
@ximion
Copy link
Contributor

ximion commented Jun 16, 2022

Is there an appstream 0.15.3 or later for ubuntu 22.04 somewhere?

Yes, you can try the PPA: https://launchpad.net/~ximion/+archive/ubuntu/appstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants