linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] kbuild: Fix install errors when INSTALL_PATH does not exist
@ 2024-02-10  7:45 Zhang Bingwu
  2024-02-10  7:46 ` [PATCH 1/2] kbuild: Abort make on install failures Zhang Bingwu
  2024-02-10  7:46 ` [PATCH 2/2] kbuild: Create INSTALL_PATH directory if it does not exist Zhang Bingwu
  0 siblings, 2 replies; 11+ messages in thread
From: Zhang Bingwu @ 2024-02-10  7:45 UTC (permalink / raw)
  To: Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Dinh Nguyen, James E.J. Bottomley, Helge Deller, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin, Zhang Bingwu,
	Masahiro Yamada, Nathan Chancellor, Nicolas Schier
  Cc: x86, linux-arm-kernel, linux-kernel, linux-m68k, linux-parisc,
	linux-riscv, linux-s390, sparclinux

From: Zhang Bingwu <xtexchooser@duck.com>

When running 'make zinstall INSTALL_PATH=somepath'
 where 'somepath' does not exist, the install
 scripts (install.sh) print error messages
 but also return a success status code.
This will make 'make' regard 'install' (and 'zinstall', etc)
 succeeded.
When there are also other targets at the same time,
 for example, 'make zinstall dtbs_install modules_install',
 make will keep going on and other outputs will fill stdout,
 and make the error message hard to find.

dtbs_install and modules_install creates the target directory
 if it does not exist. install, zinstall and others should
 have the same behaviour.

If INSTALL_PATH is not a valid directory, we should create it.
If the installation process still fails with errors, for example,
 insufficient space on disk or permission denied, make should regard
 the install target failed, stop as soon as possible,
 and exit with error.

Zhang Bingwu (2):
  kbuild: Abort make on install failures
  kbuild: Create INSTALL_PATH directory if it does not exist

 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 ++
 scripts/install.sh         | 4 ++++
 10 files changed, 22 insertions(+)


base-commit: d0f86d080e3d7d5e1e75a56d88daf8e5f56a4146
-- 
2.43.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] kbuild: Abort make on install failures
  2024-02-10  7:45 [PATCH 0/2] kbuild: Fix install errors when INSTALL_PATH does not exist Zhang Bingwu
@ 2024-02-10  7:46 ` Zhang Bingwu
  2024-02-10 10:29   ` Russell King (Oracle)
  2024-02-10  7:46 ` [PATCH 2/2] kbuild: Create INSTALL_PATH directory if it does not exist Zhang Bingwu
  1 sibling, 1 reply; 11+ messages in thread
From: Zhang Bingwu @ 2024-02-10  7:46 UTC (permalink / raw)
  To: Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Dinh Nguyen, James E.J. Bottomley, Helge Deller, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin, Zhang Bingwu,
	Masahiro Yamada, Nathan Chancellor, Nicolas Schier
  Cc: x86, linux-arm-kernel, linux-kernel, linux-m68k, linux-parisc,
	linux-riscv, linux-s390, sparclinux

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
+
 if [ "$(basename $2)" = "zImage" ]; then
 # Compressed install
   echo "Installing compressed kernel"
diff --git a/arch/arm64/boot/install.sh b/arch/arm64/boot/install.sh
index 9b7a09808a3d..cc2f4ccca6c0 100755
--- a/arch/arm64/boot/install.sh
+++ b/arch/arm64/boot/install.sh
@@ -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
diff --git a/arch/m68k/install.sh b/arch/m68k/install.sh
index af65e16e5147..b6829b3942b3 100755
--- a/arch/m68k/install.sh
+++ b/arch/m68k/install.sh
@@ -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
diff --git a/arch/nios2/boot/install.sh b/arch/nios2/boot/install.sh
index 34a2feec42c8..1161f2bf59ec 100755
--- a/arch/nios2/boot/install.sh
+++ b/arch/nios2/boot/install.sh
@@ -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
diff --git a/arch/parisc/install.sh b/arch/parisc/install.sh
index 933d031c249a..664c2d77f776 100755
--- a/arch/parisc/install.sh
+++ b/arch/parisc/install.sh
@@ -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"
diff --git a/arch/riscv/boot/install.sh b/arch/riscv/boot/install.sh
index 4c63f3f0643d..a59639dff64f 100755
--- a/arch/riscv/boot/install.sh
+++ b/arch/riscv/boot/install.sh
@@ -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"
diff --git a/arch/s390/boot/install.sh b/arch/s390/boot/install.sh
index a13dd2f2aa1c..fa41486258ee 100755
--- a/arch/s390/boot/install.sh
+++ b/arch/s390/boot/install.sh
@@ -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
diff --git a/arch/sparc/boot/install.sh b/arch/sparc/boot/install.sh
index 4f130f3f30d6..68de67c5621e 100755
--- a/arch/sparc/boot/install.sh
+++ b/arch/sparc/boot/install.sh
@@ -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
diff --git a/arch/x86/boot/install.sh b/arch/x86/boot/install.sh
index 0849f4b42745..93784abcd66d 100755
--- a/arch/x86/boot/install.sh
+++ b/arch/x86/boot/install.sh
@@ -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
-- 
2.43.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] kbuild: Create INSTALL_PATH directory if it does not exist
  2024-02-10  7:45 [PATCH 0/2] kbuild: Fix install errors when INSTALL_PATH does not exist Zhang Bingwu
  2024-02-10  7:46 ` [PATCH 1/2] kbuild: Abort make on install failures Zhang Bingwu
