public inbox for gentoo-dev@lists.gentoo.org
 help / color / mirror / Atom feed
From: "Steven J. Long" <slong@rathaus.eclipse.co.uk>
To: gentoo-dev@lists.gentoo.org
Subject: [gentoo-dev] Re: evar_push/pop helpers
Date: Sun, 2 Jun 2013 18:38:04 +0100	[thread overview]
Message-ID: <20130602173804.GA4280@rathaus.eclipse.co.uk> (raw)
In-Reply-To: <201306012303.21261.vapier@gentoo.org>

On Sat, Jun 01, 2013 at 11:03:20PM -0400, Mike Frysinger wrote:
> simple set of helpers to save/restore a variable in a limited section of code
> 
> you can see an example of it in action at the end of the file where i need to
> tweak epatch (and no, doing `LC_COLLATE=C set -- ....` does not work).
> -mike
> 
> --- eutils.eclass	22 May 2013 05:10:29 -0000	1.421
> +++ eutils.eclass	2 Jun 2013 03:00:46 -0000
> @@ -146,6 +146,77 @@ estack_pop() {
>  	eval unset ${__estack_name}\[${__estack_i}\]
>  }
Just in passing, one of the places you don't want nullglob messing things up.
  
> +# @FUNCTION: evar_push
> +# @USAGE: <variable to save> [more vars to save]
> +# @DESCRIPTION:
> +# This let's you temporarily modify a variable and then restore it (including
> +# set vs unset semantics).  Arrays are not supported at this time.
> +#
> +# For example:
> +# @CODE
> +#		evar_push LC_ALL
> +#		export LC_ALL=C
> +#		... do some stuff that needs LC_ALL=C set ...
> +#		evar_pop
> +#
> +#		# You can also save/restore more than one var at a time
> +#		evar_push BUTTERFLY IN THE SKY
> +#		... do stuff with the vars ...
> +#		evar_pop     # This restores just one var, SKY
> +#		... do more stuff ...
> +#		evar_pop 3   # This pops the remaining 3 vars
> +# @CODE
> +evar_push() {
> +	local var val
> +	for var ; do
> +		[[ ${!var+set} == "set" ]] \
> +			&& val=${!var} \
> +			|| val="${___ECLASS_ONCE_EUTILS}"
> +		estack_push evar "${var}" "${val}"
> +	done
> +}
> +
> +# @FUNCTION: evar_push_set
> +# @USAGE: <variable to save> [new value to store]
> +# @DESCRIPTION:
> +# This is a handy shortcut to save and temporarily set a variable.  If a value
> +# is not specified, the var will be unset.
> +evar_push_set() {
> +	local var=$1
> +	evar_push ${var}
> +	case $# in
> +	1) unset ${var} ;;
> +	2) eval ${var}=\$2 ;;

I wish you wouldn't use eval for this. I know it's technically okay here, or would be
if you verified the parameter, but bash has printf -v for this purpose:

printf -v "$1" '%s' "$2" 2>/dev/null || die "unable to set: '$1' to: '$2'"

Note you should verify the variable name, ime, irrespective of a die on the printf
(or eval in sh.) It's much better feedback to the developer using the routine.
Here's what we currently use in sh:
# lib_key_ok "$input_name"
# is the input name ok as a variable or key identifier
lib_key_ok(){
	case $1 in
	''|[![:alpha:]_]*|*[![:alnum:]_]*) return 1
;;	*) return 0
	esac
}
			
# lib_check_key foo bar..
# die if any param is not lib_key_ok
lib_check_key() {
	local i
	for i; do
		lib_key_ok "$i" || die "bad key: '$i'"
	done
}
							
So I'd probably just use: lib_key_ok "$1" || die "$FUNCNAME: bad varname: '$1'"
in this context, or amend the check_key function to use:
  .. || die "${FUNCNAME[1]}: bad key '$i'"
- since you're working in bash by definition.

printf -v also works with array members btw, if you do decide to extend in that
direction:
$ set -- 'foo[2]' 'array madness'
$ printf -v "$1" %s "$2"
$ echo "'${foo[2]}'"
'array madness'

And yeah, you can compose the variable name, eg:
set -- foo 'array 1' 1
printf -v "$1${3:+[$3]}" %s "$2"
echo "'${foo[1]}'"

