[gentoo-dev] [PATCH] font.eclass: Ban EAPIs < 5

[gentoo-dev] [PATCH] font.eclass: Ban EAPIs < 5

[David Seifert]

* Add inherit guard like all modern eclasses

Closes: https://bugs.gentoo.org/679658
Signed-off-by: David Seifert <soap@gentoo.org>
---
 eclass/font.eclass | 43 +++++++++++++++----------------------------
 1 file changed, 15 insertions(+), 28 deletions(-)

diff --git a/eclass/font.eclass b/eclass/font.eclass
index 76c20549ea6..cdf3494a79c 100644
--- a/eclass/font.eclass
+++ b/eclass/font.eclass
@@ -4,15 +4,17 @@
 # @ECLASS: font.eclass
 # @MAINTAINER:
 # fonts@gentoo.org
-# @SUPPORTED_EAPIS: 0 1 2 3 4 5 6 7
+# @SUPPORTED_EAPIS: 5 6 7
 # @BLURB: Eclass to make font installation uniform
 
 case ${EAPI:-0} in
-	0|1|2|3|4|5|6) inherit eutils ;;
+	[56]) inherit eutils ;;
 	7) ;;
 	*) die "EAPI ${EAPI} is not supported by font.eclass." ;;
 esac
 
+if [[ ! ${_FONT_ECLASS} ]]; then
+
 EXPORT_FUNCTIONS pkg_setup src_install pkg_postinst pkg_postrm
 
 # @ECLASS-VARIABLE: FONT_SUFFIX
@@ -76,9 +78,7 @@ font_xfont_config() {
 			-e ${EPREFIX}/usr/share/fonts/encodings \
 			-e ${EPREFIX}/usr/share/fonts/encodings/large \
 			"${ED%/}/${FONTDIR}/${1//${S}/}" || eerror "failed to create fonts.dir"
-		if [[ -e fonts.alias ]]; then
-			doins fonts.alias || die "failed to install fonts.alias" # TODO old EAPI cleanup
-		fi
+		[[ -e fonts.alias ]] && doins fonts.alias
 	fi
 }
 
@@ -90,9 +90,7 @@ font_fontconfig() {
 	if [[ -n ${FONT_CONF[@]} ]]; then
 		insinto /etc/fonts/conf.avail/
 		for conffile in "${FONT_CONF[@]}"; do
-			if [[ -e  ${conffile} ]]; then
-				doins ${conffile} || die "failed to install conf file" # TODO old EAPI cleanup
-			fi
+			[[ -e ${conffile} ]] && doins "${conffile}"
 		done
 	fi
 }
