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

misc: Conditionally port to libsoup3 #4582

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

mcrha
Copy link
Contributor

@mcrha mcrha commented Nov 16, 2021

Use --with-soup3 configure option to build with libsoup3. If not specified,
uses libsoup2. The libsoup API version is exposed in the flatpak.pc file
as well, thus the library users can check they use the same libsoup API
version as the flatpak library.

common/flatpak-oci-registry.c Outdated Show resolved Hide resolved
@mcrha
Copy link
Contributor Author

mcrha commented Nov 23, 2021

I updated the branch. It behaves differently now:

  • build the libsoup3 version by default - 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.
  • use SoupURI or GUri, according to the libsoup version, regardless of the glib version

This depends on hughsie/appstream-glib#421 , for the libsoup3 build. That change is not needed for the libsoup2 build.

@mcrha
Copy link
Contributor Author

mcrha commented Nov 23, 2021

Richard mentioned on the appstream-glib merge request that a soname version should be probably changed too. That would involve also the .pc file name change, thus both versions can be installed at the same time on a single machine.

What do you think?

@mcrha
Copy link
Contributor Author

mcrha commented Nov 23, 2021

Richard mentioned on the appstream-glib merge request that a soname version should be probably changed too. That would involve also the .pc file name change, thus both versions can be installed at the same time on a single machine.

What do you think?

Some of the required changes depend on the fact whether you want to allow installing both builds in parallel. If not, then a simple soname version bump for libsoup3 build is perfectly fine.

@mcrha
Copy link
Contributor Author

mcrha commented Nov 23, 2021

Some of the required changes depend on the fact whether you want to allow installing both builds in parallel. If not, then a simple soname version bump for libsoup3 build is perfectly fine.

Please see the discussion in hughsie/appstream-glib#421 , the parallel install-ability is not a requirement, thus the later can be done.

I did not get how the soname version is constructed (or better how it should be changed) in the configure.ac, thus I did not update the merge request with such change.

configure.ac Outdated Show resolved Hide resolved
@mcrha
Copy link
Contributor Author

mcrha commented Feb 17, 2022

Use --with-soup3 configure option to build with libsoup3. If not specified,
uses libsoup2.

As the time moved, I realized this was not a good idea, it should rather default to soup3 and have an option --with-soup2, which is a common way to do it. The idea behind it is that the libsoup3 will be a new standard in the future, thus better to compile with it by default (thus the builders do not need to carry the option in their build scripts ad infinity).

@mcatanzaro
Copy link
Collaborator

As the time moved, I realized this was not a good idea, it should rather default to soup3 and have an option --with-soup2, which is a common way to do it. The idea behind it is that the libsoup3 will be a new standard in the future, thus better to compile with it by default (thus the builders do not need to carry the option in their build scripts ad infinity).

I would actually remove the libsoup2 support. flatpak is not a library that would cause issues with porting other applications, so there's no reason to retain compatibility with libsoup2.

@smcv
Copy link
Collaborator

smcv commented May 26, 2022

I would actually remove the libsoup2 support. flatpak is not a library that would cause issues with porting other applications, so there's no reason to retain compatibility with libsoup2.

(a) Don't we want to be able to backport Flatpak to ye olde LTS distributions (Debian, Ubuntu LTS, RHEL), so that users of LTS distributions can use Flatpak to run new apps?

(b) flatpak is a library (among other things), gnome-software uses it.

@mcatanzaro
Copy link
Collaborator

(a) Don't we want to be able to backport Flatpak to ye olde LTS distributions (Debian, Ubuntu LTS, RHEL), so that users of LTS distributions can use Flatpak to run new apps?

No strong preference from me.

(b) flatpak is a library (among other things), gnome-software uses it.

Hm, good point, I forgot about libflatpak. OK, Milan's existing plan seems fine, then.

@nanonyme
Copy link
Contributor

nanonyme commented Jun 1, 2022

What is the actual discussion that needs to be resolved here to proceed? Parallel-installability of something?

Build for libsoup3 by default. Use --with-soup2 configure option to build
with libsoup2 instead. The libsoup API version is exposed in the flatpak.pc
file as well, thus the library users can check they use the same libsoup API
version as the flatpak library.
@mcrha
Copy link
Contributor Author

