diff mbox series

[bug#60069,2/2] guix-install.sh: Directly exit in case of errors in chk_require.

Message ID 20221214155603.29381-2-maxim.cournoyer@gmail.com
State New
Headers
Series [bug#60068,1/2] guix-install.sh: Add GUIX_ALLOW_OVERWRITE environment variable. |

Commit Message

Maxim Cournoyer Dec. 14, 2022, 3:56 p.m. UTC 
* etc/guix-install.sh (chk_require): Directly exit in case of errors in
chk_require, instead of relying on 'set -e'.
---
 etc/guix-install.sh | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Tobias Geerinckx-Rice Dec. 14, 2022, 4:37 p.m. UTC | #1 
Maxim Cournoyer 写道：
> -    [ "${#warn}" -ne 0 ] &&
> -        { _err "${ERR}Missing commands: ${warn[*]}.";
> -          return 1; }
> -    
> +    [ "${#warn}" -ne 0 ] && die "Missing commands: ${warn[*]}."
> +

I did not run this, but will it not itself trigger -e  when the 
test is false?

Kind regards,

T G-R
Maxim Cournoyer Dec. 14, 2022, 6:17 p.m. UTC | #2 
Hi Tobias,

Tobias Geerinckx-Rice <me@tobias.gr> writes:

> Maxim Cournoyer 写道：
>> -    [ "${#warn}" -ne 0 ] &&
>> -        { _err "${ERR}Missing commands: ${warn[*]}.";
>> -          return 1; }
>> -    +    [ "${#warn}" -ne 0 ] && die "Missing commands:
>> ${warn[*]}."
>> +
>
> I did not run this, but will it not itself trigger -e  when the test
> is false?

This apparently falls in the special casing by Bash of what is
considered a failure when using 'set -e'; here's a test:

--8<---------------cut here---------------start------------->8---
$ cat test.sh
#!/usr/bin/env bash

set -e

[ false ] && echo "hey, we made it!"
--8<---------------cut here---------------end--------------->8---

--8<---------------cut here---------------start------------->8---
$ ./test.sh
hey, we made it!
--8<---------------cut here---------------end--------------->8---

I hope this answers your question.
Maxim Cournoyer Dec. 14, 2022, 6:33 p.m. UTC | #3 
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hi Tobias,
>
> Tobias Geerinckx-Rice <me@tobias.gr> writes:
>
>> Maxim Cournoyer 写道：
>>> -    [ "${#warn}" -ne 0 ] &&
>>> -        { _err "${ERR}Missing commands: ${warn[*]}.";
>>> -          return 1; }
>>> -    +    [ "${#warn}" -ne 0 ] && die "Missing commands:
>>> ${warn[*]}."
>>> +
>>
>> I did not run this, but will it not itself trigger -e  when the test
>> is false?
>
> This apparently falls in the special casing by Bash of what is
> considered a failure when using 'set -e'; here's a test:
>
> $ cat test.sh
> #!/usr/bin/env bash
>
> set -e
>
> [ false ] && echo "hey, we made it!"
>
> $ ./test.sh
> hey, we made it!

The above example was bogus and unnecessary; looking at it more closely,
the test would return true when the 'warn' array contains 1 or more
items (missing commands), which would cause the die command to be
invoked and the script to exit.  The first test handling isn't modified,
so it'll chain though the second part the same as it does now.

I hope that's a better explanation.
diff mbox series

Patch

diff --git a/etc/guix-install.sh b/etc/guix-install.sh
index 06730f7e3f..0ca12f8b66 100755
--- a/etc/guix-install.sh
+++ b/etc/guix-install.sh
@@ -137,10 +137,8 @@  chk_require()
         command -v "$c" &>/dev/null || warn+=("$c")
     done
 
-    [ "${#warn}" -ne 0 ] &&
-        { _err "${ERR}Missing commands: ${warn[*]}.";
-          return 1; }
-    
+    [ "${#warn}" -ne 0 ] && die "Missing commands: ${warn[*]}."
+
     _msg "${PAS}verification of required commands completed"
 }