@@ -146,20 +144,8 @@ font_cleanup_dirs() {
 # @FUNCTION: font_pkg_setup
 # @DESCRIPTION:
 # The font pkg_setup function.
-# Collision protection and Prefix compat for eapi < 3.
+# Collision protection
 font_pkg_setup() {
-	# Prefix compat
-	case ${EAPI:-0} in
-		0|1|2)
-			if ! use prefix; then
-				EPREFIX=
-				ED=${D}
-				EROOT=${ROOT}
-				[[ ${EROOT} = */ ]] || EROOT+="/"
-			fi
-			;;
-	esac
-
 	# make sure we get no collisions
 	# setup is not the nicest place, but preinst doesn't cut it
 	if [[ -e "${EROOT%/}/${FONTDIR}/fonts.cache-1" ]] ; then
@@ -181,7 +167,7 @@ font_src_install() {
 			pushd "${dir}" > /dev/null
 			insinto "${FONTDIR}/${dir//${S}/}"
 			for suffix in ${FONT_SUFFIX}; do
-				doins *.${suffix} || die "font installation failed" # TODO old EAPI cleanup
+				doins *.${suffix}
 			done
 			font_xfont_config "${dir}"
 			popd > /dev/null
@@ -190,7 +176,7 @@ font_src_install() {
 		pushd "${FONT_S}" > /dev/null
 		insinto "${FONTDIR}"
 		for suffix in ${FONT_SUFFIX}; do
-			doins *.${suffix} || die "font installation failed" # TODO old EAPI cleanup
+			doins *.${suffix}
 		done
 		font_xfont_config
 		popd > /dev/null
@@ -198,10 +184,10 @@ font_src_install() {
 
 	font_fontconfig
 
-	[[ -n ${DOCS} ]] && { dodoc ${DOCS} || die "docs installation failed" ; } # TODO old EAPI cleanup
+	einstalldocs
 
 	# install common docs
-	for commondoc in COPYRIGHT README{,.md,.txt} NEWS AUTHORS BUGS ChangeLog FONTLOG.txt; do
+	for commondoc in COPYRIGHT FONTLOG.txt; do
 		[[ -s ${commondoc} ]] && dodoc ${commondoc}
 	done
 }
@@ -238,9 +224,7 @@ font_pkg_postinst() {
 		elog "The following fontconfig configuration files have been installed:"
 		elog
 		for conffile in "${FONT_CONF[@]}"; do
-			if [[ -e "${EROOT%/}"/etc/fonts/conf.avail/${conffile##*/} ]]; then
-				elog "  ${conffile##*/}"
-			fi
+			[[ -e "${EROOT%/}"/etc/fonts/conf.avail/${conffile##*/} ]] && elog "  ${conffile##*/}"
 		done
 		elog
 		elog "Use \`eselect fontconfig\` to enable/disable them."
@@ -256,3 +240,6 @@ font_pkg_postrm() {
 	font_cleanup_dirs
 	_update_fontcache
 }
+
+_FONT_ECLASS=1
+fi
-- 
2.23.0

Re: [gentoo-dev] [PATCH] font.eclass: Ban EAPIs < 5

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 1177 bytes --]

>>>>> On Sun, 20 Oct 2019, David Seifert wrote:

> -		if [[ -e fonts.alias ]]; then
> -			doins fonts.alias || die "failed to install fonts.alias" # TODO old EAPI cleanup
> -		fi
> +		[[ -e fonts.alias ]] && doins fonts.alias
>  	fi
>  }

Is the function's return value of any importance? The function will now
return shell false if fonts.alias doesn't exist, while previously it
returned true.

> -	[[ -n ${DOCS} ]] && { dodoc ${DOCS} || die "docs installation failed" ; } # TODO old EAPI cleanup
> +	einstalldocs
 
>  	# install common docs
> -	for commondoc in COPYRIGHT README{,.md,.txt} NEWS AUTHORS BUGS ChangeLog FONTLOG.txt; do
> +	for commondoc in COPYRIGHT FONTLOG.txt; do
>  		[[ -s ${commondoc} ]] && dodoc ${commondoc}
>  	done

This changes the set of installed files, if the DOCS variable is
defined. Is that intentional?

> -			if [[ -e "${EROOT%/}"/etc/fonts/conf.avail/${conffile##*/} ]]; then
> -				elog "  ${conffile##*/}"
> -			fi
> +			[[ -e "${EROOT%/}"/etc/fonts/conf.avail/${conffile##*/} ]] && elog "  ${conffile##*/}"

This doesn't change any functionality, but it adds an overlong line for
no good reason.

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [gentoo-dev] [PATCH] font.eclass: Ban EAPIs < 5

[David Seifert]

On Sun, 2019-10-20 at 19:45 +0200, Ulrich Mueller wrote:
> > > > > > On Sun, 20 Oct 2019, David Seifert wrote:
> > -		if [[ -e fonts.alias ]]; then
> > -			doins fonts.alias || die "failed to install
> > fonts.alias" # TODO old EAPI cleanup
> > -		fi
> > +		[[ -e fonts.alias ]] && doins fonts.alias
> >  	fi
> >  }
> 
> Is the function's return value of any importance? The function will
> now
> return shell false if fonts.alias doesn't exist, while previously it
> returned true.

No.

> 
> > -	[[ -n ${DOCS} ]] && { dodoc ${DOCS} || die "docs installation
> > failed" ; } # TODO old EAPI cleanup
> > +	einstalldocs
>  
> >  	# install common docs
> > -	for commondoc in COPYRIGHT README{,.md,.txt} NEWS AUTHORS BUGS
> > ChangeLog FONTLOG.txt; do
> > +	for commondoc in COPYRIGHT FONTLOG.txt; do
> >  		[[ -s ${commondoc} ]] && dodoc ${commondoc}
> >  	done
> 
> This changes the set of installed files, if the DOCS variable is
> defined. Is that intentional?
> 

You mean if it's *not* defined? Yes, that's intentional in line with
making eclasses behave around DOCS consistently. If that leads to
installing READMEwin32.txt, then so be it. Importantly, this allows
using DOCS as an array without copypasta-ing around the einstalldocs
whitespace-list-vs-array handling code.

> > -			if [[ -e
> > "${EROOT%/}"/etc/fonts/conf.avail/${conffile##*/} ]]; then
> > -				elog "  ${conffile##*/}"
> > -			fi
> > +			[[ -e
> > "${EROOT%/}"/etc/fonts/conf.avail/${conffile##*/} ]] && elog
> > "  ${conffile##*/}"
> 
> This doesn't change any functionality, but it adds an overlong line
> for
> no good reason.

The idea was to avoid if statements if you can use the more succinct
form.

> 
> Ulrich

Re: [gentoo-dev] [PATCH] font.eclass: Ban EAPIs < 5

[Ulrich Mueller]

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

>>>>> On Sun, 20 Oct 2019, David Seifert wrote:

>> > -	[[ -n ${DOCS} ]] && { dodoc ${DOCS} || die "docs installation
>> > failed" ; } # TODO old EAPI cleanup
>> > +	einstalldocs
>> > 
>> >  	# install common docs
>> > -	for commondoc in COPYRIGHT README{,.md,.txt} NEWS AUTHORS BUGS
>> > ChangeLog FONTLOG.txt; do
>> > +	for commondoc in COPYRIGHT FONTLOG.txt; do
>> >  		[[ -s ${commondoc} ]] && dodoc ${commondoc}
>> >  	done

>> This changes the set of installed files, if the DOCS variable is
>> defined. Is that intentional?

> You mean if it's *not* defined?

No, if it *is* defined. For example, if an ebuild defines DOCS but
doesn't include README.txt in the list, then that file was previously
being installed, but isn't any longer with the eclass change.

>> This doesn't change any functionality, but it adds an overlong line
>> for no good reason.

> The idea was to avoid if statements if you can use the more succinct
> form.

Then at least wrap the long line. Still, I doubt that it will improve
readability, as compared to the original if statement.

Ulrich

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: [gentoo-dev] [PATCH] font.eclass: Ban EAPIs < 5

[David Seifert]

On Sun, 2019-10-20 at 20:57 +0200, Ulrich Mueller wrote:
> > > > > > On Sun, 20 Oct 2019, David Seifert wrote:
> > > > -	[[ -n ${DOCS} ]] && { dodoc ${DOCS} || die "docs
> > > > installation
> > > > failed" ; } # TODO old EAPI cleanup
> > > > +	einstalldocs
> > > > 
> > > >  	# install common docs
> > > > -	for commondoc in COPYRIGHT README{,.md,.txt} NEWS
> > > > AUTHORS BUGS
> > > > ChangeLog FONTLOG.txt; do
> > > > +	for commondoc in COPYRIGHT FONTLOG.txt; do
> > > >  		[[ -s ${commondoc} ]] && dodoc ${commondoc}
> > > >  	done
> > > This changes the set of installed files, if the DOCS variable is
> > > defined. Is that intentional?
> > You mean if it's *not* defined?
> 
> No, if it *is* defined. For example, if an ebuild defines DOCS but
> doesn't include README.txt in the list, then that file was previously
> being installed, but isn't any longer with the eclass change.

Now I got it. Yes, this is fine because most fonts actually override
DOCS and include README in there.

> 
> > > This doesn't change any functionality, but it adds an overlong
> > > line
> > > for no good reason.
> > The idea was to avoid if statements if you can use the more
> > succinct
> > form.
> 
> Then at least wrap the long line. Still, I doubt that it will improve
> readability, as compared to the original if statement.

I'll fix it before pushing.

> 
> Ulrich