[gentoo-dev] evar_push/pop helpers

[gentoo-dev] evar_push/pop helpers

[Mike Frysinger]

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

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}\]
 }
 
+# @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 ;;
+	*) 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
+	case $# in
+	0) cnt=1 ;;
+	1)
+		: ${cnt:=bad}
+		[[ -n ${cnt//[0-9]} ]] && die "${FUNCNAME}: first arg must be a number: $*"
+		;;
+	*) 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}
+	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
 
 	elif [[ -f ${EPATCH_SOURCE}/$1 ]] ; then
 		# Re-use EPATCH_SOURCE as a search dir

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

Re: [gentoo-dev] evar_push/pop helpers

[Michał Górny]

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

Dnia 2013-06-01, o godz. 23:03:20
Mike Frysinger <vapier@gentoo.org> napisał(a):

> 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).

Why the ugly hackery instead of proper 'local' scope?

-- 
Best regards,
Michał Górny

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

Re: [gentoo-dev] evar_push/pop helpers

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 595 bytes --]

On Sunday 02 June 2013 02:51:34 Michał Górny wrote:
> Dnia 2013-06-01, o godz. 23:03:20 Mike Frysinger napisał(a):
> > 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).
> 
> Why the ugly hackery instead of proper 'local' scope?

there's no way to undo the local thus it affects the rest of the func.  this 
makes sure the change is actually localized to where it is needed.
-mike

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

Re: [gentoo-dev] evar_push/pop helpers

[Michał Górny]

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

Dnia 2013-06-02, o godz. 03:09:31
Mike Frysinger <vapier@gentoo.org> napisał(a):

> On Sunday 02 June 2013 02:51:34 Michał Górny wrote:
> > Dnia 2013-06-01, o godz. 23:03:20 Mike Frysinger napisał(a):
> > > 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).
> > 
> > Why the ugly hackery instead of proper 'local' scope?
> 
> there's no way to undo the local thus it affects the rest of the func.  this 
> makes sure the change is actually localized to where it is needed.

By use of global variables and a bunch of additional code and evals.

Is:

  local _epatch_foo=${foo}
  local foo
  ...
  foo=${_epatch_foo}

really that hurtful?

Also, do you really want the collation to be changed only in this one
place? This looks weird to me. Much like fixing tiny bug and trying to
avoid checking whether anything else is affected.

-- 
Best regards,
Michał Górny

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

Re: [gentoo-dev] evar_push/pop helpers

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1885 bytes --]

On Sunday 02 June 2013 03:16:53 Michał Górny wrote:
> Dnia 2013-06-02, o godz. 03:09:31 Mike Frysinger napisał(a):
> > On Sunday 02 June 2013 02:51:34 Michał Górny wrote:
> > > Dnia 2013-06-01, o godz. 23:03:20 Mike Frysinger napisał(a):
> > > > 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).
> > > 
> > > Why the ugly hackery instead of proper 'local' scope?
> > 
> > there's no way to undo the local thus it affects the rest of the func. 
> > this makes sure the change is actually localized to where it is needed.
> 
> By use of global variables and a bunch of additional code and evals.

the implementation details of estack_* doesn't matter

> Is:
> 
>   local _epatch_foo=${foo}
>   local foo
>   ...
>   foo=${_epatch_foo}
> 
> really that hurtful?

except you aren't handling edge cases (like set vs unset), and i've replicated 
this pattern before (in fact, you've filed bugs where the set/unset case wasn't 
handled).  API 101: implement it once correctly to simplify everyone else.

> Also, do you really want the collation to be changed only in this one
> place? This looks weird to me.

yes, i only want to force it here, because it's the only place where collation 
matters in the func currently.

> Much like fixing tiny bug and trying to
> avoid checking whether anything else is affected.

yeah, because forcing specific behavior for an entire function is always the 
correct answer.  it's like telling people to export LC_ALL=C in make.conf so 
they never hit locale related problems.

feel free to read the whole func yourself if you think there are other places 
where this matters.
-mike

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

Re: [gentoo-dev] evar_push/pop helpers

[Tom Wijsman]

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

On Sun, 2 Jun 2013 03:09:31 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> there's no way to undo the local thus it affects the rest of the
> func.  this makes sure the change is actually localized to where it
> is needed. -mike

