appstream-glib icon indicating copy to clipboard operation
appstream-glib copied to clipboard

validate-relax: icons of "stock" type considered invalid?

Open decathorpe opened this issue 4 years ago • 21 comments

For example, I'm getting the following error message when trying to validate some elementaryOS desktop components:

? tag-invalid : stock icon is not valid [preferences-system-time]

With similar messages for other preferences-system-FOO icons. I checked, and this icon (and most of the others I'm seeing this error message for) are shipped with the hicolor, HighContrast, and Adwaita icon themes on fedora, so I'm not sure why they would be considered invalid stock icons (considering that the appstream spec even gives gimp as an example of a stock icon name):

https://www.freedesktop.org/software/appstream/docs/chap-CollectionData.html#tag-ct-icon

decathorpe avatar Apr 22 '20 09:04 decathorpe

We've got a legacy list of https://github.com/hughsie/appstream-glib/blob/master/libappstream-glib/as-stock-icons.txt -- I think the idea nowadays is that applications should be shipped with their own app icon rather than relying on one from the theme. I'm open to the idea of adding stock icons to that list if they exist in adwaita and hicolor.

hughsie avatar Apr 22 '20 09:04 hughsie

Ah, yeah that's exactly what I was thinking.

I see preferences-system-time is shipped with both hicolor and HighContrast icon themes, but it's missing from the list. It would be great if that entry could be added.

On the other hand, preferences-system-parental-controls-symbolic is only shipped in a symbolic variant in Adwaita, but missing non-symbolic variants in hicolor and HighContrast ...

The other icons I'm getting this error for (preferences-desktop-applications, preferences-desktop-online-accounts, preferences-system-power, preferences-desktop-sound seem like standard names for things that might exist (given that all the other icons do), but they don't exist in either Adwaita, hicolor, or HighContrast, so that's not going to help there :disappointed:

decathorpe avatar Apr 22 '20 10:04 decathorpe

Yes, that list is deliberately conservative. The logic is that if we don't ship an icon then we need to be 100% sure the users icon theme (or fallback) has a version of it, else we get the broken exec icon.

hughsie avatar Apr 22 '20 11:04 hughsie

But this xml file fully complains standard. Why I must ask upstream to change it?

Vascom avatar May 25 '20 19:05 Vascom

I have this problem with kio-gdrive - application included in KDE. And I see that almost all KDE applications use <icon type="stock"> and some of them removed validation from %check section of spec file.

Vascom avatar May 25 '20 20:05 Vascom

@hughsie In AppStream, "stock" kind of means "search in theme directories". So if an app icon is installed in /usr/share/icons/hicolor/*x*/apps/, it also counts as "stock" icon (under the application's name) and themes are actually able to override the particular icon (and there are some particularly extensive themes, like Papirus). I briefly considered naming those differently, but technically there is no difference, so that ideas was dismissed (after also getting feedback from you on IRC ^^) @Vascom If it is an option and you need some kind of very quick fix, you could (temporarily) validate with appstreamcli, which is used by KDE CI upstream anyway to check metainfo files. Otherwise, a relaxed check for this in asutil would be needed. That said, if the app in question does ship a .desktop file, explicitly defining a stock icon in the metainfo file isn't essential. Usually people do that for apps which don't have a .desktop file, such as addons/drivers/consoleapps/services/..., or if a .desktop file is generated from a metainfo file by the build system.

ximion avatar May 25 '20 21:05 ximion

We've got a legacy list of https://github.com/hughsie/appstream-glib/blob/master/libappstream-glib/as-stock-icons.txt -- I think the idea nowadays is that applications should be shipped with their own app icon rather than relying on one from the theme. I'm open to the idea of adding stock icons to that list if they exist in adwaita and hicolor.

Can we please also add the ones for Breeze too? It's causing failures with appstream-util for Fedora KDE. I think it counts at the same level as Adwaita for stock icon names.

Conan-Kudo avatar Nov 17 '23 11:11 Conan-Kudo

I think in 2023 we out to remove the concept of stock icons, and not add to the list -- tbh.

hughsie avatar Nov 17 '23 14:11 hughsie

To be honest, I don't care what is done as long as this stops failing KDE applications when they are reasonably following their spec for stock icons.

Conan-Kudo avatar Nov 17 '23 14:11 Conan-Kudo

Are the stock icons used by KDE also present in the other icon themes?

hughsie avatar Nov 17 '23 17:11 hughsie

Yes, I believe so. @pointedstick can explain more, but KDE Plasma supports icon themes and people use this to supplant stock icons all the time.

Conan-Kudo avatar Nov 17 '23 18:11 Conan-Kudo

FWIW, AppStream places no restrictions on stock icon names at all, and expects software centers to prefer them if the currently active icon theme has them, but otherwise to ignore them and use a cached or remote icon instead.

Why can't Fedora use appstreamcli? AFAIK, it's the only place that still uses appstream-util, and the latter does not support any AppStream features of the past 3-4 years.

ximion avatar Nov 17 '23 19:11 ximion

Because appstream's validation doesn't matter if appstream-builder skips over it when we generate our data.

Conan-Kudo avatar Nov 17 '23 19:11 Conan-Kudo

I think in 2023 we out to remove the concept of stock icons, and not add to the list -- tbh.

IMO that's a valid decision to make in client software but not in cross-platform tooling, because then it locks everyone using that tooling into that decision, including orgs like KDE which do not make the same decision.

Pointedstick avatar Nov 17 '23 19:11 Pointedstick

Because appstream's validation doesn't matter if appstream-builder skips over it when we generate our data.

It obviously does though, as this bug shows. The stuff asglib-builder creates should also validate fine with appstreamcli (but I'd need to test if that's still true with AS 1.0). But this leads to another question: Why can't you use appstream-generator? Since Fedora ships the data in a package anyway, you would just need to run asgen and package its output - this could even be automated, using asgen's incremental runs. There's even a Flatpak available for it (and a Snap ^^).

If there's any particular missing feature (most stuff in the bugtracker is convenience, which is important of course, but shouldn't be a blocker), let me know and I can see if it could be added.

ximion avatar Nov 17 '23 19:11 ximion

No, this bug is that appstream-glib does not successfully validate (which is what matters when you use appstream-builder to generate the index). What appstream says does not matter because it isn't the problem.

Conan-Kudo avatar Nov 17 '23 19:11 Conan-Kudo

It absolutely is the problem, because upstream projects use modern AppStream and want to rely on features it provides, but Fedora artificially limits them (or creates extra busy work for Fedora package maintainers that they do not need to do). The appstream-glib project has a WARNING: appstream-glib is heavy maintenance mode, use appstream instead warning message in its README file for a reason.

ximion avatar Nov 17 '23 20:11 ximion

It is not. Whether asglib is in maintenance mode or not has no impact on appstream. The only reason this is a problem for you is because @hughsie decided to implement GNOME opinions in asglib rather than leaving it to be more generic.

Fedora artificially limits them (or creates extra busy work for Fedora package maintainers that they do not need to do)

This is not just Fedora. The entire RPM ecosystem uses this project for AppStream repodata indexes.

For better or worse, someone decided we should have two implementations where some distributions must use one and some distributions must use the other.

You want to fix it? We need the appstream-util validate-relax, appstream-util install, and appstream-util mirror-screenshots behaviors in appstreamcli. We also need the semantics of appstream-builder in appstream-generator or https://github.com/rpm-software-management/createrepo_c/issues/75 implemented. Personally, I would prefer the latter because then we could actually do this properly in Fedora with AppStream repodata rather than packaged indexes.

For reference, this is the script used to build the appstream repodata index files: https://github.com/hughsie/appstream-scripts/blob/master/fedora/fedora-rawhide.sh

And here's the RHEL version: https://github.com/hughsie/appstream-scripts/blob/master/rhel/rhel-9-candidate.sh

Third party repos do some variation of this blog post: https://blogs.gnome.org/hughsie/2016/04/27/3rd-party-fedora-repositories-and-appstream/

Conan-Kudo avatar Nov 17 '23 20:11 Conan-Kudo

For better or worse, someone decided we should have two implementations where some distributions must use one and some distributions must use the other.

Everyone should use AppStream (not -glib) at this point.

It is not. Whether asglib is in maintenance mode or not has no impact on appstream.

It absolutely does, because I get to deal with the bug reports by annoyed users who think this whole thing is a lot of hassle with some tools having different interpretations.

You want to fix it? We need the appstream-util validate-relax, appstream-util install, and appstream-util mirror-screenshots behaviors in appstreamcli.

appstream-util validate-relaxappstreamcli validate --no-net

appstream-util installappstreamcli put

appstream-util mirror-screenshotsappstreamcli compose --no-partial-urls --media-dir=DIR --data-dir=/tmp/dummy (but what do you even need that for separately? That should really be part of the compose step!)

For reference, this is the script used to build the appstream repodata index files: https://github.com/hughsie/appstream-scripts/blob/master/fedora/fedora-rawhide.sh

That script would actually be pretty easy to port, either using appstream-generator or just plain appstreamcli compose. I'll have a look at the script, but I am no Fedora expert, so can only go so far.

ximion avatar Nov 17 '23 21:11 ximion

The only piece not mentioned in that script is that he has a local mirror of the Fedora repositories, which you can find information on here: https://fedoraproject.org/wiki/Infrastructure/Mirroring

Conan-Kudo avatar Nov 17 '23 21:11 Conan-Kudo

The only piece not mentioned in that script is that he has a local mirror of the Fedora repositories

I don't have the resources for that, but luckily appstream-generator doesn't even need that! (at the expense of on-demand package downloads). (Still I don't have the time to process the entire Fedora repo, I leave that for people with an idle server at their disposal).

I didn't need to change anything on appstream-generator for Fedora, it just worked. Still, I recommend using the Git version, as I ported the rpmmd backend away from std.xml and added the on-demand download feature to this backend, which is needed for the script below. Check out this repository: https://github.com/ximion/appstream-fedora-config Running the command in README using the current asgen Git should yield an export/data directory. You just need to package the contents of export/data/$FEDORA_VERSION/Everything/ in your package, and upload export/media to the remote location mentioned in the config file (media is screenshots, but also high-resolution remote icons). You may also want to upload export/html to some location where people can inspect it to see why their application isn't available in the metadata. The export/hints directory contains the same stuff as export/html, but in machine-readable formats (at Debian, various automated systems use this data for QA).

ximion avatar Nov 19 '23 21:11 ximion