Re: [gentoo-dev] [PATCH 3/4] acct-user.eclass: use an eclass variable for the override variable name

["Michał Górny" <mgorny@gentoo.org>]

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

On Thu, 2024-05-09 at 07:57 -0400, Michael Orlitzky wrote:
> If (after upcasing and replacing hyphens by underscores) ACCT_USER_NAME
> is set to something that isn't valid in a bash variable name, the eclass
> will crash: it uses ACCT_USER_NAME to construct the make.conf override
> variables in pkg_pretend() and src_install().
> 
> This commit relocates the computation of the "override name" subsequent
> to the definition of ACCT_USER_NAME="${PN}". In Gentoo, policy prohibits
> redefinition of that variable; so nothing is changed. But in an overlay,
> this allows the ebuild to define ACCT_USER_NAME to something that would
> not be valid in a bash variable at the expense of violating expectations
> about the override variable names.
> 
> Signed-off-by: Michael Orlitzky <mjo@gentoo.org>
> ---
>  eclass/acct-user.eclass | 32 ++++++++++++++++++++++----------
>  1 file changed, 22 insertions(+), 10 deletions(-)
> 
> diff --git a/eclass/acct-user.eclass b/eclass/acct-user.eclass
> index a4fe5e9f5e34..fb3ccd2044da 100644
> --- a/eclass/acct-user.eclass
> +++ b/eclass/acct-user.eclass
> @@ -64,6 +64,23 @@ inherit user-info
>  # package naming restrictions would prohibit some otherwise-valid usernames.
>  ACCT_USER_NAME=${PN}
>  
> +# @ECLASS_VARIABLE: _OVERRIDE_NAME

This is not prefixed correctly.

> +# @INTERNAL
> +# @DESCRIPTION:
> +# A version of the user name suitable for use in a bash variable. This
> +# is used to construct the names of the make.conf override variables and
> +# it will always be uppercase $PN with the hyphens replaced by
> +# underscores. It is defined once, here, so that in an overlay the
> +# ebuild may set ACCT_USER_NAME to something other than $PN without
> +# affecting the name of the override variable. This is necessary if the
> +# username in question would lead to an invalid bash variable
> +# name. Doing so does violate expectations with respect to the override
> +# naming convention, but (a) your warranty is already void if you are
> +# changing ACCT_USER_NAME or using make.conf overrides, and (b) you
> +# shouldn't need make.conf overrides in an overlay.
> +_OVERRIDE_NAME=${ACCT_USER_NAME^^}
> +_OVERRIDE_NAME=${_OVERRIDE_NAME//-/_}

Any reason not to just use PN directly below?  After all, you need to
sanitize it anyway.

> +
>  # @ECLASS_VARIABLE: ACCT_USER_ID
>  # @REQUIRED
>  # @DESCRIPTION:
> @@ -231,8 +248,7 @@ acct-user_pkg_pretend() {
>  	local user_id=${ACCT_USER_ID}
>  
>  	# check for the override
> -	local override_name=${ACCT_USER_NAME^^}
> -	local override_var=ACCT_USER_${override_name//-/_}_ID
> +	local override_var=ACCT_USER_${_OVERRIDE_NAME}_ID
>  	if [[ -n ${!override_var} ]]; then
>  		user_id=${!override_var}
>  		[[ ${user_id} -ge -1 ]] || die "${override_var}=${user_id} invalid!"
> @@ -274,11 +290,9 @@ acct-user_src_install() {
>  	local ACCT_USER_GROUPS=${ACCT_USER_GROUPS[*]}
>  
>  	# support make.conf overrides
> -	local override_name=${ACCT_USER_NAME^^}
> -	override_name=${override_name//-/_}
>  	local var
>  	for var in ACCT_USER_{ID,COMMENT,SHELL,HOME{,_OWNER,_PERMS},GROUPS}; do
> -		local var_name=ACCT_USER_${override_name}_${var#ACCT_USER_}
> +		local var_name=ACCT_USER_${_OVERRIDE_NAME}_${var#ACCT_USER_}
>  		if [[ -n ${!var_name} ]]; then
>  			ewarn "${var_name}=${!var_name} override in effect, support will not be provided."
>  		else
> @@ -286,7 +300,7 @@ acct-user_src_install() {
>  		fi
>  		declare -g "_${var}=${!var_name}"
>  	done
> -	var_name=ACCT_USER_${override_name}_GROUPS_ADD
> +	var_name=ACCT_USER_${_OVERRIDE_NAME}_GROUPS_ADD
>  	if [[ -n ${!var_name} ]]; then
>  		ewarn "${var_name}=${!var_name} override in effect, support will not be provided."
>  		_ACCT_USER_GROUPS+=" ${!var_name}"
> @@ -436,12 +450,10 @@ acct-user_pkg_postinst() {
>  		has "${g}" "${groups[@]}" || del_groups+="${del_groups:+, }${g}"
>  	done
>  	if [[ -n ${del_groups} ]]; then
> -		local override_name=${ACCT_USER_NAME^^}
> -		override_name=${override_name//-/_}
>  		ewarn "Removing user ${ACCT_USER_NAME} from group(s): ${del_groups}"
>  		ewarn "To retain the user's group membership in the local system"
> -		ewarn "config, override with ACCT_USER_${override_name}_GROUPS or"
> -		ewarn "ACCT_USER_${override_name}_GROUPS_ADD in make.conf."
> +		ewarn "config, override with ACCT_USER_${_OVERRIDE_NAME}_GROUPS or"
> +		ewarn "ACCT_USER_${_OVERRIDE_NAME}_GROUPS_ADD in make.conf."
>  		ewarn "Documentation reference:"
>  		ewarn "https://wiki.gentoo.org/wiki/Practical_guide_to_the_GLEP_81_migration#Override_user_groups"
>  	fi

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 512 bytes --]