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

WIP: Port to libappstream #418

Closed
wants to merge 7 commits into from

Conversation

pwithnall
Copy link
Collaborator

appstream-glib is mostly unmaintained, and appstream is more
actively developed (and up to date with the AppStream specification).

This ports flatpak-builder to use the command line tools from appstream
(appstreamcli and appstreamcli-compose) rather than the ones from
appstream-glib (appstream-util, appstream-builder and
appstream-compose).

This should result in no changes for most things, but should fix
propagation of <requires>/<recommends> elements, screenshot
<caption>s, and other elements which have been added to the AppStream
specification recently.

For example, see flathub/flathub#2439

Signed-off-by: Philip Withnall pwithnall@endlessos.org

@pwithnall
Copy link
Collaborator Author

See flatpak/flatpak#4426 for the related flatpak changes.

CC @ximion

@TingPing
Copy link
Member

TingPing commented Sep 21, 2021

My main worry is if there are subtle differences between the behavior of the CLI tools that affect Flathub. But in general it makes sense to move to this.

@pwithnall
Copy link
Collaborator Author

Yeah, this is going to need some good testing.

@nanonyme
Copy link
Contributor

Note I don't think this is going to work. flatpak-builder is supposed to support newer and older runtimes so the command in sandbox probably needs to remain parameter by parameter (and command name) identical. There could probably be a shim that passes to libappstream.

@ximion
Copy link
Contributor

ximion commented Sep 21, 2021

Screenshot captions? Wow, those have been part of AppStream since the February 2014 release! I had no idea this was missing!
appstreamcli-compose is rather new and currently not installed in $PATH to avoid issues - if it is installed, it can be run via appstreamcli compose, but I'm not sure yet whether I like this behavior.

Compose will cache screenshots in newer versions soon, I am not sure of you want this (feature can be disabled). There probably are differences, appstreamcli-compose and appstream-compose are quite different in what they can do, but the resulting data should be compatible with appstream-glib and older libappstream versions (older versions will just ignore any new elements). So I don't expect much breakage in other tools which just consume the metadata.

@nanonyme
Copy link
Contributor

As said, the very fact the command name is different is a starting point for pain here. And it looks like parameters are not. If this is merged and released, it means this flatpak-builder version can no longer be used to build any apps using old runtimes. This does not seem acceptable. We need some approach that will be backwards and forwards compatible.

@ximion
Copy link
Contributor

ximion commented Sep 21, 2021

I see a few options we have:

  • Make sure all old runtimes include AppStream, so it'd have to be retroactively added for them to work again
  • Make people not use a newer flatpak-builder with older runtimes (which basically means force everyone to upgrade)
  • Add some compatibility code so flatpak-builder can use appstreamcli if that's available, but fall back to appstream-util if it's not there for older runtimes

I think the last option would probably be the most compatible and the one associated with the least amount of pain.

@nanonyme
Copy link
Contributor

I'm pretty sure Flathub infra would not work with the second option (@barthalion can probably say something about that) so that's probably a no-go. The first option is nigh impossible. Even if we could sort out freedesktop-sdk , there are gnome runtime which need to be dealt with separately. And then there's external child runtimes.

@TingPing
Copy link
Member

The third option is the only realistic one. Supporting both.

@barthalion
Copy link
Member

I think the third option is the way to go. Ironically, I think using appstream-util from the runtime was not the best idea, so from Flathub's perspective, it would be vastly preferred to use appstream from the host, and fall back to appstream-util inside the sandbox.

@ramcq
Copy link
Contributor

ramcq commented Sep 22, 2021

Boo host dependency. If flatpak-builder is going to invoke a binary from outside the build environment, shouldn't it be a Flatpak? Then it's independent of whatever runtime it needs, the Flathub buildbot/flat-manager can use the same flatpak, and we can expect/assume the same behaviour from flatpak-builder of this version or later, regardless of the host or the SDK being used. (We could potentially even remove the fallback behaviour and just say, flatpak-builder after X.Y will always need org.whatever.appstream-badger to be installed.)

