Re: [PATCH 1/2] kbuild: Abort make on install failures

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Feb 11, 2024 at 6:21 AM Nicolas Schier <nicolas@xxxxxxxxx> wrote:

On Sat, Feb 10, 2024 at 10:29:00AM +0000 Russell King (Oracle) wrote:
On Sat, Feb 10, 2024 at 03:46:00PM +0800, Zhang Bingwu wrote:
From: Zhang Bingwu <xtexchooser@xxxxxxxx>

Setting '-e' flag tells shells to exit with error exit code immediately
after any of commands fails, and causes make(1) to regard recipes as
failed.

Before this, make will still continue to succeed even after the
installation failed, for example, for insufficient permission or
directory does not exist.

Signed-off-by: Zhang Bingwu <xtexchooser@xxxxxxxx>
---

Thanks for fixing!

[...]
diff --git a/arch/arm/boot/install.sh b/arch/arm/boot/install.sh
index 9ec11fac7d8d..34e2c6e31fd1 100755
--- a/arch/arm/boot/install.sh
+++ b/arch/arm/boot/install.sh
@@ -17,6 +17,8 @@
 #   $3 - kernel map file
 #   $4 - default install path (blank if root directory)

+set -e
+

What about #!/bin/sh -e on the first line, which is the more normal way
to do this for an entire script?

are you sure?  I can find many more occurrences of 'set -e' than the
shebang version in the Linux tree, especially in the kbuild scripts, thus
it's bike-shedding, isn't it?

Reviewed-by: Nicolas Schier <nicolas@xxxxxxxxx>

Kind regards,
Nicolas






When you put -e on the shebang line, like

    #!/bin/sh -e

the option -e is set when you do:

    $ arch/arm/boot/install.sh


But, -e is not set when you do:

    $ sh arch/arm/boot/install.sh



The reason is obvious because the latter case
does not use the shebang line.




In Kbuild, some places run the script directly like the former case,
and others use CONFIG_SHELL like

   $(CONFIG_SHELL) arch/arm/boot/install.sh


The inconsistency is not nice, but that is a different issue.


The separate 'set -e' statement works for both cases,
so I think this is safer, though it is kind of bike-shedding.





-- 
Best Regards
Masahiro Yamada





[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux