Re: [gentoo-dev] [PATCH] gnome2-utils.eclass add support for gdk-pixbuf cache update

[Gilles Dartiguelongue]

Le samedi 31 août 2013 à 13:40 +0200, Michał Górny a écrit :
> Dnia 2013-08-31, o godz. 13:07:41
> Gilles Dartiguelongue <eva@g.o> napisał(a):
>
> > +# @FUNCTION: gnome2_gdk_pixbuf_savelist
> > +# @DESCRIPTION:
> > +# Find if there is any gdk-pixbuf loader to install and save the list in
> > +# GNOME2_ECLASS_GDK_PIXBUF_LOADERS variable.
> > +# This function should be called from pkg_preinst.
> > +gnome2_gdk_pixbuf_savelist() {
> > +	has ${EAPI:-0} 0 1 2 && ! use prefix && ED="${D}"
> > +	pushd "${ED}" &>/dev/null
>
> pushd "${ED}" >/dev/null || die
>
> (don't hide errors)

ok, I'll check all other usage in gnome eclasses.

> > +	export GNOME2_ECLASS_GDK_PIXBUF_LOADERS=$(find "usr/$(get_libdir)/gdk-pixbuf-2.0" -type f 2>/dev/null)
> > +	popd &>/dev/null
> > +}
> > +
> > +# @FUNCTION: gnome2_gdk_pixbuf_update
> > +# @USAGE: gnome2_gdk_pixbuf_update
> > +# @DESCRIPTION:
> > +# Updates gdk-pixbuf loader cache if GNOME2_ECLASS_GDK_PIXBUF_LOADERS has some.
> > +# This function should be called from pkg_postinst and pkg_postrm.
> > +gnome2_gdk_pixbuf_update() {
> > +	has ${EAPI:-0} 0 1 2 && ! use prefix && EROOT="${ROOT}"
> > +	local updater="${EROOT}${GDK_PIXBUF_UPDATE_BIN}"
> > +
> > +	if [[ ! -x ${updater} ]]; then
> > +		debug-print "${updater} is not executable"
> > +		return
> > +	fi
> > +
> > +	if [[ -z ${GNOME2_ECLASS_GDK_PIXBUF_LOADERS} ]]; then
> > +		debug-print "gdk-pixbuf loader cache does not need an update"
> > +		return
> > +	fi
> > +
> > +	ebegin "Updating gdk-pixbuf loader cache"
> > +	local tmp_file=$(mktemp -t tmp.XXXXXXXXXX_gdkpixbuf)
> > +	${updater} 1> "${tmp_file}" 2>/dev/null &&
>
> Why do you hide errors from user? '[FAIL]' with no explanation doesn't
> seem really helpeful.

True.

> > +	cat "${tmp_file}" > "${EROOT}usr/$(get_libdir)/gdk-pixbuf-2.0/2.10.0/loaders.cache"
>
> Why not mv or cp? Also you need '|| die' here since 'cat' can fail
> writing.

I'd have to look back at the original bug report to get the exact reason
but it seems mv/cp was not atomic enough.

> Is it safe to assume constant '2.10.0'?

afaik yes, it's not changed in ages.

> > +	eend $?
> > +}
> > +
> > +
> >  # @FUNCTION: gnome2_query_immodules_gtk2
> >  # @USAGE: gnome2_query_immodules_gtk2
> >  # @DESCRIPTION:
>
> Also, please make 'loaders.cache' owned by x11-libs/gdk-pixbuf.

Yes, we need to work on that for all other caches as well.

> And please ensure to remove it in pkg_postrm() when last version
> of gdk-pixbuf is unmerged.

I am not clear on this last sentence. Could you reformulate it please ?

-- 
Gilles Dartiguelongue <eva@g.o>
Gentoo