@ 2024-02-10  7:46 ` Zhang Bingwu
  2024-02-10 21:26   ` Nicolas Schier
  1 sibling, 1 reply; 11+ messages in thread
From: Zhang Bingwu @ 2024-02-10  7:46 UTC (permalink / raw)
  To: Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Dinh Nguyen, James E.J. Bottomley, Helge Deller, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin, Zhang Bingwu,
	Masahiro Yamada, Nathan Chancellor, Nicolas Schier
  Cc: x86, linux-arm-kernel, linux-kernel, linux-m68k, linux-parisc,
	linux-riscv, linux-s390, sparclinux

From: Zhang Bingwu <xtexchooser@duck.com>

If INSTALL_PATH is not a valid directory, create it, like what
modules_install and dtbs_install will do in the same situation.

Signed-off-by: Zhang Bingwu <xtexchooser@duck.com>
---
 scripts/install.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/install.sh b/scripts/install.sh
index 9bb0fb44f04a..02b845e7ab33 100755
--- a/scripts/install.sh
+++ b/scripts/install.sh
@@ -20,6 +20,10 @@ do
 	fi
 done
 
+if [ "${INSTALL_PATH}" != "" ] && ! [ -e "${INSTALL_PATH}" ]; then
+	mkdir -p "${INSTALL_PATH}"
+fi
+
 # User/arch may have a custom install script
 for file in "${HOME}/bin/${INSTALLKERNEL}"		\
 	    "/sbin/${INSTALLKERNEL}"			\
-- 
2.43.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] kbuild: Abort make on install failures
  2024-02-10  7:46 ` [PATCH 1/2] kbuild: Abort make on install failures Zhang Bingwu
@ 2024-02-10 10:29   ` Russell King (Oracle)
  2024-02-10 10:33     ` xtex
  2024-02-10 21:19     ` Nicolas Schier
  0 siblings, 2 replies; 11+ messages in thread
From: Russell King (Oracle) @ 2024-02-10 10:29 UTC (permalink / raw)
  To: Zhang Bingwu
  Cc: Catalin Marinas, Will Deacon, Geert Uytterhoeven, Dinh Nguyen,
	James E.J. Bottomley, Helge Deller, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, David S. Miller,
	Andreas Larsson, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H. Peter Anvin, Zhang Bingwu, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, x86, linux-arm-kernel,
	linux-kernel, linux-m68k, linux-parisc, linux-riscv, linux-s390,
	sparclinux

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?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] kbuild: Abort make on install failures
  2024-02-10 10:29   ` Russell King (Oracle)
@ 2024-02-10 10:33     ` xtex
  2024-02-10 21:19     ` Nicolas Schier
  1 sibling, 0 replies; 11+ messages in thread
From: xtex @ 2024-02-10 10:33 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Catalin Marinas, Will Deacon, Geert Uytterhoeven, Dinh Nguyen,
	James E.J. Bottomley, Helge Deller, Paul Walmsley, Palmer Dabbelt,
	Albert Ou, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, David S. Miller,
	Andreas Larsson, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H. Peter Anvin, Masahiro Yamada, Nathan Chancellor,
	Nicolas Schier, x86, linux-arm-kernel, linux-kernel, linux-m68k,
	linux-parisc, linux-riscv, linux-s390, sparclinux,
	Linux Kbuild mailing list


[-- Attachment #1.1: Type: text/plain, Size: 249 bytes --]

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.

-- 
xtex @ Sat Feb 10 10:32:32 AM UTC 2024

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] kbuild: Abort make on install failures
  2024-02-10 10:29   ` Russell King (Oracle)
  2024-02-10 10:33     ` xtex
@ 2024-02-10 21:19     ` Nicolas Schier
  2024-02-10 23:35       ` Masahiro Yamada
  1 sibling, 1 reply; 11+ messages in thread
From: Nicolas Schier @ 2024-02-10 21:19 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Zhang Bingwu, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Dinh Nguyen, James E.J. Bottomley, Helge Deller, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin, Zhang Bingwu,
	Masahiro Yamada, Nathan Chancellor, x86, linux-arm-kernel,
	linux-kernel, linux-m68k, linux-parisc, linux-riscv, linux-s390,
	sparclinux


[-- Attachment #1.1: Type: text/plain, Size: 1330 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/2] kbuild: Create INSTALL_PATH directory if it does not exist
  2024-02-10  7:46 ` [PATCH 2/2] kbuild: Create INSTALL_PATH directory if it does not exist Zhang Bingwu
