On Sun, 2019-03-24 at 18:17 +0100, Andreas Sturmlechner wrote:
> ---
>  eclass/font.eclass | 57 +++++++++++++++++++++++-----------------------
>  1 file changed, 28 insertions(+), 29 deletions(-)

-U9999, please.  This is a huge eclass and probably requires more work
than you're showing us ;-).

> 
> diff --git a/eclass/font.eclass b/eclass/font.eclass
> index 58ec9e3..622f143 100644
> --- a/eclass/font.eclass
> +++ b/eclass/font.eclass
> @@ -1,4 +1,4 @@
> -# Copyright 1999-2015 Gentoo Foundation
> +# Copyright 1999-2019 Gentoo Foundation
>  # Distributed under the terms of the GNU General Public License v2
>  
>  # @ECLASS: font.eclass
> @@ -7,12 +7,11 @@
>  # @BLURB: Eclass to make font installation uniform
>  
>  case ${EAPI:-0} in
> -     0|1|2|3|4|5|6) ;;
> -     *)             die "EAPI ${EAPI} is not supported by font.eclass." ;;
> +     0|1|2|3|4|5|6) inherit eutils ;;
> +     7) ;;
> +     *) die "EAPI ${EAPI} is not supported by font.eclass." ;;
>  esac
>  
> -inherit eutils
> -
>  EXPORT_FUNCTIONS pkg_setup src_install pkg_postinst pkg_postrm
>  
>  # @ECLASS-VARIABLE: FONT_SUFFIX
> @@ -68,12 +67,12 @@ font_xfont_config() {
>       if has X ${IUSE//+} && use X ; then
>               dir_name="${1:-${FONT_PN}}"
>               ebegin "Creating fonts.scale & fonts.dir in ${dir_name##*/}"
> -             rm -f 
> "${ED}${FONTDIR}/${1//${S}/}"/{fonts.{dir,scale},encodings.dir}
> -             mkfontscale "${ED}${FONTDIR}/${1//${S}/}"
> +             rm -f 
> "${ED%/}/${FONTDIR}/${1//${S}/}"/{fonts.{dir,scale},encodings.dir}
> +             mkfontscale "${ED%/}/${FONTDIR}/${1//${S}/}"
>               mkfontdir \
>                       -e ${EPREFIX}/usr/share/fonts/encodings \
>                       -e ${EPREFIX}/usr/share/fonts/encodings/large \
> -                     "${ED}${FONTDIR}/${1//${S}/}"
> +                     "${ED%/}/${FONTDIR}/${1//${S}/}"
>               eend $?
>               if [[ -e fonts.alias ]] ; then
>                       doins fonts.alias

Would it make sense to add some error handling?  It seems silly to try
to do a few things, then report one $? for the last of them,
independently of whether previous actions succeeded or failed.

> @@ -103,7 +102,7 @@ font_cleanup_dirs() {
>       local d f g generated candidate otherfile
>  
>       ebegin "Cleaning up font directories"
> -     find -L "${EROOT}"usr/share/fonts/ -type d -print0 | while read -d 
> $'\0' d; do
> +     find -L "${EROOT%/}"/usr/share/fonts/ -type d -print0 | while read -d 
> $'\0' d; do

Move while to primary shell, and use < <(find ...).

>               candidate=false
>               otherfile=false
>               for f in "${d}"/*; do
> @@ -160,7 +159,7 @@ font_pkg_setup() {
>  
>       # make sure we get no collisions
>       # setup is not the nicest place, but preinst doesn't cut it
> -     [[ -e "${EROOT}/${FONTDIR}/fonts.cache-1" ]] && rm -f 
> "${EROOT}/${FONTDIR}/fonts.cache-1"
> +     [[ -e "${EROOT%/}/${FONTDIR}/fonts.cache-1" ]] && rm -f 
> "${EROOT%/}/${FONTDIR}/fonts.cache-1"

What's the point of checking the existence, if you pass '-f' to rm
anyway?

>  }
>  
>  # @FUNCTION: font_src_install
> @@ -202,36 +201,42 @@ font_src_install() {
>       done
>  }
>  
> +# @FUNCTION: _update_fontcache
> +# @DESCRIPTION:
> +# Updates fontcache if !prefix and media-libs/fontconfig installed
> +_update_fontcache() {
> +     # TODO: cleanup after <EAPI-7 is banned
> +     if has_version media-libs/fontconfig && { [[ -z ${ROOT} ]] || [[ 
> ${ROOT} == / ]] ; } then

[[ -z ${ROOT} || ${ROOT} == / ]] is valid in bash.

[[ -z ${ROOT%/} ]] is shorter.

> +             ebegin "Updating global fontcache"
> +             fc-cache -fs
> +             eend $?
> +     else
> +             einfo "Skipping fontcache update (media-libs/fontconfig is not 
> installed or ROOT != /)"
> +     fi
> +}
> +
>  # @FUNCTION: font_pkg_postinst
>  # @DESCRIPTION:
>  # The font pkg_postinst function.
>  font_pkg_postinst() {
>       # unreadable font files = fontconfig segfaults
> -     find "${EROOT}"usr/share/fonts/ -type f '!' -perm 0644 -print0 \
> +     find "${EROOT%/}"/usr/share/fonts/ -type f '!' -perm 0644 -print0 \
>               | xargs -0 chmod -v 0644 2>/dev/null

find ... -exec chmod ... {} +

>  
>       if [[ -n ${FONT_CONF[@]} ]]; then
>               local conffile
> -             echo
>               elog "The following fontconfig configuration files have been 
> installed:"
>               elog
>               for conffile in "${FONT_CONF[@]}"; do
> -                     if [[ -e ${EROOT}etc/fonts/conf.avail/$(basename 
> ${conffile}) ]]; then
> +                     if [[ -e ${EROOT%/}/etc/fonts/conf.avail/$(basename 
> ${conffile}) ]]; then

Don't call basename if you can do the same via ${conffile##*/}.

>                               elog "  $(basename ${conffile})"
>                       fi
>               done
>               elog
>               elog "Use \`eselect fontconfig\` to enable/disable them."
> -             echo
>       fi
>  
> -     if has_version media-libs/fontconfig && [[ ${ROOT} == / ]]; then
> -             ebegin "Updating global fontcache"
> -             fc-cache -fs
> -             eend $?
> -     else
> -             einfo "Skipping fontcache update (media-libs/fontconfig is not 
> installed or ROOT != /)"
> -     fi
> +     _update_fontcache
>  }
>  
>  # @FUNCTION: font_pkg_postrm
> @@ -241,14 +246,8 @@ font_pkg_postrm() {
>       font_cleanup_dirs
>  
>       # unreadable font files = fontconfig segfaults
> -     find "${EROOT}"usr/share/fonts/ -type f '!' -perm 0644 -print0 \
> +     find "${EROOT%/}"/usr/share/fonts/ -type f '!' -perm 0644 -print0 \
>               | xargs -0 chmod -v 0644 2>/dev/null

Likewise.  Also, wouldn't it make sense to put it inside
_update_fontcache()?  It seems identical here and in postinst.

>  
> -     if has_version media-libs/fontconfig && [[ ${ROOT} == / ]]; then
> -             ebegin "Updating global fontcache"
> -             fc-cache -fs
> -             eend $?
> -     else
> -             einfo "Skipping fontcache update (media-libs/fontconfig is not 
> installed or ROOT != /)"
> -     fi
> +     _update_fontcache
>  }

-- 
Best regards,
Michał Górny

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to