So long as the developer using the routines knows about quoting, that works fine.
(If they don't, you have bigger problems.)

> +	*) die "${FUNCNAME}: incorrect # of args: $*" ;;
> +	esac
> +}
> +
> +# @FUNCTION: evar_pop
> +# @USAGE: [number of vars to restore]
> +# @DESCRIPTION:
> +# Restore the variables to the state saved with the corresponding
> +# evar_push call.  See that function for more details.
> +evar_pop() {
> +	local cnt=$1
might as well use: cnt=${1:-bad}
> +	case $# in
> +	0) cnt=1 ;;
> +	1)
and lose this line.
> +		: ${cnt:=bad}
> +		[[ -n ${cnt//[0-9]} ]] && die "${FUNCNAME}: first arg must be a number: $*"
> +		;;
Though a generic is_int function comes in much handier, ime.
is_int() {
	[[ $1 && $1 != *[^0-9]* ]]
}
	1) is_int "$1" || die ..
		cnt=$1

(I tend to guard against 0? as well, as octal inputs tend to mess things up later. I'm
sure you can see the case for sh.)

> +	*) die "${FUNCNAME}: only accepts one arg: $*" ;;
> +	esac
> +
> +	local var val
> +	while (( cnt-- )) ; do
> +		estack_pop evar val || die "${FUNCNAME}: unbalanced push"
> +		estack_pop evar var || die "${FUNCNAME}: unbalanced push"
> +		[[ ${val} == "${___ECLASS_ONCE_EUTILS}" ]] \
> +			&& unset ${var} \
> +			|| eval ${var}=\${val}

again: printf -v "$var" %s "$val"
Though I'd rather this were in an if, so you can test for error better:

if [[ $val = "$___ECLASS_ONCE_EUTILS" ]]; then
	unset "$var" 2>/dev/null || die "unable to unset: '$var'"
else printf -v "$var" %s "$val" 2>/dev/null \
   || die "unable to reset: '$var'"
fi

(Or you can use: fi 2>/dev/null || die "unable to pop: '$var'"
if you prefer.)

> +	done
> +}
> +
>  # @FUNCTION: eshopts_push
>  # @USAGE: [options to `set` or `shopt`]
>  # @DESCRIPTION:
> @@ -344,8 +415,11 @@ epatch() {
>  		local EPATCH_SUFFIX=$1
>  
>  	elif [[ -d $1 ]] ; then
> -		# Some people like to make dirs of patches w/out suffixes (vim)
> +		# We have to force sorting to C so that the wildcard expansion is consistent #471666.
> +		evar_push_set LC_COLLATE C
> +		# Some people like to make dirs of patches w/out suffixes (vim).
>  		set -- "$1"/*${EPATCH_SUFFIX:+."${EPATCH_SUFFIX}"}
> +		evar_pop

Have to say I'd just do this in a function adding to a locally-scoped array, if it were me,
eg local args; foo "$1" "$EPATCH_SUFFIX"; set -- "${args[@]}"; unset args
foo() {
	local LC_COLLATE=C
	args=("$1"/*${2:+."$2"})
}
though I'd prefer it if EPATCH_SUFFIX were allowed to be a list, personally.
Something like this (untested) with foo "$1" $EPATCH_SUFFIX
foo() {
	local LC_COLLATE=C
	set_nullglob
	case $# in
	0) fnusage
;;	1) args=("$1"/*)
;; 2) args=("$1"/*${2+".$2"})
;;	*) local i n=0 d=$1
		shift; args=()
		for i; do
			[[ $i ]] || { n=1; continue; }
			args+=("$d"/*."$i")
		done
		# if you want to allow for empty args from an ebuild to mean sth:
		((n && ! ${#args[*]})) && args=("$d"/*)
	esac
	reset_nullglob
}
(I'm not discounting dealing with the input variable to check for array etc,
it's just not my concern here.)

HTH,
steveL.
-- 
#friendly-coders -- We're friendly, but we're not /that/ friendly ;-)


  parent reply	other threads:[~2013-06-02 17:39 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-02  3:03 [gentoo-dev] evar_push/pop helpers Mike Frysinger
2013-06-02  6:51 ` Michał Górny
2013-06-02  7:09   ` Mike Frysinger
2013-06-02  7:16     ` Michał Górny
2013-06-02  7:29       ` Mike Frysinger
2013-06-02  7:48         ` Tom Wijsman
2013-06-17  5:45           ` Mike Frysinger
2013-06-02  8:39         ` Michał Górny
2013-06-02 15:40           ` Mike Frysinger
2013-06-02 15:57             ` Andreas K. Huettel
2013-06-02  7:33     ` Tom Wijsman
2013-06-02 17:38 ` Steven J. Long [this message]
2013-06-17  5:42   ` [gentoo-dev] " Mike Frysinger
2013-06-17 16:06     ` Mike Frysinger
2013-06-17  5:46 ` [gentoo-dev] " Mike Frysinger
2013-06-17 17:51   ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130602173804.GA4280@rathaus.eclipse.co.uk \
    --to=slong@rathaus.eclipse.co.uk \
    --cc=gentoo-dev@lists.gentoo.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox