After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 167941 - the icon-cache breaks the icons installed in subdirs
the icon-cache breaks the icons installed in subdirs
Status: RESOLVED NOTABUG
Product: gtk+
Classification: Platform
Component: .General
2.6.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2005-02-20 13:26 UTC by Sebastien Bacher
Modified: 2006-04-03 00:27 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Sebastien Bacher 2005-02-20 13:26:04 UTC
One example of bug due to this: http://bugs.debian.org/296029

gtk_icon_theme_lookup_icon () seems to not return an icon when a subdir is
updated after the cache generation.
 
"Let's see what happens here:
* Since Gtk 2.6.0, the IconCache exists
* New theme packages call gtk-update-icon-cache in their postinst
* A new package is installed and drops icons in /usr/share/icons/$foo
* It can't find its icons

The reason for that is the flawed logic of the IconCache code... To
prevent the use of old caches that don't reflect what's actually on the
disk, the code checks some timestamps:

  cache_filename = g_build_filename (path, "icon-theme.cache", NULL);

[...]

  if (g_stat (path, &path_st) < 0)
    goto done;

  /* Open the file and map it into memory */
  fd = g_open (cache_filename, O_RDONLY|_O_BINARY, 0);

[...]
  
  if (fstat (fd, &st) < 0 || st.st_size < 4)
    goto done;

  /* Verify cache is uptodate */
  if (st.st_mtime < path_st.st_mtime)
    {
      GTK_NOTE (ICONTHEME, 
		g_print ("cache outdated\n"));
      goto done; 
    }

path is something like "/usr/share/icons/hicolor/". This means that the
code checks if the mtime of $PATH/icon-theme.cache is newer than
$PATH. The mtime for a directory changes whenever something in this
directory is changed. The problem is that mtime stays the same if
something in a *subdir* is changed, which is the case here (as all icons
go to $PATH/$SIZE/$SECTION/).

Look at this little snippet to verify it:
 mkdir -p foo/bar; sleep 5; touch foo/bar/baz; 
 perl -e 'for (qw(foo foo/bar foo/bar/baz)) { printf "\%-12s \%d\n", $_,
(stat($_))[9]}'

The fix would be to check that the icon-theme.cache file's mstamp is
higher than the mstamps of all subdirs of $PATH."
Comment 1 Owen Taylor 2005-02-20 16:38:54 UTC
The icon theme spec requires the toplevel directory mtime to
be updated when installing something into a subdir.
Comment 2 Owen Taylor 2005-02-20 16:39:14 UTC
*** Bug 167942 has been marked as a duplicate of this bug. ***
Comment 3 Sebastien Bacher 2005-02-20 21:43:18 UTC
that's not the current situation. So you basically force every single
application installing an icon to be updated or the cache feature will be broken
? It should fallback to the standard icon if the cache has no entry for it ...
Comment 4 Owen Taylor 2005-02-20 21:53:20 UTC
We could, if the search *entirely* fails, retry again without
the icon cache (since paying the 100-200ms and 500k penalty
to load the icon cache is prohibitive, that probably means 
doing a ton of stats with all the directories and extensions
that the icon could have)

But what you can't do that way is catch when someone has misinstalled
an icon in a theme but the icon exists in the default theme.

Programs installing icons into an icon theme need to abide by
the rules of the icon theme spec.
Comment 5 Sebastien Bacher 2005-02-21 13:16:45 UTC
if an applications installs an icon fooapp-icon.png in
/usr/share/icons/hicolor/48x48/apps without updating the
/usr/share/icons/hicolor mtime the cache just masks the icon and the app get
broken ... blaming the apps is easy, but that's going to create a lot of
annoyance for users.

Comment 6 Loïc Minier 2005-10-02 10:41:23 UTC
Why not scan a different directory for cache aware applications and icons?

What about /usr/share/icons/cacheable or something similar where applications
which know about gtkiconcache would install their icons, and where the mtimes
rules would apply?
Comment 7 Matthias Clasen 2005-10-02 23:47:04 UTC
Just follow the icon theme spec. Come on, its not hard.
Comment 8 Josselin Mouette 2005-10-03 10:33:12 UTC
The icon theme specification is completely stupid. Following it leads to
guaranteed breakage.
Comment 9 Loïc Minier 2005-10-03 11:22:06 UTC
Matthias, we can't make every application on earth follow the spec
simultaneously.  The semantics of icons directories changed when suddenly gtk
started paying attention to the mtimes.  The way software is installed in RPMs,
and DEBs doesn't permit fixing mtimes so that Gtk detects changes in all cases.


What I personally think of the spec doesn't matter much, but it should at least
permit a smooth transition from a non-cache-aware world to a cache-aware world.
 It doesn't necessarily have to be at runtime, and hence it doesn't require to
write new code for Gtk.

It's a choice between:
1/ rewriting part of the cache code (for example to check for all directories,
or to stat in case of a cache miss)
2/ having to update every application which installs icons in a cache-unaware
way so that they install them in a cache-aware way, prior to turning caching on
3/ updating applications that want to benefit of the caching to put icons in a
different directory

The third option looks the most ugly, but the easiest to me.

The second option is the one we're facing right now, and can't deal with easily.

The first option would require more work, and there's no guarantee we do not
introduce new issues / bugs, or discover a new flaw in the specification.

Cheers,
Comment 10 Josselin Mouette 2005-10-03 12:06:50 UTC
Loïc>>
Choice #2 (which the freedesktop people recommend) is simply impossible. It's
impossible because it would require updating tar, rpm and dpkg so that they act
this way. The authors of these pieces of software will never accept that, and
they have good reasons to do so.

Choice #3 is simply ugly.

I think the only way to go is to change the implementation to do this caching in
a sane way. When there's a working implementation, we can submit changes to the
specification.
Comment 11 Matthias Clasen 2005-10-03 14:23:07 UTC
All this talk about cache-aware vs cache-unaware is rubbish. It is a simple
matter of following the spec you are using. Any further discussion of the merits
or flaws of the icon theme spec should go onto xdg-list@freedesktop.org. This
bug is closed.
Comment 12 Josselin Mouette 2005-10-03 14:33:27 UTC
It is *not* possible to follow this spec. Full stop. We can start a discussion
to modify the spec, but in the meantime something has to be done. Otherwise
we're just stuck with a broken system.
Comment 13 Matthias Clasen 2005-10-03 14:47:34 UTC
With that position, its not even worth starting a discussion about the spec,
Josselin. Just do something else.
Comment 14 Loïc Minier 2005-10-03 15:21:48 UTC
(Matthias, I'm sorry I hijack this bug again, I just want to clarify 2 things
that have been expressed _here_; I understand you want the real discussion to
happen on the list.)

Joss: what I meant with "2/" was to have some post installation task for all
applications installing icons, such as a debhelper snipset, prepare all
applications to use it, and then enable cache.  Note that such things would be
prettier with some tools at the dpkg level to express things such as "if some
files are installed below this directory, run this", but this is completely OT.

Matthias, I don't want to talk giberrish to you, let's call that "spec-aware"
and "spec-unware" packages / applications instead. :)

Really, think about it, how could apps written prior to the spec conform to it?
  Distributions ship a lot of these.  If they don't conform, you have to think
of some progressive way of making them comply, ie. a transition.  You wake up
one day, and all these packages are suddenly borken.

(/end of comments, I go subscribing to xdg-list.)
Comment 15 Josselin Mouette 2005-10-03 15:23:00 UTC
Sorry for only wanting things to work correctly, but following a broken
specification isn't a way to go. Fixing the spec and fixing the implementation
should be done in parallel.