mcrha commented Jun 2, 2022

What is the actual discussion that needs to be resolved here to proceed?

I rebased the merge request against the latest master.

Parallel-installability of something?

It depends what the decision will be. Having the distro always use one or the other libsoup version would be really much easier than to maintain two versions.

@nanonyme
Copy link
Contributor

nanonyme commented Jun 2, 2022

I consider it s very niche thing distro would need parallel libflatpaks with soup2 and soup3 at the same time. Seems not worth supporting upstream. @smcv any thoughts?

@nanonyme
Copy link
Contributor

nanonyme commented Jun 2, 2022

If distro needs it, it can be supported through downstream patching like libcurl-gnutls anyway.

@mcatanzaro
Copy link
Collaborator

It depends on how much software is using libflatpak. If you do not have many dependencies, then it's not a big deal. But if you do, a distro may find it effectively impossible to upgrade if it's not parallel-installable.

@alexlarsson
Copy link
Member

I feel there is a potential risk for updates here. Suppose you're using gnome-software to update libflatpak, and your gnome-software links to libflatpak. If the libflatpak update gets updated, and g-s not, and the new lfp uses soup3, then the gnome-software will fail to start the next time as it links in both soup2 and soup3.

I wonder if it is possible to avoid this though? Maybe distros really just have to be very careful.

Also, what is the libostree approach to soup3? It seems to be soup2 only. I guess you can avoid conflict there by linking it to libcurl instead. Should we do that in flatpak too?

I really want to avoid having the libflatpak soname dependent on a build option though. Especially for a technically internal dependency.

@mcrha
Copy link
Contributor Author

mcrha commented Jun 3, 2022

I wonder if it is possible to avoid this though? Maybe distros really just have to be very careful.

I do not think you can avoid partial updates (users updating only smaller parts, not the whole distro/all packages at once). The distros still should be careful, that's true, though it was always like this.

The trick in the build time is to verify the correct libsoup version is used for the known dependencies. An example is here:

  flatpak_soupapiversion = flatpak.get_pkgconfig_variable('soupapiversion')
  if flatpak_soupapiversion == ''
    flatpak_soupapiversion = '2.4' # Pre libsoup3 support
  endif
  if flatpak_soupapiversion != libsoupapiversion
    error('flatpak was built against a different API of libsoup. @0@ instead of @1@.'.format(flatpak_soupapiversion, libsoupapiversion))
  endif

I guess you can avoid conflict there by linking it to libcurl instead. Should we do that in flatpak too?

That's an option too.

@alexlarsson
Copy link
Member

alexlarsson commented Jun 3, 2022

Honestly, I think the correct solution here is to drop soup for libcurl. We rely on ostree that uses libcurl anyway (on most distros i think, and at least its not moving to soup3). This way we avoid libflatpak makign other things unstable by getting conflicting soup3/2 in the same process.

I don't actually think this is a lot of work even, we can steal the code from ostree, and the http code is quite centralized in the codebase.

@nanonyme
Copy link
Contributor

nanonyme commented Jun 3, 2022

Is there some reason why libsoup would be preferrable here? I recall some comments about proxy support differences.

@mcatanzaro
Copy link
Collaborator

Is there some reason why libsoup would be preferrable here? I recall some comments about proxy support differences.

I've been told libcurl does not support web proxy autoconfig (so presumably it does not use libproxy?). Presumably some businesses that are currently using flatpak would not be able to do so anymore?

@nanonyme
Copy link
Contributor

nanonyme commented Jun 3, 2022

To be fair, if RHEL uses ostree with libcurl, it might not that much change current situation. (I was only able to find out Fedora spec which proves Fedora uses curl with ostree but not RHEL)

@alexlarsson
Copy link
Member

Well, if you need autoconfig, then you need to build ostree with soup, and since that only does soup2 we can't make flatpak soup3. So, either port both to soup3 (which still has potential conflicts with other libs) or flatpak to curl. It seems like curl is the better choice atm.

@alexlarsson
Copy link
Member

And yes, rhel ostree is curl.

@mcrha
Copy link
Contributor Author

mcrha commented Jun 7, 2022

