Suggested ScriptletSnippets (icon cache) changes

[Ville Skyttä]

Hello,

I'd suggest the following changes to the "GTK+ icon cache" entry in 
Packaging:ScriptletSnippets:

1) Rename the entry to "Icon cache".  It's not all about GTK+ but KDE as
well.

2) Append " &>/dev/null" to the "touch" line in %postun.  If
"touch" is not 
available when the package is removed, I find it more than likely that GTK+ 
or KDE are not there to benefit from icon cache updates any longer 
either.  /usr/share may also be mounted read only with %_netsharedpath.

3) Append " &>/dev/null" to the "touch" line in %post.  If
"touch" is not yet 
available, neither should be kdelibs, and thus there is no reason to touch 
the dir.  kdelibs itself does a forced update in its %post.  /usr/share may 
also be mounted read only with %_netsharedpath.  (This avoids the need to add 
unnecessary "Requires(post): coreutils" to every package that installs icons 
just to quiet down a pretty rare and harmless warning.)

4) Remove the -x test and --quiet option to gtk-update-icon-cache, redirect 
all output (including stderr) to /dev/null.  gtk-update-icon-cache may not be 
installed, /usr/share may be mounted read only with %_netsharedpath, etc... 
and the --quiet tells me that there was no interest in seeing g-u-i-c's 
stdout anyway.

5) Wrap %post in "if [ $1 -eq 1 ] ; then ..." to take care of the initial 
installation and let %postun handle package upgrade cases in order to 
eliminate one touch and one g-u-i-c invocation per package upgrade.  I 
suppose this would be "safe enough" to do, it'd just result in possibly
stale 
icon caches for the duration of the upgrade transaction.  (Icons change 
relatively rarely, and even if they do, does this smallish stale cache window 
really matter at all?)

So the scriptlets would become:

%post
if [ $1 -eq 1 ] ; then
    touch --no-create %{_datadir}/icons/hicolor &>/dev/null
    gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
fi

%postun
touch --no-create %{_datadir}/icons/hicolor &>/dev/null
gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :

[Enrico Scholz]

Ville Skyttä <ville.skytta(a)iki.fi&gt; writes:

>  So the scriptlets would become:
>
>  %post
>  if [ $1 -eq 1 ] ; then
>      touch --no-create %{_datadir}/icons/hicolor &>/dev/null
>      gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
>  fi
>
>  %postun
>  touch --no-create %{_datadir}/icons/hicolor &>/dev/null
>  gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || : 
Why not

| %post
| touch --no-create %{_datadir}/icons/hicolor &>/dev/null
| 
| %postun
| touch --no-create %{_datadir}/icons/hicolor &>/dev/null
| 
| %posttrans
| gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :

?  All relevant Fedora releases should understand %posttrans and this
speeds up things significantly.



Enrico

[Bill Nottingham]

Enrico Scholz (enrico.scholz(a)informatik.tu-chemnitz.de) said: 
>  | %postun
>  | touch --no-create %{_datadir}/icons/hicolor &>/dev/null
>  | 
>  | %posttrans
>  | gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
>  
>  ?  All relevant Fedora releases should understand %posttrans and this
>  speeds up things significantly. 
As posttrans isn't collated into a single run, it just moves the total
of calls from point A to point B, and therefore would theoretically
only speed things up via cache effects.

Bill

[Rex Dieter]

Bill Nottingham wrote:
>  Enrico Scholz (enrico.scholz(a)informatik.tu-chemnitz.de) said: 
> > | %postun
> > | touch --no-create %{_datadir}/icons/hicolor &>/dev/null
> > | 
> > | %posttrans
> > | gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
> >
> > ?  All relevant Fedora releases should understand %posttrans and this
> > speeds up things significantly.
>  
>  As posttrans isn't collated into a single run, it just moves the total
>  of calls from point A to point B, and therefore would theoretically
>  only speed things up via cache effects. 
posttrans could be a win if the idea from
https://bugzilla.redhat.com/show_bug.cgi?id=170335#c33
ever came to fruition.

-- Rex

[Rex Dieter]

