Re: [gentoo-dev] [PATCH] app-alternatives.eclass: New eclass to streamline app-alternatives/*

[Michał Górny]

On Sat, 2022-12-03 at 09:19 +0100, Ulrich Mueller wrote:
> > > > > > On Sat, 03 Dec 2022, Michał Górny wrote:
>
> > --- /dev/null
> > +++ b/eclass/app-alternatives.eclass
> > @@ -0,0 +1,84 @@
> > +# Copyright 2022 Gentoo Authors
> > +# Distributed under the terms of the GNU General Public License v2
> > +
> > +# @ECLASS: app-alternatives.eclass
> > +# @MAINTAINER:
> > +# Michał Górny <mgorny@g.o>
> > +# @AUTHOR:
> > +# Michał Górny <mgorny@g.o>
> > +# @SUPPORTED_EAPIS: 8
> > +# @BLURB: Common logic for app-alternatives/*
> > +# @DESCRIPTION:
> > +# This eclass provides common logic shared by app-alternatives/*
> > +# ebuilds.  A global ALTERNATIVES variable needs to be declared
> > +# that lists available options and their respective dependencies.
> > +# HOMEPAGE, S, LICENSE, SLOT, IUSE, REQUIRED_USE and RDEPEND are
> > set.
> > +# A get_alternative() function is provided that prints the
> > determines
>
> Grammar?

Nope, ^w error.

>
> > +# the selected alternative and prints its respective flag name.
> > +
> > +case ${EAPI} in
> > +	8) ;;
> > +	*) die "${ECLASS}: EAPI ${EAPI} unsupported."
>
> ${EAPI:-0} here?

Sure.

>
> > +esac
> > +
> > +if [[ ! ${_APP_ALTERNATIVES_ECLASS} ]]; then
> > +_APP_ALTERNATIVES_ECLASS=1
> > +
> > +# @ECLASS_VARIABLE: ALTERNATIVES
> > +# @PRE_INHERIT
> > +# @REQUIRED
> > +# @DESCRIPTION:
> > +# Array of "flag:dependency" pairs specifying the available
> > +# alternatives.  The default provider must be listed first.
> > +
> > +# @FUNCTION: _app-alternatives_set_globals
> > +# @INTERNAL
> > +# @DESCRIPTION:
> > +# Set ebuild metadata variables.
> > +_app-alternatives_set_globals() {
> > +	debug-print-function ${FUNCNAME} "${@}"
> > +
> > +	if [[ $(declare -p ALTERNATIVES) != "declare -a"* ]]; then
>
> The more modern (and presumably, more reliable) check would be:
>
> 	if [[ ${ALTERNATIVES@a} != *a* ]]; then
>
> Since the eclass supports EAPI 8 only, bash-5.0 is guaranteed, so the
> feature will be available.

Ok.

>
> > +		die 'ALTERNATIVES must be an array.'
> > +	elif [[ ${#ALTERNATIVES[@]} -eq 0 ]]; then
> > +		die 'ALTERNATIVES must not be empty.'
> > +	fi
> > +
> > +	HOMEPAGE="
> > https://wiki.gentoo.org/wiki/Project:Base/Alternatives"
> > +	S=${WORKDIR}
> > +
> > +	LICENSE="CC0-1.0"
> > +	SLOT="0"
> > +
> > +	# yep, that's a cheap hack adding '+' to the first flag
> > +	IUSE="+${ALTERNATIVES[*]%%:*}"
> > +	REQUIRED_USE="^^ ( ${ALTERNATIVES[*]%%:*} )"
> > +	RDEPEND=""
> > +
> > +	local flag dep
> > +	for flag in "${ALTERNATIVES[@]}"; do
> > +		[[ ${flag} != *:* ]] && die "Invalid ALTERNATIVES
> > item: ${flag}"
> > +		dep=${flag#*:}
> > +		flag=${flag%%:*}
> > +		RDEPEND+="
> > +			${flag}? ( ${dep} )
> > +		"
>
> Why two newlines? A single space should be enough.

What's the difference?  It will be normalized by the PM anyway.

>
> > +	done
> > +}
> > +_app-alternatives_set_globals
> > +
> > +# @FUNCTION: get_alternative
> > +# @DESCRIPTION:
> > +# Get the flag name for the selected alterantive (i.e. the USE flag
> > set).
>
> Typo.

Sam already spotted this one.

-- 
Best regards,
Michał Górny