builder-manifest: Run appstream-compose with no-partial-urls argument

[bbhtt]

See commits

Alternative to #576, closes #576

We can optionally pick up the meson/autotools bits from there, just to signal packagers that we ideally want >= 0.16.3 but then we have to bump CI or turn them into warnings.

[bbhtt]

For optional testability, I need the last commit of #649 a41f2e4 merged.

[bbhtt]

@ximion what do you think about this? atm the only use of appstreamcli in f-b is using compose but you last said compose api is not stable so we don't want to switch to it and using the api is also part of a larger cleanup.

linking to libappstream just to check the version seems a bit much, so this is an easy way out.

[ximion]

I'd say you do what you think is best for flatpak-builder - but I can offer my opinion and a bit of background information :-)

The CLI interfaces of both ascli-compose as well as ascli are pretty much guaranteed to remain backwards-compatible. However, the generated output isn't something you generally should rely on, unless it is any of the YAML/JSON output (that will not change as it is intended to be machine-readable).

That said, I can pretty much guarantee that the output of --version won't ever change. There is one caveat you need to be aware of: If, for whatever reason, you are using an appstreamcli built for one libappstream version with a libappstream that has a different version, passing that command will print both version numbers, which might confuse your parsing.

On partial URLs: This is a feature to allow for caching data while having the URL change and to allow for very cheap load balancing. E.g. if you have a database of components and change the URL of your CDN, you would have to go through each and every one to adjust the URL, while with partial URLs you just have to change the component catalog header. You can also - in theory - define multiple URLs where to get the assets from and have app-stores choose one at random, to distribute load on multiple sites.
So, this feature does have its purpose, just maybe not for Flatpak/Flathub ;-)

On depending on libappstream: Personally, TBH, I'd just do it here. Flatpak already depends on it, so you aren't introducing any exotic new dependency. Depending on the library is way more robust than any string parsing. You get the AS_CHECK_VERSION compile-time macro to enable/disable code depending on which AppStream is available if you want to use it in future. So, all around, even if it's just for checking versions I see no drawback and only advantages in depending on it.
The reason we avoided exposing AppStream in the past was because libappstream-glib was used by Flatpak, which used conflicting symbol names with libappstream, so, when the Flatpak library exposed it it broke every software center out there except for GNOME Software. This issue no longer exists, and I promised the Flatpak developers some refactoring on this front (which I may get to this month or next month, coincidentally!).

On libappstream-compose API: It's marked as unstable because it is "leaking" some internals like the font renderer that are needed by tools like appstream-generator so far. The API is marked as unstable because the hope is to reduce it a bit in future, if possible. It's also missing a version test macro and a tiny bit of polish.
However, here's the secret: Ever since the library was introduced, it only broke API once. The AscCompose class has been API-stable for a very, very long time now. So as long as you do not use stuff like AscImage or AscCanvas, you'll probably be fine.
If you need it, I would also be open to just give it one or two more releases of appstream/appstream-generator to see if anything needs changing (asgen was just ported to C++, so there may be a bit of churn now), and then just stabilize the API. That wouldn't mean no more changes ever, but definitely a lot more conservative changes with a proper protocol (deprecation periods -> SONAME bump, instead of "API changed immediately between two releases"). What do you think?

So yeah, that's my (long) 2 cents ^^

[bbhtt]

However, the generated output isn't something you generally should rely on

Yea we don't rely on that.

That said, I can pretty much guarantee that the output of --version won't ever change.

Ok thanks.

There is one caveat you need to be aware of: If, for whatever reason, you are using an appstreamcli built for one libappstream version with a libappstream that has a different version, passing that command will print both version numbers, which might confuse your parsing.

Yea I'm handling that by choosing the library version.

[bbhtt]

That wouldn't mean no more changes ever, but definitely a lot more conservative changes with a proper protocol (deprecation periods -> SONAME bump, instead of "API changed immediately between two releases"). What do you think?

I think it makes sense to use the API to compose, as long as there aren't any differences in the output between cli and api. But if it was marked stable first it'd be nice.

f-b is just using this much:

flatpak-builder/src/builder-manifest.c