@nanonyme
Copy link
Contributor

nanonyme commented Sep 22, 2021

@ramcq you mean like org.flatpak.Builder? FWIW flatpak-builder is primarily invoking tools from outside sandbox (stripping, fetching sources). Appstream handling is an exception here.

@ramcq
Copy link
Contributor

ramcq commented Sep 22, 2021

No I mean like we provide an appstreamcli equiv of https://github.com/flathub/org.freedesktop.appstream-glib and flatpak-builder uses that going forward. Maybe with a host fallback? But appstream is an evolving spec and we've already suffered on our infra from being pinned to a host version of appstream-glib, which is why that flatpak exists. Let's not re-invent that problem here?

@barthalion
Copy link
Member

Being a host dependency is exactly what lets us to easily decouple it from runtimes, like we already do with org.freedesktop.appstream-glib, which is a symlink to appstream-util on the build infra. I don't think tying flatpak-builder directly to a flatpak hosted on Flathub is something everyone will be happy about.

@barthalion
Copy link
Member

Host dependency means org.flatpak.Builder could be theoretically self-contained as well.

@ramcq
Copy link
Contributor

ramcq commented Sep 22, 2021

Being a host dependency is exactly what lets us to easily decouple it from runtimes, like we already do with org.freedesktop.appstream-glib, which is a symlink to appstream-util on the build infra. I don't think tying flatpak-builder directly to a flatpak hosted on Flathub is something everyone will be happy about.

Alright, makes sense. Host vs flatpak can be a deployment decision.

@ximion
Copy link
Contributor

ximion commented Sep 22, 2021

Since AppStream may add new things in future, having its revision pinned to the runtime is probably unexpected - when it's a host dependency, Flatpak could say "require at least version X or higher" and be done with it, while when it's part of the runtime users of older runtimes can't get new stuff until AppStream is updated in the runtime as well.
If it would be part of the host, you'd also only need to test AppStream-related changes once with one runtime, instead of verifying them with all older versions as well. I see arguments for both, but having it as host dependency does make some sense to me.

We also haven't had any breaking changes in AppStream for many years now, and the 1.0 release will just be removal of deprecated stuff (which hopefully will not break much stuff), so I think the only issue you might run into is a stricter validator (which I don't think is a huge issue, as only errors and warnings fail validation, and those are all quite severe issues that need to be fixed).

@mwleeds mwleeds added this to the 1.3.1 milestone Nov 24, 2021
sonnyp added a commit to flathub/re.sonny.Junction that referenced this pull request Dec 16, 2021
On flathub.org/builds - appstream-util mirror screenshots runs in the sandbox where glib fails to load glib-networking. Bundling glib-networking fixes the problem.

https://discourse.flathub.org/t/missing-screenshots/1852/6

There is a WIP PR that moves this outside of the sandbox

flatpak/flatpak-builder#418
@JakobDev
Copy link
Contributor

What's the current state of this? Flatpak itself has already switched to libappstream in the latest pre-release and I really want to use the new features like in my Programs.

@barthalion
Copy link
Member

Rebased it on top of the main branch and added a commit to run appstreamcli commands on the host. CI will likely break but it's a start.

@barthalion
Copy link
Member

I've kicked off a test build at flathub/org.flatpak.Builder#81. When I manage to build it, I will do some testing on Flathub apps.

@barthalion
Copy link
Member

OK, so this is something that produces a supposedly working appstreamcli invocation:

Running appstreamcli-compose
FB: Running: appstreamcli compose --prefix= --origin=flatpak --result-root=/var/home/bpiotrowski/dev/flathub/org.gabmus.whatip/.flatpak-builder/rofiles/rofiles-q1EnZD/files --components=org.gabmus.whatip /var/home/bpiotrowski/dev/flathub/org.gabmus.whatip/.flatpak-builder/rofiles/rofiles-q1EnZD/files
Only accepting component: org.gabmus.whatip
Processing directory: /var/home/bpiotrowski/dev/flathub/org.gabmus.whatip/.flatpak-builder/rofiles/rofiles-q1EnZD/files
Composing metadata...
Success!

