ximion / appstream-dep11

Python module to handle Debian DEP-11 metadata
https://github.com/ximion/appstream-generator
GNU Lesser General Public License v3.0
3 stars 3 forks source link

No icons found for Elementary apps #6

Closed tintou closed 8 years ago

tintou commented 8 years ago

In elementary, we use a lot of apps with the icon from the theme (following the fd.o icons specification) and most of our applications are ignored because of it. for example: Hints:

ximion commented 8 years ago

Can you please tell me:

We won't include apps without icon, but the problem here is that an icon is not found.

ximion commented 8 years ago

My current guess is that the icon is in some kind of stock-icon package, in that case I would suggest using an XDG default name (likely not possible for the given example), or supply a default icon with the app itself. Depending on the quantity of affected apps, we could also talk about adding the elementary icons to the preseeded set of stock icons, but I would kind of like to have some strict criteria for adding those packages, simply because adding more of them has a price.

tintou commented 8 years ago

The icons are in our stock icon set (elementary-icons) Most of our apps are using the stock icon names (yeah the example I gave you wasn't using the right icon name, it's fixed now) but they aren't imported because the icon is missing

danirabbit commented 8 years ago

Hey ximion, I think the ones we should be able to make a good case for are those covered by the FD.o naming spec. All of these should be the bare minimum covered by the icon sets provided by GNOME, KDE, Pantheon (elementary's DE), etc.

Other generic names that just elementary ships are:

ximion commented 8 years ago

Ok, hmm, this is a tricky case. I am reluctant to add new icon packages to the preseed step because of these reasons:

So, if Elementary apps would ship their default icons in /usr/share/icons/hicolor, and have those overridden by Elementary's default theme, this problem wouldn't exist. Icons in hicolor are always preferred (which is a nice solution in case your app accidentally got a KDE or GNOME icon). So, a solution would be placing default icons, or I could preseed the Elementary theme as well, but if I do that, I can't easily reject doing that for other desktop environments as well, so we might have lots of themes in the pool, in the end, and apps still would get the wrong icon.

danirabbit commented 8 years ago

Hm right, as you said there would be lots of problems by trying to pull the icon from an arbitrary icon set, so I don't think doing that sounds like a solution.

I don't think shipping fd.o named icons into hicolor is a good idea. This would imply that (for example) packages like gnome-terminal and pantheon-terminal supply the exact same file to the exact same location on the system, causing them to conflict in packaging.

I guess my suggestion would be that these apps shouldn't be skipped and it's okay to include these apps without an icon because those icon names should almost certainly exist on the system.

ximion commented 8 years ago

I don't think shipping fd.o named icons into hicolor is a good idea.

Jup, that's a terribly bad idea. We already preseed the GNOME and KDE iconthemes (Adwaita and Oxygen (soon Breeze)), so all the XDG icon theme names are covered. They would get the "wrong" icon from an Elementary perspective though, which is an issue we can't solve at time (maybe in future by extending the .desktop file spec).

All the non-standard icons though could easily exist in hicolor (if the name isn't too generic).

I guess my suggestion would be that these apps shouldn't be skipped and it's okay to include these apps without an icon because those icon names should almost certainly exist on the system.

How do we know if the icon certainly exists? We actually have no idea about that when generating the metadata, and in case we can't find an icon, we must assume it is missing. And we don't publish a GUI application in the metadata which doesn't have an Icon field (forbidden by the spec anyway). If you find an app referencing an icon from the XDG icon naming spec, and that icon isn't found, please relay it to me since that's a bug. Any non-standard icons should be available in hicolor, or a preseeded icon theme.

danirabbit commented 8 years ago

Hey ximinion, as I noted earlier, the icons we're most concerned with are ones that are covered by fd.o spec. It looks like Adwaita does not cover the full spec, while the old GNOME theme does :)

That's what this report is meant to note. We have apps with icons that are covered by the fd.o spec that are being skipped. The icon name should exist since fd.o prescribes the above list as the minimum for an icon set.

ximion commented 8 years ago

Can you give me some examples? Ideally links to the issues pages on appstream.debian.org or another instance producing this information. This is clearly a bug which needs to be fixed, by adding another icon theme or by making the generator a bit smarter (obviously, if I really have to add another icon theme, I want it to be in active maintenance, so the old GNOME theme will likely not be the first choice).

ximion commented 8 years ago

I might have found two potential issues which prevent apps from getting the right stock icon. I'll work on addressing both today.

ximion commented 8 years ago

The recent commits, and especially https://github.com/ximion/appstream-dep11/commit/b231ceb085e308cb78f9471ed2d470f5a1079194 should have resolved this problem. Can you please get a Git snapshot and re-run the metadata generator? (you don't need to drop the whole database, just running dep11-generator remove-processed is enough to clear existing processed data and hints end reprocess with the next run (ignored packages with no metadata stay ignored, so using this is much faster than a full rebuild).

tintou commented 8 years ago

This doesn't work for our repository as it doesn't ship the Adwaita or Oxygene theme So I end-up with an error from python

Traceback (most recent call last):
  File "/usr/lib/python3.5/multiprocessing/pool.py", line 119, in worker
    result = (True, func(*args, **kwds))
  File "/usr/lib/python3/dist-packages/dep11/generator.py", line 50, in extract_metadata
    cpts = mde.process(pkgname, version, arch, package_fname)
  File "/usr/lib/python3/dist-packages/dep11/extractor.py", line 650, in process
    cpts = self._process_pkg(pkgname, pkgversion, pkgarch, pkg_fname, metainfo_files)
  File "/usr/lib/python3/dist-packages/dep11/extractor.py", line 636, in _process_pkg
    self._fetch_icon(cpt, export_path, pkg_fname, filelist)
  File "/usr/lib/python3/dist-packages/dep11/extractor.py", line 460, in _fetch_icon
    icon_dict = self._icon_finder.find_icons(cpt.pkgname, icon_str, all_icon_sizes)
  File "/usr/lib/python3/dist-packages/dep11/iconfinder.py", line 198, in find_icons
    flist = self._search_icon(str(size), icon)
  File "/usr/lib/python3/dist-packages/dep11/iconfinder.py", line 185, in _search_icon
    icon = self._search_icon_in_theme(size_str, icon_name, theme)
  File "/usr/lib/python3/dist-packages/dep11/iconfinder.py", line 134, in _search_icon_in_theme
    files_list = self._icon_themes_data[theme_name]
KeyError: 'Adwaita'
ximion commented 8 years ago

Ah, so the key problem here is that you need to add your own icon-theme to the mix. I wonder if there is any way we can make it generally easier for 3rd-party repos to have applications reference stock icons... Right now I don't see a solution (at least none which doesn't involve the config file referencing a location where those icons are provided statically).

For your specific usecase though, a config option which lets you set your icon-theme-name should work. I will add it soon. (Your repo does ship a Contents.gz file, right?)

tintou commented 8 years ago

Yes I don't have a Contents-*.gz file in the repo but I generate it in my local copy

ximion commented 8 years ago

Okay, that is an essential prerequisite for the global iconsearch - otherwise we have no idea where to get icons from ;-)

ximion commented 8 years ago

Add useIconTheme: Whatever as child of a suite name in the config (at the same place where your dataPriority setting is). That should make the generator include the theme in the loop. Thank you for reporting this issue, was really helpful to fix these corner cases! (in case the bug isn't fixed yet, feel free to reopen)