Re: [gentoo-dev] evar_push/pop helpers
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
Re: [gentoo-dev] evar_push/pop helpers
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 signature.asc Description: This is a digitally signed message part.
Re: [gentoo-dev] evar_push/pop helpers
here's v2
-mike
--- eutils.eclass 22 May 2013 05:10:29 - 1.421
+++ eutils.eclass 17 Jun 2013 05:41:58 -
@@ -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
signature.asc
Description: This is a digitally signed message part.
Re: [gentoo-dev] evar_push/pop helpers
Dnia 2013-06-01, o godz. 23:03:20 Mike Frysinger vap...@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 signature.asc Description: PGP signature
Re: [gentoo-dev] evar_push/pop helpers
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 signature.asc Description: This is a digitally signed message part.
Re: [gentoo-dev] evar_push/pop helpers
Dnia 2013-06-02, o godz. 03:09:31
Mike Frysinger vap...@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
signature.asc
Description: PGP signature
Re: [gentoo-dev] evar_push/pop helpers
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
signature.asc
Description: This is a digitally signed message part.
Re: [gentoo-dev] evar_push/pop helpers
On Sun, 2 Jun 2013 03:09:31 -0400
Mike Frysinger vap...@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 : tom...@gentoo.org
GPG Public Key : 6D34E57D
GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D
signature.asc
Description: PGP signature
Re: [gentoo-dev] evar_push/pop helpers
On Sun, 2 Jun 2013 03:29:33 -0400 Mike Frysinger vap...@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 : tom...@gentoo.org GPG Public Key : 6D34E57D GPG Fingerprint : C165 AF18 AB4C 400B C3D2 ABF0 95B2 1FCD 6D34 E57D signature.asc Description: PGP signature
Re: [gentoo-dev] evar_push/pop helpers
Dnia 2013-06-02, o godz. 03:29:33
Mike Frysinger vap...@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
signature.asc
Description: PGP signature
Re: [gentoo-dev] evar_push/pop helpers
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 signature.asc Description: This is a digitally signed message part.
Re: [gentoo-dev] evar_push/pop helpers
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 dilfri...@gentoo.org http://www.akhuettel.de/
[gentoo-dev] evar_push/pop helpers
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 - 1.421
+++ eutils.eclass 2 Jun 2013 03:00:46 -
@@ -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
signature.asc
Description: This is a digitally signed message part.