But the resulting builddir/files/share/swcatalog/xml/flatpak.xml.gz is empty, and I don't see app-info anywhere. Have I missed something about how the new compose command works?

@barthalion
Copy link
Member

build-update-repo confirms that it's not really correct:

> flatpak build-update-repo repo
Updating appstream branch
No appstream data for app/org.gabmus.whatip/x86_64/localtest: No such file or directory: /files/share/app-info

So I guess that build-update-repo needs to start reading files from the swcatalog directory and the compose command needs to actually produce a proper XML file.

@smcv
Copy link
Collaborator

smcv commented May 18, 2022

We use flatpak-builder through flatpak at https://github.com/flathub/org.flatpak.Builder/, so we can control all dependencies. I mentioned the need for having newer AppStream in some PPA as we need CI to pass here.

OK, that's less concerning.

The Flatpak PPAs are really serving two or three purposes:

  • they're a convenience for Ubuntu users who want to run newer Flatpak apps but are not provided with backports by their distro
  • they're used by Flathub infrastructure
  • they're used to run CI for flatpak-builder

For the "convenience for Ubuntu users" use-case, I don't really want to add an appstream backport to the set of packages that we aim to support, because it feels like upgrading flatpak shouldn't apply potentially disruptive changes to OS components that depend on libappstream.

For Flathub infrastructure, obviously whatever the Flathub sysadmins are comfortable with is fine.

Similarly, for CI, whatever we're OK with build-depending on is fine.

It might make sense to have a new ppa:flatpak/for-flathub or ppa/flatpak:builder-development (or something like that) alongside ppa:flatpak/development and ppa:flatpak/stable, so that we can put more-disruptive changes into the new PPA without causing unexpected package upgrades for users of flatpak/development and flatpak/stable?

@nanonyme
Copy link
Contributor

Hmm, but isn't this about backporting libappstream for flatpak-builder, not flatpak itself? Former is used by developers who probably know what they're doing whereas latter is used by end-users who might not.

@smcv
Copy link
Collaborator

smcv commented May 18, 2022

Hmm, but isn't this about backporting libappstream for flatpak-builder, not flatpak itself? Former is used by developers who probably know what they're doing whereas latter is used by end-users who might not.

Yes, but if the backport is in the same PPA that's used by end-users of Flatpak, then adding that PPA as an apt source will upgrade their libappstream, whether it's actually necessary or not.

@ximion
Copy link
Contributor

ximion commented May 22, 2022

@smcv

For the "convenience for Ubuntu users" use-case, I don't really want to add an appstream backport to the set of packages that we aim to support, because it feels like upgrading flatpak shouldn't apply potentially disruptive changes to OS components that depend on libappstream.

Fair enough - AppStream isn't a Flatpak-exclusive component after all, Ubuntu uses it for other purposes as well. Many AppStream upgrades are fairly safe nowadays, issues occur specifically when newer tags are added, like e.g. the screenshot image/video localization, which software centers initially didn't know about. This is an issue with older libappstream releases, while newer versions will abstract this away so older frontends continue to work without issues. If Flatpak apps wanted to use these, upgrading AppStream for the frontends would be needed, or these features should not be used until newer AppStream releases have percolated through the various distros and packaging solutions (the strategy I've used in the past).

In any case, most stable distros have recent enough AppStream versions now and those changes aren't that frequent. And the issue here simply falls into the "unfortunate bug" category.
That said, flatpak-builder does need quite a recent AppStream version simply because a bunch of changes to help Flatpak have only been added in recent releases, as originally (lib)appstream-compose was more a utility for generating data from distribution packaging.

The proposed PPA split makes a lot of sense to me, as it would generate the absolute smallest amount of potential friction.

I've also made the latest AppStream release available for Ubuntu 22.04 in a PPA here: https://launchpad.net/~ximion/+archive/ubuntu/appstream/+packages?field.name_filter=&field.series_filter=jammy
Also, 0.15.4 is out now and contains mostly validation and compose bugfixes and enhancements :-)

