[pacman-dev] [PATCH 1/3] makepkg: use bash 4.4 to localize `set` without explicitly saving/restoring

[Allan McRae]

On 29/08/18 15:20, Eli Schwartz wrote:
> On 8/29/18 12:54 AM, Allan McRae wrote:
>> On 14/08/18 11:20, Eli Schwartz wrote:
>>> Signed-off-by: Eli Schwartz <eschwartz at archlinux.org>
>>> ---
>>>  scripts/makepkg.sh.in | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
>>> index 4024f477..bb8332c6 100644
>>> --- a/scripts/makepkg.sh.in
>>> +++ b/scripts/makepkg.sh.in
>>> @@ -418,13 +418,14 @@ prepare_buildenv() {
>>>  }
>>>  
>>>  run_function_safe() {
>>> -	local restoretrap restoreset restoreshopt
>>> +	local restoretrap restoreshopt
>>>  
>>>  	# we don't set any special shopts of our own, but we don't want the user to
>>>  	# muck with our environment.
>>>  	restoreshopt=$(shopt -p)
>>>  
>>> -	restoreset=$(shopt -o -p)
>>> +	# localize sets, sadly this does not work for shopt
>>> +	local -
>>
>> My understanding is this does not quite do the same thing...   "local -"
>> only save the single-letter shell options.  There are set options that
>> do not have a single letter variants (although they are unlikely to be
>> used...).
> 
> The manpage does not indicate this; it just says:
> 
> the set of shell options is made local to the function in which local is
> invoked: shell options changed using the set builtin inside the function
> are restored to their original values when the function returns.
> 

I was going off the bash-4.4 release notes.

> So...
> 
> $ testfunc() { shopt -p -o pipefail; local -; set -o pipefail; shopt -p
> -o pipefail; }
> $ testfunc; shopt -p -o pipefail
> set +o pipefail
> set -o pipefail
> set +o pipefail
> 
> Seems to work okay, fortunately.

Great.  Patch is fine then.   I'll adjust the comment to "localize set
options" as "localize sets" is not clear.

A