And yes, rhel ostree is curl.

For what it's worth, the same as Fedora. Furthermore, the .spec file claims it uses libsoup only for internal tests and rpm -q --requires ostree ostree-libs ostree-devel | grep soup returns nothing (while it has a hit for curl).

Nonetheless, this is Fedora. It looks like the ostree itself can use libsoup instead of curl too (it's a build-time option, if I read it correctly), thus using curl can be safer for flatpak libs. It cannot be safer for the flatpak libs users, because they still need to take care of the all flatpak libs dependencies to use the same libsoup version.

@nanonyme
Copy link
Contributor

nanonyme commented Jun 7, 2022

It can definitely be safer. It is one less moving part for flatpak lib consumers when migrating to libsoup3.

@janbrummer
Copy link

Please consider supporting libsoup3 as we (in the company) are forced to use a pac proxy which is not supported within curl. Thus this change would cause serious flatpak problems.

@alexlarsson
Copy link
Member

@janbrummer Well, that would require ostree to support soup3 too. I'm not sure that is happening. I wouldn't mind if we supported both as a compile option, but again, given that most people build ostree w/ curl I think that is the best approach for default.

@mcatanzaro
Copy link
Collaborator

There is a pull request for ostree here: ostreedev/ostree#2547

@nanonyme
Copy link
Contributor

nanonyme commented Jun 8, 2022

As one of the people maintaining flatpak build as part of freedesktop-sdk project curl option would definitely be worth considering if it existed.

alexlarsson added a commit to alexlarsson/flatpak that referenced this pull request Jun 13, 2022
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.
alexlarsson added a commit to alexlarsson/flatpak that referenced this pull request Jun 13, 2022
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.
alexlarsson added a commit to alexlarsson/flatpak that referenced this pull request Jun 13, 2022
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.
alexlarsson added a commit to alexlarsson/flatpak that referenced this pull request Jun 13, 2022
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.
alexlarsson added a commit to alexlarsson/flatpak that referenced this pull request Jun 13, 2022
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.
alexlarsson added a commit to alexlarsson/flatpak that referenced this pull request Jun 13, 2022
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.
alexlarsson added a commit to alexlarsson/flatpak that referenced this pull request Jun 13, 2022
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.
alexlarsson added a commit to alexlarsson/flatpak that referenced this pull request Jun 13, 2022
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.
alexlarsson added a commit to alexlarsson/flatpak that referenced this pull request Jun 13, 2022
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.
alexlarsson added a commit to alexlarsson/flatpak that referenced this pull request Jun 13, 2022
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.
@alexlarsson
Copy link
Member

The preparatory work for curl support is here:
#4943

alexlarsson added a commit to alexlarsson/flatpak that referenced this pull request Jun 14, 2022
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.
alexlarsson added a commit that referenced this pull request Jun 16, 2022
As discussed in #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.
@mcrha
Copy link
Contributor Author

mcrha commented Jul 19, 2022

I suppose this should be closed in favor of the #4943, right?

gnomesysadmins pushed a commit to GNOME/gnome-software that referenced this pull request Aug 25, 2022
The `soupapiversion` variable had been added in [1], but it did not
land yet and possibly never will. The flatpak library can be built
with libcurl instead, which should be detectable somehow too. As there
is currently no way to know what libsoup version the flatpak library
had been built with, if any, drop the check in the build script.

Closes https://gitlab.gnome.org/GNOME/gnome-software/-/issues/1877

[1] flatpak/flatpak#4582
@mcrha
Copy link
Contributor Author

mcrha commented Aug 25, 2022

I suppose this should be closed in favor of the #4943, right?

Just a note, if this is still relevant, there should be made a change to indicate in the .pc file that the flatpak library had been built with libcurl instead, thus any soupapiversion build-time test won't think the libsoup API version being used was 2.4 (it is the current common practice to expect inexistent variable means 2.4 API, because many projects cannot choose between libsoup and libcurl (and any other similar library).

@TingPing
Copy link
Member

@mcrha #5054

@dbnicholson
Copy link
Contributor

FWIW, the ostree PR was merged and was released in ostree v2023.3. In case anyone cares to keep working on a soup3 backend for flatpak.

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

9 participants