@nanonyme
Copy link
Contributor

nanonyme commented May 31, 2022

@ximion so CI is now failing because libappstream did not have compose support present. This is actually a quite important thing. libappstream build system declared compose as an experimental feature until quite recently so it's unlikely to be enabled by distros. It's still disabled by default. Also it has a quite large set of dependencies which might make it uncomfortable for distros https://github.com/ximion/appstream/blob/master/compose/meson.build#L62-L68.

@nanonyme
Copy link
Contributor

nanonyme commented May 31, 2022

One possible approach would be biting the bullet and introducing libappstream into sdk in freedesktop-sdk 22.08. We have already gotten it to build to the extent that we appstreamcli compose can generate data for runtime and extensions. Then the question would be is it okay if building with new freedesktop-sdk again requires a newer flatpak-builder or does appstream-glib still need to be persisted. Probably it's not okay and appstream-glib can only be removed in 23.08.

@ximion
Copy link
Contributor

ximion commented May 31, 2022

it's unlikely to be enabled by distros. It's still disabled by default.

Fedora, Arch, Debian and Ubuntu have it enabled by default for quite a while now. You could just depend on it and distros will enable this feature ;-) (And I think we are only missing OpenSUSE to cover 99% of Linux users)

Also it has a quite large set of dependencies which might make it uncomfortable for distros

Extremely unlikely. It does not have more dependencies than appstream-glib had, all of the dependencies are fairly common for systems with a GUI, and only the resulting compose components depend on the GUI stuff, so can be split out.
The main AppStream library is very light on dependencies as it may be used on systems which really do not want GUI components, but the compose library also doesn't depend on anything that KDE and GNOME don't already have by default.

One possible approach would be biting the bullet and introducing libappstream into sdk in freedesktop-sdk 22.08.

I think that would be a good idea in general. Means projects which validate their metadata in a test via appstreamcli won't need to disable that or build libappstream when creating their Flatpak, but could just run this out of the box.

@nanonyme
Copy link
Contributor

nanonyme commented May 31, 2022

I think that would be a good idea in general.

Note it's a deceptively complex idea. If flatpak-builder calls tool from runtime that is only present in new runtimes and not older ones, that imposes a version dependency on flatpak-builder and runtimes. As time goes, older runtimes would also have obsolete versions of libappstream. (as is now the case even with appstream-glib, hence why org.flatpak.Builder works for apps that flatpak-builder doesn't since it can patch flatpak-builder to call bundled appstream-glib)

@nanonyme
Copy link
Contributor

nanonyme commented May 31, 2022

Fedora, Arch, Debian and Ubuntu have it enabled by default for quite a while now. You could just depend on it and distros will enable this feature ;-) (And I think we are only missing OpenSUSE to cover 99% of Linux users)

Ubuntu 20.04 LTS does not build compose for libappstream as displayed by failing CI. This means all those dozens of Ubuntu derivatives do not have it either.

@ximion
Copy link
Contributor

ximion commented May 31, 2022

Ubuntu 22.04 LTS does have it though, so if you are on the latest LTS, things just work :-)
Can the CI be upgraded to use the latest Ubuntu LTS?

@nanonyme
Copy link
Contributor

nanonyme commented Jun 1, 2022

@ximion you're missing the point. There are like a dozen or so desktop distros basing on Ubuntu 20.04 LTS. They will gradually rebase to 22.04 LTS in a year or so. It is not correct to say atm libappstream compose is commonly available. Of course, this might not matter if people are not supposed to use flatpak-builder but instead org.flatpak.Builder.

@ximion
Copy link
Contributor

ximion commented Jun 1, 2022

