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

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

Open
decathorpe opened this issue Apr 22, 2020 · 21 comments
Open

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

decathorpe opened this issue Apr 22, 2020 · 21 comments

Comments

@decathorpe
Copy link
Contributor

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

@hughsie
Copy link
Owner

hughsie commented Apr 22, 2020

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.

@decathorpe
Copy link
Contributor Author

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 😞

@hughsie
Copy link
Owner

hughsie commented Apr 22, 2020

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.

@Vascom
Copy link

Vascom commented May 25, 2020

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

@Vascom
Copy link

Vascom commented May 25, 2020

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.

@ximion
Copy link
Collaborator

ximion commented May 25, 2020

@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.

@Conan-Kudo
Copy link
Contributor

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.

@hughsie
Copy link
Owner

hughsie commented Nov 17, 2023

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

@Conan-Kudo
Copy link
Contributor

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.

@hughsie
Copy link
Owner

hughsie commented Nov 17, 2023

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

@Conan-Kudo
Copy link
Contributor

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.

@ximion
Copy link
Collaborator

ximion commented Nov 17, 2023

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.

@Conan-Kudo
Copy link
Contributor

Conan-Kudo commented Nov 17, 2023

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

@Pointedstick
Copy link

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.

@ximion
Copy link
Collaborator

ximion commented Nov 17, 2023

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.

@Conan-Kudo
Copy link
Contributor

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.

@ximion
Copy link
Collaborator

ximion commented Nov 17, 2023

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.

@Conan-Kudo
Copy link
Contributor

Conan-Kudo commented Nov 17, 2023

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 rpm-software-management/createrepo_c#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/

@ximion
Copy link
Collaborator

ximion commented Nov 17, 2023

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.

@Conan-Kudo
Copy link
Contributor

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

@ximion
Copy link
Collaborator

ximion commented Nov 19, 2023

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).

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

No branches or pull requests

6 participants