[1/2] kbuild: Abort make on install failures
Commit Message
From: Zhang Bingwu <xtexchooser@duck.com>
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@duck.com>
---
arch/arm/boot/install.sh | 2 ++
arch/arm64/boot/install.sh | 2 ++
arch/m68k/install.sh | 2 ++
arch/nios2/boot/install.sh | 2 ++
arch/parisc/install.sh | 2 ++
arch/riscv/boot/install.sh | 2 ++
arch/s390/boot/install.sh | 2 ++
arch/sparc/boot/install.sh | 2 ++
arch/x86/boot/install.sh | 2 ++
9 files changed, 18 insertions(+)
Comments
On Sat, Feb 10, 2024 at 03:46:00PM +0800, Zhang Bingwu wrote:
> From: Zhang Bingwu <xtexchooser@duck.com>
>
> 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@duck.com>
> ---
> arch/arm/boot/install.sh | 2 ++
> arch/arm64/boot/install.sh | 2 ++
> arch/m68k/install.sh | 2 ++
> arch/nios2/boot/install.sh | 2 ++
> arch/parisc/install.sh | 2 ++
> arch/riscv/boot/install.sh | 2 ++
> arch/s390/boot/install.sh | 2 ++
> arch/sparc/boot/install.sh | 2 ++
> arch/x86/boot/install.sh | 2 ++
> 9 files changed, 18 insertions(+)
>
> 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?
On Saturday, February 10, 2024 6:29:00 PM CST Russell King (Oracle) wrote:
> What about #!/bin/sh -e on the first line, which is the more normal way
> to do this for an entire script?
Will do this in V2.
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@duck.com>
> >
> > 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@duck.com>
> > ---
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@fjasle.eu>
Kind regards,
Nicolas
On Sun, Feb 11, 2024 at 6:21 AM Nicolas Schier <nicolas@fjasle.eu> 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@duck.com>
> > >
> > > 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@duck.com>
> > > ---
>
> 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@fjasle.eu>
>
> 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.
On Sunday, February 11, 2024 7:35:35 AM CST Masahiro Yamada wrote:
>
> The separate 'set -e' statement works for both cases,
> so I think this is safer, though it is kind of bike-shedding.
Thanks!
I also think it is safer to use 'set -e' in the case of 'sh install.sh',
so I support not to use 'sh -e' in the shebang line. The planned V2 patch for
this disappeared.
@@ -17,6 +17,8 @@
# $3 - kernel map file
# $4 - default install path (blank if root directory)
+set -e
+
if [ "$(basename $2)" = "zImage" ]; then
# Compressed install
echo "Installing compressed kernel"
@@ -17,6 +17,8 @@
# $3 - kernel map file
# $4 - default install path (blank if root directory)
+set -e
+
if [ "$(basename $2)" = "Image.gz" ] || [ "$(basename $2)" = "vmlinuz.efi" ]
then
# Compressed install
@@ -16,6 +16,8 @@
# $3 - kernel map file
# $4 - default install path (blank if root directory)
+set -e
+
if [ -f $4/vmlinuz ]; then
mv $4/vmlinuz $4/vmlinuz.old
fi
@@ -16,6 +16,8 @@
# $3 - kernel map file
# $4 - default install path (blank if root directory)
+set -e
+
if [ -f $4/vmlinuz ]; then
mv $4/vmlinuz $4/vmlinuz.old
fi
@@ -16,6 +16,8 @@
# $3 - kernel map file
# $4 - default install path (blank if root directory)
+set -e
+
if [ "$(basename $2)" = "vmlinuz" ]; then
# Compressed install
echo "Installing compressed kernel"
@@ -17,6 +17,8 @@
# $3 - kernel map file
# $4 - default install path (blank if root directory)
+set -e
+
if [ "$(basename $2)" = "Image.gz" ]; then
# Compressed install
echo "Installing compressed kernel"
@@ -15,6 +15,8 @@
# $3 - kernel map file
# $4 - default install path (blank if root directory)
+set -e
+
echo "Warning: '${INSTALLKERNEL}' command not available - additional " \
"bootloader config required" >&2
if [ -f "$4/vmlinuz-$1" ]; then mv -- "$4/vmlinuz-$1" "$4/vmlinuz-$1.old"; fi
@@ -16,6 +16,8 @@
# $3 - kernel map file
# $4 - default install path (blank if root directory)
+set -e
+
if [ -f $4/vmlinuz ]; then
mv $4/vmlinuz $4/vmlinuz.old
fi
@@ -16,6 +16,8 @@
# $3 - kernel map file
# $4 - default install path (blank if root directory)
+set -e
+
if [ -f $4/vmlinuz ]; then
mv $4/vmlinuz $4/vmlinuz.old
fi