diff mbox series

kbuild: deb-dpkg: Check optional env variables before use

Message ID 20240625-mkdebian-check-if-optional-vars-are-defined-v1-1-53f196b9f83a@avm.de (mailing list archive)
State New
Headers show
Series kbuild: deb-dpkg: Check optional env variables before use | expand

Commit Message

Nicolas Schier June 25, 2024, 12:45 p.m. UTC 
Add checks to mkdebian for undefined optional environment variables
before evaluating them.

Some variables used by scripts/package/mkdebian are fully optional and
not set by kbuild.  In order to allow enabling 'set -u' (shell script
exits on dereference of undefined variables), introduce the explicit
checking.

Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Nicolas Schier <n.schier@avm.de>
---
This allows application of the original patch
   [PATCH 1/2] kconfig: add -e and -u options to *conf-cfg.sh scripts
   [PATCH 2/2] kbuild: package: add -e and -u options to shell scripts
as sent https://lore.kernel.org/linux-kbuild/20240611160938.3511096-1-masahiroy@kernel.org/
---
 scripts/package/mkdebian | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)


---
base-commit: 3893a22a160edd1c15b483deb3dee36b36ee4226
change-id: 20240625-mkdebian-check-if-optional-vars-are-defined-a46cf0524e4e

Best regards,

Comments

Nathan Chancellor June 25, 2024, 5 p.m. UTC | #1 
On Tue, Jun 25, 2024 at 02:45:56PM +0200, Nicolas Schier wrote:
> Add checks to mkdebian for undefined optional environment variables
> before evaluating them.
> 
> Some variables used by scripts/package/mkdebian are fully optional and
> not set by kbuild.  In order to allow enabling 'set -u' (shell script
> exits on dereference of undefined variables), introduce the explicit
> checking.
> 
> Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Nicolas Schier <n.schier@avm.de>