In other languages you can freely introduce local scopes { ... }, this
isn't possible in Bash since a local corresponds to a function; but
it's not really that hard to replicate, now consider this instead:

	test()
	{
		local test="FUNCTION"
		echo ${test}

		x(){
			local test="LOCAL SCOPE 1"
			echo ${test}
		};x

		echo ${test}

		x(){
			local test="LOCAL SCOPE 2"
			echo ${test}
		};x
		
		echo ${test}
	}

	test

Now 'x' is vague, you could replace 'x' by a name documenting the scope.

I consider this to be more clean than using a variable to remember it,
especially when multiple local scopes are to be used after each other.

-- 
With kind regards,

Tom Wijsman (TomWij)
Gentoo Developer

E-mail address  : TomWij@gentoo.org
GPG Public Key  : 6D34E57D
GPG Fingerprint : C165 AF18 AB4C 400B C3D2  ABF0 95B2 1FCD 6D34 E57D

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

Re: [gentoo-dev] evar_push/pop helpers

[Tom Wijsman]

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

On Sun, 2 Jun 2013 03:29:33 -0400
Mike Frysinger <vapier@gentoo.org> wrote:

> except you aren't handling edge cases (like set vs unset)

You've got me there, though this is quite an exception; I don't see
why we have to introduce something that's barely used...

`qgrep -eH '^\s*\bset\b' | wc -l`
yields 150

`qgrep -eH '^\s*\bset\b' | sed 's/:.*//g' | sed
's/\/[a-zA-Z0-9_.-]*$//g' | sort -u | wc -l`
yields 34

Only 34 packages, and do these all _really_ need a 'set'? I doubt it.

> API 101: implement it once correctly to simplify everyone else.

For set this would make sense, unless someone finds a way to deal with
that too; any Bash experts around here?

> > Much like fixing tiny bug and trying to
> > avoid checking whether anything else is affected.
> 
> yeah, because forcing specific behavior for an entire function is
> always the correct answer.  it's like telling people to export
> LC_ALL=C in make.conf so they never hit locale related problems.

Actually, bug wranglers stopped asked for English build logs because
setting LC_ALL=C ended up breaking things.

-- 
With kind regards,

Tom Wijsman (TomWij)
Gentoo Developer

E-mail address  : TomWij@gentoo.org
GPG Public Key  : 6D34E57D
GPG Fingerprint : C165 AF18 AB4C 400B C3D2  ABF0 95B2 1FCD 6D34 E57D

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

Re: [gentoo-dev] evar_push/pop helpers

[Michał Górny]

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

Dnia 2013-06-02, o godz. 03:29:33
Mike Frysinger <vapier@gentoo.org> napisał(a):

> On Sunday 02 June 2013 03:16:53 Michał Górny wrote:
> > Dnia 2013-06-02, o godz. 03:09:31 Mike Frysinger napisał(a):
> > > On Sunday 02 June 2013 02:51:34 Michał Górny wrote:
> > > > Dnia 2013-06-01, o godz. 23:03:20 Mike Frysinger napisał(a):
> > > > > 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).
> > > > 
> > > > Why the ugly hackery instead of proper 'local' scope?
> > > 
> > > there's no way to undo the local thus it affects the rest of the func. 
> > > this makes sure the change is actually localized to where it is needed.
> > 
> > By use of global variables and a bunch of additional code and evals.
> 
> the implementation details of estack_* doesn't matter

It's not beautiful language with proper local scopes, so it *does*
matter.

> > Is:
> > 
> >   local _epatch_foo=${foo}
> >   local foo
> >   ...
> >   foo=${_epatch_foo}
> > 
> > really that hurtful?
> 
> except you aren't handling edge cases (like set vs unset), and i've replicated 
> this pattern before (in fact, you've filed bugs where the set/unset case wasn't 
> handled).  API 101: implement it once correctly to simplify everyone else.

Edge cases matter where they actually matter. Inventing use cases just
to prove we need something doesn't help anyone.

And we can't implement it correctly since we're talking about bash.
Bash doesn't provide means to implement it correctly.

> > Also, do you really want the collation to be changed only in this one
> > place? This looks weird to me.
> 
> yes, i only want to force it here, because it's the only place where collation 
> matters in the func currently.

So, effectively, changing it once in the beginning of the function
would be simpler and wouldn't cost anything.

> 
> > Much like fixing tiny bug and trying to
> > avoid checking whether anything else is affected.
> 
> yeah, because forcing specific behavior for an entire function is always the 
> correct answer.  it's like telling people to export LC_ALL=C in make.conf so 
> they never hit locale related problems.

Feel free to try that. But on yourself, not our users. Or just grep
bugzie.

-- 
Best regards,
Michał Górny

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

Re: [gentoo-dev] evar_push/pop helpers

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1929 bytes --]

On Sunday 02 June 2013 04:39:32 Michał Górny wrote:
> Dnia 2013-06-02, o godz. 03:29:33 Mike Frysinger napisał(a):
> > On Sunday 02 June 2013 03:16:53 Michał Górny wrote:
> > > Dnia 2013-06-02, o godz. 03:09:31 Mike Frysinger napisał(a):
> > > > On Sunday 02 June 2013 02:51:34 Michał Górny wrote:
> > > > > Dnia 2013-06-01, o godz. 23:03:20 Mike Frysinger napisał(a):
> > > > > > 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).
> > > > > 
> > > > > Why the ugly hackery instead of proper 'local' scope?
> > > > 
> > > > there's no way to undo the local thus it affects the rest of the
> > > > func. this makes sure the change is actually localized to where it
> > > > is needed.
> > > 
> > > By use of global variables and a bunch of additional code and evals.
> > 
> > the implementation details of estack_* doesn't matter
> 
> It's not beautiful language with proper local scopes, so it *does*
> matter.

then go ahead and propose something different.  otherwise you're pointlessly 
twisting in the wind.

> > > Also, do you really want the collation to be changed only in this one
> > > place? This looks weird to me.
> > 
> > yes, i only want to force it here, because it's the only place where
> > collation matters in the func currently.
> 
> So, effectively, changing it once in the beginning of the function
> would be simpler and wouldn't cost anything.

most likely, *today*, yes.  in the future, who knows.  but since this is the 
only place in the func where we need to force a specific sorting, it makes 
sense to localize the change to that.

i snipped the rest of your e-mail because it wasn't worth responding to
-mike

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

Re: [gentoo-dev] evar_push/pop helpers

[Andreas K. Huettel]

Am Sonntag, 2. Juni 2013, 17:40:45 schrieb Mike Frysinger:
>
> i snipped the rest of your e-mail because it wasn't worth responding to
> -mike

Please. Save your social skills for other people.

-- 

Andreas K. Huettel
Gentoo Linux developer 
dilfridge@gentoo.org
http://www.akhuettel.de/

[gentoo-dev] Re: evar_push/pop helpers

[Steven J. Long]

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 ;-)

Re: [gentoo-dev] Re: evar_push/pop helpers

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 3493 bytes --]

On Sunday 02 June 2013 13:38:04 Steven J. Long wrote:
> On Sat, Jun 01, 2013 at 11:03:20PM -0400, Mike Frysinger wrote:
> > --- 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.

not sure what you mean.  that escapes the [] so that bash doesn't accidentally 
glob things.  when the code actually executes, you don't need to escape 
things.

	touch f i d x
	unset arr
	declare -A arr
	arr[f]=1234
	arr[idx]=4321
	echo "${arr[f]}" "${arr[idx]}"
	__estack_name=arr
	__estack_i=f
	eval unset ${__estack_name}\[${__estack_i}\]
	echo ${#arr[@]}
	__estack_i=idx
	eval unset ${__estack_name}\[${__estack_i}\]
	echo ${#arr[@]}

seems to work

> > +# 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:

interesting, i hadn't seen that before ... looks new to bash-3.1.  /me tucks 
that into his tool belt.

although it doesn't quite work in the edge case where the value is an empty 
string.  consider:
	unset x
	printf -v x ''
	echo ${x+set}

that should show "set", but it does not.  i'll have to keep `eval ${var}=` 
when the value we're setting is empty.  or just keep the eval code since i 
have to do eval anyways at that point.

i'll report it upstream to the bash guys.

> 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.

i don't think it's worth it.  if you screw up the name, it tends to be a one-
time cost, and the error shown is pretty clear as to where it's coming from.

> printf -v also works with array members btw, if you do decide to extend in
> that direction:

i don't think i will, but i probably can convert the other core stack code to 
use this ...

> > +		: ${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.

yeah, and we have other code that wants this (a simple grep for 0-9 in eclass/ 
shows a bunch of hits)

> > -		# 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"})
> }

since i plan on using these funcs in other places, using the existing api 
keeps things simple

> though I'd prefer it if EPATCH_SUFFIX were allowed to be a list,
> personally.

wouldn't really make this any easier
-mike

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

Re: [gentoo-dev] evar_push/pop helpers

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1205 bytes --]

On Sunday 02 June 2013 03:48:17 Tom Wijsman wrote:
> On Sun, 2 Jun 2013 03:29:33 -0400 Mike Frysinger wrote:
> > except you aren't handling edge cases (like set vs unset)
> 
> You've got me there, though this is quite an exception; I don't see
> why we have to introduce something that's barely used...
> 
> `qgrep -eH '^\s*\bset\b' | wc -l`
> yields 150
> 
> `qgrep -eH '^\s*\bset\b' | sed 's/:.*//g' | sed
> 's/\/[a-zA-Z0-9_.-]*$//g' | sort -u | wc -l`
> yields 34
> 
> Only 34 packages, and do these all _really_ need a 'set'? I doubt it.

i've got at least 4 consumers in mind, and that's more than enough imo to 
implement it right than half-ass it every time

> > > Much like fixing tiny bug and trying to
> > > avoid checking whether anything else is affected.
> > 
> > yeah, because forcing specific behavior for an entire function is
> > always the correct answer.  it's like telling people to export
> > LC_ALL=C in make.conf so they never hit locale related problems.
> 
> Actually, bug wranglers stopped asked for English build logs because
> setting LC_ALL=C ended up breaking things.

i know it doesn't work which is why i was providing it as an example
-mike

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

Re: [gentoo-dev] evar_push/pop helpers

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 3499 bytes --]