Lines 3034 to 3088 in d2abdda

	if (self->appstream_compose && appdata_file != NULL)
	{
	g_autofree char *origin = g_strdup_printf ("--origin=%s",
	builder_manifest_get_id (self));
	g_autofree char *components_arg = g_strdup_printf ("--components=%s,%s.desktop",
	self->id, self->id);
	const char *app_root_path = flatpak_file_get_path_cached (app_root);
	g_autofree char *result_root_arg = g_strdup_printf ("--result-root=%s", app_root_path);
	g_autoptr(GFile) xml_dir = flatpak_build_file (app_root, "share/app-info/xmls", NULL);
	g_autoptr(GFile) icon_out = flatpak_build_file (app_root, "share/app-info/icons/flatpak", NULL);
	g_autoptr(GFile) media_dir = flatpak_build_file (app_root, "share/app-info/media", NULL);
	g_autofree char *data_dir = g_strdup_printf ("--data-dir=%s",
	flatpak_file_get_path_cached (xml_dir));
	g_autofree char *icon_dir = g_strdup_printf ("--icons-dir=%s",
	flatpak_file_get_path_cached (icon_out));
	const char *opt_mirror_screenshots_url = builder_context_get_opt_mirror_screenshots_url (context);
	gboolean opt_export_only = builder_context_get_opt_export_only (context);
	if (opt_mirror_screenshots_url && !opt_export_only)
	{
	g_autofree char *url = g_build_filename (opt_mirror_screenshots_url, NULL);
	g_autofree char *arg_base_url = g_strdup_printf ("--media-baseurl=%s", url);
	g_autofree char *arg_media_dir = g_strdup_printf ("--media-dir=%s",
	flatpak_file_get_path_cached (media_dir));
	g_print ("Running appstreamcli compose\n");
	g_print ("Saving screenshots in %s\n", flatpak_file_get_path_cached (media_dir));
	if (!appstreamcli_compose (error,
	"--prefix=/",
	origin,
	arg_base_url,
	arg_media_dir,
	result_root_arg,
	data_dir,
	icon_dir,
	components_arg,
	app_root_path,
	NULL))
	return FALSE;
	}
	else
	{
	g_print ("Running appstreamcli compose\n");
	if (!appstreamcli_compose (error,
	"--prefix=/",
	origin,
	result_root_arg,
	data_dir,
	icon_dir,
	components_arg,
	app_root_path,
	NULL))
	return FALSE;
	}
	}

also for flathub, we also are setting ASC_COMPOSE_FLAG_PROPAGATE_CUSTOM through a patch but we probably want that in f-b too.

[ximion]

also for flathub, we also are setting ASC_COMPOSE_FLAG_PROPAGATE_CUSTOM through a patch but we probably want that in f-b too.

You know you can just pass --allow-custom= to ascli-compose with the custom keys you want to permit, right? ;-)

I think it makes sense to use the API to compose, as long as there aren't any differences in the output between cli and api.

As long as you use the same flags, the output is identical. All the CLI tool does is to call the respective API anyway.

[bbhtt]

ah i didn't knew about the --allow-custom flag.

[bbhtt]

I just remembered that we use the patch because we want to propagate all custom tags to our Flathub catalogue data.

Sometimes some distributors use flathub catalogue for their own app store and eg. wants to highlight their apps in their store. Usually they end up using some custom tag in metainfo for their apps and want that in catalogue.

Anyway once the api is stable and we can switch to it, we should be able to set a custom policy i think.

[bbhtt]

I wonder what others think of it. The options are

1. go with this for now and once compose api is stable switch to api fully

or

2. link to libappstream to just check the version for now (and in the future port the compose part to use api once it is stable)

[ximion]

Couldn't they go by developer-id if they wanted to highlight their own apps? There's also a merge mechanism in AppStream to edit components after the fact, but it was designed for Linux distributions, so I have no idea how well that would work for Flatpak: https://www.freedesktop.org/software/appstream/docs/chap-CatalogData.html#spec-asxml-tags

Or, well, you could just define the custom key-name they should use for this, that will solve it too ;-)

[ximion]

link to libappstream to just check the version for now (and in the future port the compose part to use api once it is stable)

It's worth noting that once you link to libas-compose, you also automatically link against libas, because the former builds upon the latter :-)

[bbhtt]

I tried the initial port to API here #664 but looks like there the private asc-fonts header is not available, so it doesn't compile by default.

[ximion]

I tried the initial port to API here #664 but looks like there the private asc-fonts header is not available, so it doesn't compile by default.

Upgrade to AppStream 1.0.6 or higher and that issue will be gone. Nothing in the public API uses anything from that header anyway, so alternatively you can also just drop that reference or provide an empty header.

[bbhtt]

I will get back to this a bit later. I realised this needs to be behind an arg something like --compose-no-partial-urls otherwise it breaks the behaviour in the stable release line. Then we can make a new 1.4.x release with this.

And drop/change it later for 1.5.0 or later when the appstream api is done.

[ximion]

What actually is the minimum AppStream release that you need to support? Looks like you have to support stuff from the 0.16.x stable series and can't yet fully depend on the 1.0-or-later series? (I also guess that backporting stuff to the 0.16.x stable branch won't help you if your versions are frozen).

[bbhtt]

Ideally as minimum as possible, so that it can go to LTS distros without much trouble. I can bump it to whatever is needed but that's when 1.5.0 (unstable) starts. Not in 1.4.x.

[ximion]

Yeah, that's fair, but I am asking because the last Ubuntu LTS, 24.04, has AppStream 1.0.2 which would be a much easier baseline for you.
Even on RHEL 10 you have that version. If you go to older RHEL/CentOS or Ubuntu 22.04, then you'll be in the 0.16.x series though.

[bbhtt]

I need to adjust the tests, since offline tests are wanted. Instead of testing for the screenshot tags, I will test for the icon tag which works offline.

[bbhtt]

Looks like sourceware is down...

[bbhtt]

It should be ready for review now.

[swick]

Nice!