[devtools,5/7] lib/common.sh: Adjust to work properly with `set -u`

Message ID 20180115165759.26127-6-lukeshu@lukeshu.com
State Accepted
Headers show
Series Backports from Parabola v20180103 | expand

Commit Message

Luke Shumaker Jan. 15, 2018, 4:57 p.m. UTC
From: Luke Shumaker <lukeshu@parabola.nu>

Support for working with `set -u` was broken by 94160d6.  Egg on my
face; I'm the one who wants `set -u` support, and I'm the author of
that commit!

libmakepkg does not work with `set -u`; but mostly because of the include
guards!  So we just need to temporarily disable `set -u` (nounset) while
loading libmakepkg.  Instead of introducing a new variable, just store the
initial nounset status in _INCLUDE_COMMON_SH; rather than a useless
fixed-string "true".

While we're at it, disable POSIX-mode (just in case we're running as "sh"
instead of "bash"), since libmakepkg uses bash-isms that won't parse in
POSIX mode.
---
 lib/common.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Emil Velikov via arch-projects Jan. 15, 2018, 5:25 p.m. UTC | #1
On 01/15/2018 11:57 AM, Luke Shumaker wrote:
> From: Luke Shumaker <lukeshu@parabola.nu>

> libmakepkg does not work with `set -u`; but mostly because of the include
> guards!  So we just need to temporarily disable `set -u` (nounset) while
> loading libmakepkg.  Instead of introducing a new variable, just store the
> initial nounset status in _INCLUDE_COMMON_SH; rather than a useless
> fixed-string "true".

Would it make sense to fix this in libmakepkg instead? devtools are not
the only project that would reuse libmakepkg components. Also, working
towards having makepkg be able to use set -u could be interesting.
Luke Shumaker Jan. 15, 2018, 7:34 p.m. UTC | #2
On Mon, 15 Jan 2018 12:25:22 -0500,
Eli Schwartz via arch-projects wrote:
> 
> [1 Re: [arch-projects] [devtools] [PATCH 5/7] lib/common.sh: Adjust to work properly with `set -u` <multipart/mixed (7bit)>]
> [1.1  <text/plain; utf-8 (quoted-printable)>]
> On 01/15/2018 11:57 AM, Luke Shumaker wrote:
> > From: Luke Shumaker <lukeshu@parabola.nu>
> 
> > libmakepkg does not work with `set -u`; but mostly because of the include
> > guards!  So we just need to temporarily disable `set -u` (nounset) while
> > loading libmakepkg.  Instead of introducing a new variable, just store the
> > initial nounset status in _INCLUDE_COMMON_SH; rather than a useless
> > fixed-string "true".
> 
> Would it make sense to fix this in libmakepkg instead? devtools are not
> the only project that would reuse libmakepkg components. Also, working
> towards having makepkg be able to use set -u could be interesting.

Long-term: It absolutely would make sense.  But libmakepkg has a lot
slower release schedule than devtools/libretools, and I needed to get
a release out :) libretools has a few consumers of common.sh that `set
-u`; the switch to libmakepkg would have broken them otherwise.

Patch

diff --git a/lib/common.sh b/lib/common.sh
index a3c2ec2..821f8df 100644
--- a/lib/common.sh
+++ b/lib/common.sh
@@ -4,10 +4,12 @@ 
 # License: Unspecified
 
 [[ -z ${_INCLUDE_COMMON_SH:-} ]] || return 0
-_INCLUDE_COMMON_SH=true
+_INCLUDE_COMMON_SH="$(set +o|grep nounset)"
 
+set +u +o posix
 # shellcheck disable=1091
 . /usr/share/makepkg/util.sh
+$_INCLUDE_COMMON_SH
 
 # Avoid any encoding problems
 export LANG=C