Message ID | 20180115165759.26127-6-lukeshu@lukeshu.com |
---|---|
State | Accepted |
Headers | show |
Series | Backports from Parabola v20180103 | expand |
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.
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.
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
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(-)