But how many of these Ubuntu derivatives are actually relevant? One of the bigger ones, Pop!_OS has already been rebased on 22.04 months ago, Mint is expected to do that this month or next month. Even without derivatives, depending on which statistics you use Ubuntu still has a 33% market share, and if the latest LTS has a recent enough AppStream, it definitely is widely available and either already in use or just an upgrade away (available != in use).

The compose library is available on even older systems as it existed long before the binary (but the library API only has a stability guarantee since very recently, so using older versions is not a great idea).

I would agree that an insufficient AppStream version is a problem if the library that is used for running Flatpaks was the issue - however, that library has been available everywhere for many years now (even without ABI breaks), so is not a problem. The "problem" is solely the compose component, and you only need compose if you are building a Flatpak application. So, this is a developer tool. And for a developer tool, I think it is reasonable to expect people to upgrade to the latest stable version of their distribution or add a PPA if they want to have the latest development features available. It's not like appstream-glib-based Flatpak will break as soon as a newer version is available that uses a different tool.
If appstream-compose was part of the SDK and independent of the host that would of course solve all the issues even for people on older distributions who still want to upgrade Flatpak.

@JakobDev
Copy link
Contributor

Just a question: Flathub validate Appstream files. It currently uses appstream-util, which makes sense, because it is part of appstream-glib. But the validation is not as good as appstreamcli. There are currently a few Apps with invalid Appstream on Flathub e.g. flathub-infra/website#379. What will happen after the chance? Will the App not be able to push update until the Appstream is fixed (which is weird for someone who does not know much about the Baclground, because the Appstream was marked as valid earlier) or did it cause more problems?

@ximion
Copy link
Contributor

ximion commented Jun 15, 2022

Will the App not be able to push update until the Appstream is fixed

That is what I would expect to happen, yes. But I also think that's okay, it would pretty much be the same thing when appstream-util was updated and introduced new checks, and I think it's reasonable to expect an app maintainer to fix these issues.
To make it easier for them, Flathub could pass the --explain option to appstreamcli validate - that will generate a very verbose report, but will also make it easy to get information about the found issue.

@nanonyme
Copy link
Contributor

Freedesktop-sdk 22.08 is going to have both libappstream and appstream-glib in SDK. I'm deeply concerned these are writing metadata into different places and flatpak can only handle latter supposedly.

@JakobDev
Copy link
Contributor

@barthalion Any news on this?

@ximion
Copy link
Contributor

ximion commented Oct 1, 2022

This patch actually looks fine to me - @pwithnall, which tasks are left to do, and can I help with anything?

@pwithnall
Copy link
Collaborator Author

This patch actually looks fine to me - @pwithnall, which tasks are left to do, and can I help with anything?

Sorry, I’ve kind of checked out on this PR since people started talking about host vs runtime dependencies, which are well outside my area of expertise. (I was also away for a long time this summer.)

From my point of view, there’s nothing left WIP in the PR, but you should probably be asking @barthalion.

@nanonyme
Copy link
Contributor

nanonyme commented Nov 8, 2022

@ximion afaik the problem was that this simply does not work as is. There's another WIP branch against flatpak flatpak/flatpak#4904 which this work supposedly depends on.

@ximion
Copy link
Contributor

ximion commented Nov 26, 2022

@nanonyme That PR looks good to me, but we could also live without it by passing --data-dir to appstreamcli and redirect data to the old locations for compatibility - either solution should work.

@nanonyme
Copy link
Contributor

I suggest this is switched to --data-dir so it's not dependent on flatpak changes.

@barthalion
Copy link
Member

@Lunarequest will try to push this forward with my guidance. We found out it needs some restructuring to execute the compose command only once and pass screenshots mirroring when needed.

@ximion
Copy link
Contributor

ximion commented Feb 1, 2023

Nice, thanks! If there's anything you need me for, just ask - it also looks like I'll be able to make it to FOSDEM this year, so I'll be very easy to reach in case anyone of you is also there.

@barthalion
Copy link
Member

Superseded by #517

@barthalion barthalion closed this Feb 6, 2023
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.

10 participants