Looks good to me.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
> This allows application of the original patch
>    [PATCH 1/2] kconfig: add -e and -u options to *conf-cfg.sh scripts
>    [PATCH 2/2] kbuild: package: add -e and -u options to shell scripts
> as sent https://lore.kernel.org/linux-kbuild/20240611160938.3511096-1-masahiroy@kernel.org/
> ---
>  scripts/package/mkdebian | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/package/mkdebian b/scripts/package/mkdebian
> index b9a5b789c655..2a7bb05c7f60 100755
> --- a/scripts/package/mkdebian
> +++ b/scripts/package/mkdebian
> @@ -19,7 +19,7 @@ if_enabled_echo() {
>  }
>  
>  set_debarch() {
> -	if [ -n "$KBUILD_DEBARCH" ] ; then
> +	if [ "${KBUILD_DEBARCH:+set}" ] ; then
>  		debarch="$KBUILD_DEBARCH"
>  		return
>  	fi
> @@ -147,7 +147,7 @@ fi
>  
>  # Some variables and settings used throughout the script
>  version=$KERNELRELEASE
> -if [ -n "$KDEB_PKGVERSION" ]; then
> +if [ "${KDEB_PKGVERSION:+set}" ]; then
>  	packageversion=$KDEB_PKGVERSION
>  else
>  	packageversion=$(${srctree}/scripts/setlocalversion --no-local ${srctree})-$($srctree/scripts/build-version)
> @@ -164,7 +164,7 @@ debarch=
>  set_debarch
>  
>  # Try to determine distribution
> -if [ -n "$KDEB_CHANGELOG_DIST" ]; then
> +if [ "${KDEB_CHANGELOG_DIST:+set}" ]; then
>          distribution=$KDEB_CHANGELOG_DIST
>  # In some cases lsb_release returns the codename as n/a, which breaks dpkg-parsechangelog
>  elif distribution=$(lsb_release -cs 2>/dev/null) && [ -n "$distribution" ] && [ "$distribution" != "n/a" ]; then
> 
> ---
> base-commit: 3893a22a160edd1c15b483deb3dee36b36ee4226
> change-id: 20240625-mkdebian-check-if-optional-vars-are-defined-a46cf0524e4e
> 
> Best regards,
> -- 
> Nicolas Schier
>
Masahiro Yamada July 2, 2024, 5:31 p.m. UTC | #2 
On Tue, Jun 25, 2024 at 9:46 PM Nicolas Schier <n.schier@avm.de> wrote:
>
> Add checks to mkdebian for undefined optional environment variables
> before evaluating them.
>
> Some variables used by scripts/package/mkdebian are fully optional and
> not set by kbuild.  In order to allow enabling 'set -u' (shell script
> exits on dereference of undefined variables), introduce the explicit
> checking.


This patch is not enough for enabling -u.



email=${DEBEMAIL-$EMAIL}

causes an error.


$ unset DEBEMAIL; unset EMAIL; make deb-pkg
  GEN     debian
./scripts/package/mkdebian: 128: EMAIL: parameter not set




I can do this work myself.









>
> Suggested-by: Masahiro Yamada <masahiroy@kernel.org>
> Signed-off-by: Nicolas Schier <n.schier@avm.de>
> ---
> This allows application of the original patch
>    [PATCH 1/2] kconfig: add -e and -u options to *conf-cfg.sh scripts
>    [PATCH 2/2] kbuild: package: add -e and -u options to shell scripts
> as sent https://lore.kernel.org/linux-kbuild/20240611160938.3511096-1-masahiroy@kernel.org/
> ---
>  scripts/package/mkdebian | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/package/mkdebian b/scripts/package/mkdebian
> index b9a5b789c655..2a7bb05c7f60 100755
> --- a/scripts/package/mkdebian
> +++ b/scripts/package/mkdebian
> @@ -19,7 +19,7 @@ if_enabled_echo() {
>  }
>
>  set_debarch() {
> -       if [ -n "$KBUILD_DEBARCH" ] ; then
> +       if [ "${KBUILD_DEBARCH:+set}" ] ; then
>                 debarch="$KBUILD_DEBARCH"
>                 return
>         fi
> @@ -147,7 +147,7 @@ fi
>
>  # Some variables and settings used throughout the script
>  version=$KERNELRELEASE
> -if [ -n "$KDEB_PKGVERSION" ]; then
> +if [ "${KDEB_PKGVERSION:+set}" ]; then
>         packageversion=$KDEB_PKGVERSION
>  else
>         packageversion=$(${srctree}/scripts/setlocalversion --no-local ${srctree})-$($srctree/scripts/build-version)
> @@ -164,7 +164,7 @@ debarch=
>  set_debarch
>
>  # Try to determine distribution
> -if [ -n "$KDEB_CHANGELOG_DIST" ]; then
> +if [ "${KDEB_CHANGELOG_DIST:+set}" ]; then
>          distribution=$KDEB_CHANGELOG_DIST
>  # In some cases lsb_release returns the codename as n/a, which breaks dpkg-parsechangelog
>  elif distribution=$(lsb_release -cs 2>/dev/null) && [ -n "$distribution" ] && [ "$distribution" != "n/a" ]; then
>
> ---
> base-commit: 3893a22a160edd1c15b483deb3dee36b36ee4226
> change-id: 20240625-mkdebian-check-if-optional-vars-are-defined-a46cf0524e4e
>
> Best regards,
> --
> Nicolas Schier
>


--
Best Regards
Masahiro Yamada
diff mbox series

Patch

diff --git a/scripts/package/mkdebian b/scripts/package/mkdebian
index b9a5b789c655..2a7bb05c7f60 100755
--- a/scripts/package/mkdebian
+++ b/scripts/package/mkdebian
@@ -19,7 +19,7 @@  if_enabled_echo() {
 }
 
 set_debarch() {
-	if [ -n "$KBUILD_DEBARCH" ] ; then
+	if [ "${KBUILD_DEBARCH:+set}" ] ; then
 		debarch="$KBUILD_DEBARCH"
 		return
 	fi
@@ -147,7 +147,7 @@  fi
 
 # Some variables and settings used throughout the script
 version=$KERNELRELEASE
-if [ -n "$KDEB_PKGVERSION" ]; then
+if [ "${KDEB_PKGVERSION:+set}" ]; then
 	packageversion=$KDEB_PKGVERSION
 else
 	packageversion=$(${srctree}/scripts/setlocalversion --no-local ${srctree})-$($srctree/scripts/build-version)
@@ -164,7 +164,7 @@  debarch=
 set_debarch
 
 # Try to determine distribution
-if [ -n "$KDEB_CHANGELOG_DIST" ]; then
+if [ "${KDEB_CHANGELOG_DIST:+set}" ]; then
         distribution=$KDEB_CHANGELOG_DIST
 # In some cases lsb_release returns the codename as n/a, which breaks dpkg-parsechangelog
 elif distribution=$(lsb_release -cs 2>/dev/null) && [ -n "$distribution" ] && [ "$distribution" != "n/a" ]; then