@ 2024-02-10 21:26   ` Nicolas Schier
  0 siblings, 0 replies; 11+ messages in thread
From: Nicolas Schier @ 2024-02-10 21:26 UTC (permalink / raw)
  To: Zhang Bingwu
  Cc: Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Dinh Nguyen, James E.J. Bottomley, Helge Deller, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin, Zhang Bingwu,
	Masahiro Yamada, Nathan Chancellor, x86, linux-arm-kernel,
	linux-kernel, linux-m68k, linux-parisc, linux-riscv, linux-s390,
	sparclinux


[-- Attachment #1.1: Type: text/plain, Size: 917 bytes --]

On Sat, Feb 10, 2024 at 03:46:01PM +0800 Zhang Bingwu wrote:
> From: Zhang Bingwu <xtexchooser@duck.com>
> 
> If INSTALL_PATH is not a valid directory, create it, like what
> modules_install and dtbs_install will do in the same situation.
> 
> Signed-off-by: Zhang Bingwu <xtexchooser@duck.com>
> ---
>  scripts/install.sh | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/scripts/install.sh b/scripts/install.sh
> index 9bb0fb44f04a..02b845e7ab33 100755
> --- a/scripts/install.sh
> +++ b/scripts/install.sh
> @@ -20,6 +20,10 @@ do
>  	fi
>  done
>  
> +if [ "${INSTALL_PATH}" != "" ] && ! [ -e "${INSTALL_PATH}" ]; then
> +	mkdir -p "${INSTALL_PATH}"
> +fi
> +
>  # User/arch may have a custom install script
>  for file in "${HOME}/bin/${INSTALLKERNEL}"		\
>  	    "/sbin/${INSTALLKERNEL}"			\
> -- 
> 2.43.0
> 

Thanks.

Reviewed-by: Nicolas Schier <nicolas@jasle.eu>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] kbuild: Abort make on install failures
  2024-02-10 21:19     ` Nicolas Schier
@ 2024-02-10 23:35       ` Masahiro Yamada
  2024-02-11 10:31         ` xtex
  0 siblings, 1 reply; 11+ messages in thread
From: Masahiro Yamada @ 2024-02-10 23:35 UTC (permalink / raw)
  To: Nicolas Schier
  Cc: Russell King (Oracle), Zhang Bingwu, Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Dinh Nguyen, James E.J. Bottomley,
	Helge Deller, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, David S. Miller,
	Andreas Larsson, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H. Peter Anvin, Zhang Bingwu, Nathan Chancellor, x86,
	linux-arm-kernel, linux-kernel, linux-m68k, linux-parisc,
	linux-riscv, linux-s390, sparclinux

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.





-- 
Best Regards
Masahiro Yamada

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] kbuild: Abort make on install failures
  2024-02-10 23:35       ` Masahiro Yamada
@ 2024-02-11 10:31         ` xtex
  0 siblings, 0 replies; 11+ messages in thread
From: xtex @ 2024-02-11 10:31 UTC (permalink / raw)
  To: Nicolas Schier, Masahiro Yamada
  Cc: Russell King (Oracle), Catalin Marinas, Will Deacon,
	Geert Uytterhoeven, Dinh Nguyen, James E.J. Bottomley,
	Helge Deller, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
	Christian Borntraeger, Sven Schnelle, David S. Miller,
	Andreas Larsson, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	Dave Hansen, H. Peter Anvin, Nathan Chancellor, x86,
	linux-arm-kernel, linux-kernel, linux-m68k, linux-parisc,
	linux-riscv, linux-s390, sparclinux

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.

-- 
Zhang Bingwu @ Sun Feb 11 10:27:48 AM UTC 2024




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] kbuild: Abort make on install failures
  2024-07-14  8:57 [RESEND PATCH 0/2] kbuild: Fix install errors when INSTALL_PATH " Zhang Bingwu
@ 2024-07-14  8:57 ` Zhang Bingwu
  2024-07-14 19:10   ` Masahiro Yamada
  0 siblings, 1 reply; 11+ messages in thread