Rex Dieter wrote:
>  Bill Nottingham wrote:
> > Enrico Scholz (enrico.scholz(a)informatik.tu-chemnitz.de) said: 
> >> ?  All relevant Fedora releases should understand %posttrans
> and this
> >> speeds up things significantly.
> >
> > As posttrans isn't collated into a single run, it just moves the total
> > of calls from point A to point B, and therefore would theoretically
> > only speed things up via cache effects.
>  
>  posttrans could be a win if the idea from
>  https://bugzilla.redhat.com/show_bug.cgi?id=170335#c33
>  ever came to fruition. 
nevermind, recalled that posttrans was previously considered and 
rejected, since posttrans isn't called on erasure (only install/upgrade).

-- Rex

[Enrico Scholz]

Rex Dieter <rdieter(a)math.unl.edu&gt; writes:

>  nevermind, recalled that posttrans was previously considered and
> rejected,
>  since posttrans isn't called on erasure (only install/upgrade). 
Then use a modified %postun scriptlet like

%postun
touch --no-create %{_datadir}/icons/hicolor &>/dev/null
test $1 -ge 1 || gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null


--> install and update are cheap, but erase is expensive (but correct).


Enrico

[Rex Dieter]

Enrico Scholz wrote:
>  Rex Dieter <rdieter(a)math.unl.edu&gt; writes: 
> > nevermind, recalled that posttrans was previously considered and
> rejected,
> > since posttrans isn't called on erasure (only install/upgrade). 
>  Then use a modified %postun scriptlet like
>  %postun
>  touch --no-create %{_datadir}/icons/hicolor &>/dev/null
>  test $1 -ge 1 || gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null
>  --> install and update are cheap, but erase is expensive (but
> correct). 
me likie.

-- Rex

Suggested ScriptletSnippets (=?iso-8859-1?q?icon=09cache?=) changes

[Ville Skyttä]

On Wednesday 28 January 2009, Rex Dieter wrote:
>  Enrico Scholz wrote:
>  > Rex Dieter <rdieter(a)math.unl.edu&gt; writes:
>  >> nevermind, recalled that posttrans was previously considered and
>  >> rejected, since posttrans isn't called on erasure (only
>  >> install/upgrade).
>  >
>  > Then use a modified %postun scriptlet like
>  > %postun
>  > touch --no-create %{_datadir}/icons/hicolor &>/dev/null
>  > test $1 -ge 1 || gtk-update-icon-cache %{_datadir}/icons/hicolor
>  > &>/dev/null
>  >
>  > --> install and update are cheap, but erase is expensive (but correct).
>
>  me likie. 
Just remember to add something to prevent a non-zero gtk-update-icon-cache 
exit status from causing trouble.

Suggested ScriptletSnippets (=?iso-8859-1?q?icon=09cache?=) changes

[Ville Skyttä]

Hello,

So I gather as the result of this discussion would be:

----
%post
touch --no-create %{_datadir}/icons/hicolor &>/dev/null || :

%postun
if [ $1 -eq 0 ] ; then
    touch --no-create %{_datadir}/icons/hicolor &>/dev/null
    gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
fi

%posttrans
gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
----

Initial install and upgrades are handled by %post and %posttrans, final erase 
by %postun.  Anything more to tweak?  If not, is this discussion (see also 1) 
in my initial mail [0]) and summary enough for the FPC so you can look into 
it in a near future meeting?

[0] 
https://www.redhat.com/archives/fedora-packaging/2009-January/msg00138.html

[Toshio Kuratomi]

Ville Skyttä wrote:
>  Hello,
>  
>  So I gather as the result of this discussion would be:
>  
>  ----
>  %post
>  touch --no-create %{_datadir}/icons/hicolor &>/dev/null || :
>  
>  %postun
>  if [ $1 -eq 0 ] ; then
>      touch --no-create %{_datadir}/icons/hicolor &>/dev/null
>      gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
>  fi
>  
>  %posttrans
>  gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
>  ----
>  
>  Initial install and upgrades are handled by %post and %posttrans, final erase 
>  by %postun.  Anything more to tweak?  If not, is this discussion (see also 1) 
>  in my initial mail [0]) and summary enough for the FPC so you can look into 
>  it in a near future meeting?
>   
https://fedoraproject.org/wiki/PackagingDrafts/Icon_Cache

Does that look good?

Couple questions, in the %postun, there's no || : for the touch.  Is
that intentional?  (If not, please change it for me :-)

If %posttrans should prove controversial (I don't see a problem but if
it is) is including the gtk-update-icon-cache call in %post in its
modified state acceptable to the proposal as a whole?

-Toshio

[Ville Skyttä]

On Saturday 14 February 2009, Toshio Kuratomi wrote:

>  https://fedoraproject.org/wiki/PackagingDrafts/Icon_Cache
>
>  Does that look good? 
Yep, pretty much.  I found some things that could be improved, and reworded 
the first paragraph.

>  Couple questions, in the %postun, there's no || : for the touch. 
> Is
>  that intentional? 
Yes.  Only the final exit statuses of the scriptlets matter here.

>  If %posttrans should prove controversial (I don't see a problem
> but if
>  it is) is including the gtk-update-icon-cache call in %post in its
>  modified state acceptable to the proposal as a whole? 
I'm not sure I understand you correctly.  Do you mean that if %posttrans for 
some reason is not accepted, just move the g-u-i-c call from %posttrans 
to %post?  I suppose that would work, but it would result in icons that are 
removed on package upgrades left behind in the GTK icon cache.  Dunno if 
that's a problem at all, but my initial proposal did not have that issue: 
https://www.redhat.com/archives/fedora-packaging/2009-January/msg00138.html

[Enrico Scholz]

Ville Skyttä <ville.skytta(a)iki.fi&gt; writes:

>  %postun
>  if [ $1 -eq 0 ] ; then
>      touch --no-create %{_datadir}/icons/hicolor &>/dev/null
>      gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
>  fi 
The 'touch' should be inconditional and outside of the 'if' block.
E.g. when we have a transaction like


1. UPDATE new1
2. UPDATE old
3. UPDATE new0
4. ERASE new0
5. ERASE old
6. ERASE new1
7. %posttrans


where newX have the scriptlet above and 'old' has a legacy scriptlet
which executes 'gtk-update-icon-cache' everytime.  Then, 'old' will
update the cache in step 5 and %posttrans will be a noop.  Changes in
step 6 will be ignored and system has the cache from step 5.



Enrico

Suggested ScriptletSnippets (=?iso-8859-1?q?icon=09cache?=) changes

[Ville Skyttä]

On Sunday 22 February 2009, Enrico Scholz wrote:
>  Ville Skyttä <ville.skytta(a)iki.fi&gt; writes:
>  > %postun
>  > if [ $1 -eq 0 ] ; then
>  >     touch --no-create %{_datadir}/icons/hicolor &>/dev/null
>  >     gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
>  > fi
>
>  The 'touch' should be inconditional and outside of the 'if' block.
>  E.g. when we have a transaction like
>
>
>  1. UPDATE new1
>  2. UPDATE old
>  3. UPDATE new0
>  4. ERASE new0
>  5. ERASE old
>  6. ERASE new1
>  7. %posttrans
>
>
>  where newX have the scriptlet above and 'old' has a legacy scriptlet
>  which executes 'gtk-update-icon-cache' everytime.  Then, 'old' will
>  update the cache in step 5 and %posttrans will be a noop.  Changes in
>  step 6 will be ignored and system has the cache from step 5. 
What practical problems would that cause?  The resulting icon cache could 
contain entries for icons that were removed in step 6, but that'd only affect 
scenarios where the old version of new1 in the above had icons that the new 
version of new1 didn't have which I bet is a very rare case and it isn't 
obvious to me where this would be an actual problem because references to 
those icons have almost certainly gone away updated desktop files shipped 
with the new version of new1, or will go away along with the old version of 
new1 if it contained desktop files that the new new1 doesn't have.

Updating the timestamp of the toplevel icon dir more often than necessary 
would probably cause environments that auto-update their caches or the like 
based on the timestamp ending up doing it more often than necessary.

Unless I missed something crucial, IMO this (theoretical?) problem should be 
just ignored, packagers should start updating their cache update scriptlets 
to the ones in the Wiki draft at their earliest convenience and it isn't 
necessary to carry around legacy cruft.

[Enrico Scholz]

Ville Skyttä <ville.skytta(a)iki.fi&gt; writes:

>  What practical problems would that cause?  The resulting icon cache
> could 
>  contain entries for icons that were removed in step 6 
When we do not care about this, why do we have the %postun scriptlet at
all?


Enrico

Suggested ScriptletSnippets (=?iso-8859-1?q?icon=09cache?=) changes

[Ville Skyttä]

On Sunday 22 February 2009, Enrico Scholz wrote:
>  Ville Skyttä <ville.skytta(a)iki.fi&gt; writes:
>  > What practical problems would that cause?  The resulting icon cache could
>  > contain entries for icons that were removed in step 6
>
>  When we do not care about this, why do we have the %postun scriptlet at
>  all? 
Apples and oranges.  Not caring about a case occurring very rarely on package 
updates in legacy related scenarios which is going away as packages are being 
updated is not the same thing that forgetting about it altogether for all 
updates and erase-only transactions.

Granted, I don't know if dropping the %postun script altogether would cause 
any problems in practice.

[Enrico Scholz]

Ville Skyttä <ville.skytta(a)iki.fi&gt; writes:

> > > What practical problems would that cause?  The resulting
> icon cache
> > > could contain entries for icons that were removed in step 6
> >
> > When we do not care about this, why do we have the %postun scriptlet at
> > all?
>
>  Apples and oranges.  Not caring about a case occurring very rarely 
Package removals are occuring very rarely too...  I really do not
understand why you want to handle only 9x% although it would be
trivial to make things race free and to catch the whole 100%.


Enrico

Suggested ScriptletSnippets (=?iso-8859-1?q?icon=09cache?=) changes

[Ville Skyttä]

On Sunday 22 February 2009, Enrico Scholz wrote:

>  I really do not
>  understand why you want to handle only 9x% although it would be
>  trivial to make things race free and to catch the whole 100%. 
The keyword is feasibility, and reasons were already included in my previous 
reply.  I believe you'll find it less than trivial to come up with an 
implementation that is _really_ race free and _really_ catches 100%, that's 
where feasibility thoughts kick in.

Anyway, it doesn't make much sense to continue this discussion since you 
haven't answered the first question (about real world problem you're trying 
to solve) and instead seem to be replying to/finding things to 
nitpick/disagree on even in messages whose bottom line pretty much agree with 
what you suggested (about %postun possibly being droppable).

[Panu Matilainen]

On Wed, 28 Jan 2009, Rex Dieter wrote:

>  Rex Dieter wrote:
> > Bill Nottingham wrote:
> >> Enrico Scholz (enrico.scholz(a)informatik.tu-chemnitz.de) said:
>
> >>> ?  All relevant Fedora releases should understand %posttrans and this
> >>> speeds up things significantly.
> >> 
> >> As posttrans isn't collated into a single run, it just moves the total
> >> of calls from point A to point B, and therefore would theoretically
> >> only speed things up via cache effects.
> > 
> > posttrans could be a win if the idea from
> > https://bugzilla.redhat.com/show_bug.cgi?id=170335#c33
> > ever came to fruition.
>
>  nevermind, recalled that posttrans was previously considered and rejected, 
>  since posttrans isn't called on erasure (only install/upgrade). 
Yup, %posttrans for erase is kinda hard when the package header(s) that 
the regular rpm scriptlet machinery needs are long gone by the time 
%posttrans runs.

I've been playing around a bit with the "file triggers" concept recently, 
a very crude but lightweight version is actually possible with a 
three-liner patch to rpm 4.6 (heck, even 4.4.2.x) + bit of lua scripting. 
This is nowhere near real-world usable (see below) but in case somebody 
wants to tinker and play around, a quick and dirty example is here: 
http://laiskiainen.org/tmp/rpmhook-example/

This tries to get by with just watching bunch of selected directories and 
launching associated hooks if the directory times change, which is fairly 
close to being enough, at least for many things:

[root@turre rpm-4.6.x]# ./rpm -Uvh --noscripts 
/tmp/gthumb-2.10.10-3.fc10.x86_64.rpm 
/tmp/seahorse-2.24.1-1.fc10.x86_64.rpm
Preparing...                ########################################### 
[100%]
    1:seahorse               ########################################### [ 
50%]
    2:gthumb                 ########################################### 
[100%]
ldconfig hook caught:
   -> /usr/lib64
desktop_db hook caught:
   -> /usr/share/applications
scrollkeeper hook caught:
   -> /usr/share/omf

Icon cache gets missed here as the files go into subdirectories of 
/usr/share/icons/hicolor/, it'd need watching on all the directories 
recursively. This approach will give some false positives too, like gthumb 
putting files into /usr/lib64/gthumb/ triggering ldconfig needlessly, otoh 
in many cases the false positives are harmless as it only runs once.

The nice point about using directory timestamps would be that it avoids 
potentially *very* expensive filename/regex matching on the entire 
transaction set filelist + needing to store the matches somewhere, but 
whether that's sufficient for real-world use I dunno... Also this doesn't 
handle chroot at all, and the hooks should come from the relevant 
packages, not from some central rpm scripts which in turn means the hooks 
should be in headers etc.

 	- Panu -

[Matthias Clasen]

On Thu, 2009-01-29 at 00:59 +0200, Panu Matilainen wrote:

>  
>  Icon cache gets missed here as the files go into subdirectories of 
>  /usr/share/icons/hicolor/, it'd need watching on all the directories 
>  recursively.  

As per the icon theme spec, anybody who installs an icon in a icon theme
is required to touch the toplevel directory (for precisely this
reason).

[Enrico Scholz]

Bill Nottingham <notting(a)redhat.com&gt; writes:

> > | %postun
> > | touch --no-create %{_datadir}/icons/hicolor &>/dev/null
> > | 
> > | %posttrans
> > | gtk-update-icon-cache %{_datadir}/icons/hicolor &>/dev/null || :
>
>  As posttrans isn't collated into a single run, it just moves the total
>  of calls from point A to point B, 
no; gtk-update-icon-cache is cheap when icons/hicolor is up-to-date.
Hence, only the first %posttrans will update the icon cache.  Subsequent
ones will be noops effectively.

With old method, there is a 'touch icons/hicolor' which invalidates
cache and makes every operation expensive.



Enrico

[Bruno Wolff III]

On Wed, Jan 28, 2009 at 01:36:36 +0200,
  Ville Skyttä <ville.skytta(a)iki.fi&gt; wrote:
>  
>  2) Append " &>/dev/null" to the "touch" line in %postun.  If
> "touch" is not 
>  available when the package is removed, I find it more than likely that GTK+ 
>  or KDE are not there to benefit from icon cache updates any longer 
>  either.  /usr/share may also be mounted read only with %_netsharedpath. 
The right way to do things is to make sure touch is available by
having an approiate script dependency on the package that provides touch.
Something like:
Requires(pre): coreutils

[Ville Skyttä]

On Wednesday 28 January 2009, Bruno Wolff III wrote:
>  On Wed, Jan 28, 2009 at 01:36:36 +0200,
>
>    Ville Skyttä <ville.skytta(a)iki.fi&gt; wrote:
>  > 2) Append " &>/dev/null" to the "touch" line in %postun. 
> If "touch" is
>  > not available when the package is removed, I find it more than likely
>  > that GTK+ or KDE are not there to benefit from icon cache updates any
>  > longer either.  /usr/share may also be mounted read only with
>  > %_netsharedpath.
>
>  The right way to do things is to make sure touch is available by
>  having an approiate script dependency on the package that provides touch.
>  Something like:
>  Requires(pre): coreutils 
The point of the suggestion I posted is to avoid *unnecessary* dependencies 
like this one as well as unnecessary calls to touch and 
gtk-update-icon-cache.  The part of my original message you quoted above 
explains why I think it doesn't matter if touch is available at %postun time 
or not (my assumption is that coreutils can't be removed while gtk2 or 
kdelibs is installed).  If I'm correct and it doesn't 
matter, "Requires(postun): coreutils" is just unnecessary bloat in repodata, 
rpmdb and spec files.