gnunn1
Make appstreamcli dependency optional
Part of the code that use appstreamcli is conditionnaly executed (validation) if the dependency is found, but another part is unconditionnaly executed (generation).
Wrap all tthe code in the condition and mark the dependency as optional.
Why? This will result in both a silently untranslated and uninstalled metainfo file, breaking discoverability of Tilix in software centers, and will break Flatpak builds due to missing release information in the (template) metainfo files. If there is any merit to having this at all (as appstreamcli is fairly lightweight and available pretty much everywhere), putting this behind a default-on build option would be much better, so people need to consciously ignore it rather than getting a silent semi-broken build.
Why?
Mainly consistency: if some code checks whether the dependency was found, I assumed it was expected to be an optional dependency.
As a matter of fact, I was updating the FreeBSD port for tilix and appstreamcli is not available on this platform ATM (same for Flatpak), so the related tasks can be ignored on this platform as far as I understand. When inspecting my build failure I saw I could skip this entirely and make the logic more consistent, hence this PR as it is now.
I have no objection to adjust this PR to add a meson option to opt-out of the appstreamcli stuff, how would you like me to name such an option @ximion? flatpak?
option('flatpak', type : 'boolean', value : true)
Perhaps it would be good enough, when appstreamcli is not found, to use i18n.merge_file on the metainfo file without first preprocessing it through appstreamcli in order to insert the NEWS contents?
It should still be discoverable by default, and translated as well... it would just not include release notes.
The Flatpak SDKs do include appstreamcli & Co though (at least any newer ones) and Flatpak itself depends on it - so as far as Flatpak is concerned, this really shouldn't be an issue.
This shouldn't be a problem any longer. If it still is for some reason, a new PR where the metainfo file is at least installed and translated without the NEWS entries would probably be okay as workaround (as long as these things are default-included). But appstreamcli is so ubiquitous that I doubt it's an issue to use it now.