From: Zhang Bingwu @ 2024-07-14  8:57 UTC (permalink / raw)
  To: Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Dinh Nguyen, James E.J. Bottomley, Helge Deller, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin, Zhang Bingwu,
	Masahiro Yamada, Nathan Chancellor, Nicolas Schier
  Cc: x86, linux-kbuild, linux-arm-kernel, linux-kernel, linux-m68k,
	linux-parisc, linux-riscv, linux-s390, sparclinux

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
+
 if [ "$(basename $2)" = "zImage" ]; then
 # Compressed install
   echo "Installing compressed kernel"
diff --git a/arch/arm64/boot/install.sh b/arch/arm64/boot/install.sh
index 9b7a09808a3d..cc2f4ccca6c0 100755
--- a/arch/arm64/boot/install.sh
+++ b/arch/arm64/boot/install.sh
@@ -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
diff --git a/arch/m68k/install.sh b/arch/m68k/install.sh
index af65e16e5147..b6829b3942b3 100755
--- a/arch/m68k/install.sh
+++ b/arch/m68k/install.sh
@@ -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
diff --git a/arch/nios2/boot/install.sh b/arch/nios2/boot/install.sh
index 34a2feec42c8..1161f2bf59ec 100755
--- a/arch/nios2/boot/install.sh
+++ b/arch/nios2/boot/install.sh
@@ -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
diff --git a/arch/parisc/install.sh b/arch/parisc/install.sh
index 933d031c249a..664c2d77f776 100755
--- a/arch/parisc/install.sh
+++ b/arch/parisc/install.sh
@@ -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"
diff --git a/arch/riscv/boot/install.sh b/arch/riscv/boot/install.sh
index 4c63f3f0643d..a59639dff64f 100755
--- a/arch/riscv/boot/install.sh
+++ b/arch/riscv/boot/install.sh
@@ -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"
diff --git a/arch/s390/boot/install.sh b/arch/s390/boot/install.sh
index a13dd2f2aa1c..fa41486258ee 100755
--- a/arch/s390/boot/install.sh
+++ b/arch/s390/boot/install.sh
@@ -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
diff --git a/arch/sparc/boot/install.sh b/arch/sparc/boot/install.sh
index 4f130f3f30d6..68de67c5621e 100755
--- a/arch/sparc/boot/install.sh
+++ b/arch/sparc/boot/install.sh
@@ -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
diff --git a/arch/x86/boot/install.sh b/arch/x86/boot/install.sh
index 0849f4b42745..93784abcd66d 100755
--- a/arch/x86/boot/install.sh
+++ b/arch/x86/boot/install.sh
@@ -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
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] kbuild: Abort make on install failures
  2024-07-14  8:57 ` [PATCH 1/2] kbuild: Abort make on install failures Zhang Bingwu
@ 2024-07-14 19:10   ` Masahiro Yamada
  0 siblings, 0 replies; 11+ messages in thread
From: Masahiro Yamada @ 2024-07-14 19:10 UTC (permalink / raw)
  To: Zhang Bingwu
  Cc: Russell King, Catalin Marinas, Will Deacon, Geert Uytterhoeven,
	Dinh Nguyen, James E.J. Bottomley, Helge Deller, Paul Walmsley,
	Palmer Dabbelt, Albert Ou, Heiko Carstens, Vasily Gorbik,
	Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
	David S. Miller, Andreas Larsson, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin, Zhang Bingwu,
	Nathan Chancellor, Nicolas Schier, x86, linux-kbuild,
	linux-arm-kernel, linux-kernel, linux-m68k, linux-parisc,
	linux-riscv, linux-s390, sparclinux

On Sun, Jul 14, 2024 at 5:58 PM Zhang Bingwu <xtex@envs.net> 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>


Applied to linux-kbuild. Thanks!


-- 
Best Regards
Masahiro Yamada


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2024-07-14 19:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-10  7:45 [PATCH 0/2] kbuild: Fix install errors when INSTALL_PATH does not exist Zhang Bingwu
2024-02-10  7:46 ` [PATCH 1/2] kbuild: Abort make on install failures Zhang Bingwu
2024-02-10 10:29   ` Russell King (Oracle)
2024-02-10 10:33     ` xtex
2024-02-10 21:19     ` Nicolas Schier
2024-02-10 23:35       ` Masahiro Yamada
2024-02-11 10:31         ` xtex
2024-02-10  7:46 ` [PATCH 2/2] kbuild: Create INSTALL_PATH directory if it does not exist Zhang Bingwu
2024-02-10 21:26   ` Nicolas Schier
  -- strict thread matches above, loose matches on Subject: below --
2024-07-14  8:57 [RESEND PATCH 0/2] kbuild: Fix install errors when INSTALL_PATH " Zhang Bingwu
2024-07-14  8:57 ` [PATCH 1/2] kbuild: Abort make on install failures Zhang Bingwu
2024-07-14 19:10   ` Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).