here's v2
-mike

--- eutils.eclass	22 May 2013 05:10:29 -0000	1.421
+++ eutils.eclass	17 Jun 2013 05:41:58 -0000
@@ -146,6 +146,79 @@ estack_pop() {
 	eval unset ${__estack_name}\[${__estack_i}\]
 }
 
+# @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.
+#
+# This is meant for variables where using `local` does not work (such as
+# exported variables, or only temporarily changing things in a func).
+#
+# 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} ;;
+	# Can't use `printf -v` as that does not set $var when $2 is "".
+	2) eval ${var}=\$2 ;;
+	*) 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:-bad}
+	case $# in
+	0) cnt=1 ;;
+	1) isdigit "${cnt}" || die "${FUNCNAME}: first arg must be a number: $*" ;;
+	*) 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"
+		# Can't use `printf -v` as that does not set $var when $2 is "".
+		[[ ${val} == "${___ECLASS_ONCE_EUTILS}" ]] \
+			&& unset ${var} \
+			|| eval ${var}=\${val}
+	done
+}
+
 # @FUNCTION: eshopts_push
 # @USAGE: [options to `set` or `shopt`]
 # @DESCRIPTION:
@@ -218,6 +291,18 @@ eumask_pop() {
 	umask ${s} || die "${FUNCNAME}: sanity: could not restore umask: ${s}"
 }
 
+# @FUNCTION: isdigit
+# @USAGE: <number> [more numbers]
+# @DESCRIPTION:
+# Return true if all arguments are numbers.
+isdigit() {
+	local d
+	for d ; do
+		[[ ${d:-bad} == *[!0-9]* ]] && return 1
+	done
+	return 0
+}
+
 # @VARIABLE: EPATCH_SOURCE
 # @DESCRIPTION:
 # Default directory to search for patches.
@@ -344,8 +429,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
 
 	elif [[ -f ${EPATCH_SOURCE}/$1 ]] ; then
 		# Re-use EPATCH_SOURCE as a search dir

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

Re: [gentoo-dev] Re: evar_push/pop helpers

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1198 bytes --]

On Monday 17 June 2013 01:42:15 Mike Frysinger wrote:
> On Sunday 02 June 2013 13:38:04 Steven J. Long wrote:
> > On Sat, Jun 01, 2013 at 11:03:20PM -0400, Mike Frysinger wrote:
> > > +# 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:
> 
> interesting, i hadn't seen that before ... looks new to bash-3.1.  /me
> tucks that into his tool belt.
> 
> although it doesn't quite work in the edge case where the value is an empty
> string.  consider:
> 	unset x
> 	printf -v x ''
> 	echo ${x+set}
> 
> that should show "set", but it does not.  i'll have to keep `eval ${var}=`
> when the value we're setting is empty.  or just keep the eval code since i
> have to do eval anyways at that point.
> 
> i'll report it upstream to the bash guys.

looks like it can be worked around by doing:
	printf -v x '%s' ''
which is arguably what we want anyways
-mike

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

Re: [gentoo-dev] evar_push/pop helpers

[Greg KH]

On Mon, Jun 17, 2013 at 01:46:02AM -0400, Mike Frysinger wrote:
> here's v2

These changes look good to me, and quite useful, thanks for doing this
work.

greg k-h