| Message ID | 20240611160938.3511096-2-masahiroy@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] kconfig: add -e and -u options to *conf-cfg.sh scripts | expand |
On Wed, Jun 12, 2024 at 01:08:06AM +0900, Masahiro Yamada wrote: > Set -e to make these scripts fail on the first error. > > Set -u because these scripts are invoked by Makefile, and do not work > properly without necessary variables defined. > > Remove the explicit "test -n ..." from scripts/package/install-extmod-build. > > Both options are described in POSIX. [1] > > [1]: https://pubs.opengroup.org/onlinepubs/009604499/utilities/set.html > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- Reviewed-by: Nicolas Schier <nicolas@fjasle.eu>
On Wed, Jun 12, 2024 at 1:09 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > Set -e to make these scripts fail on the first error. > > Set -u because these scripts are invoked by Makefile, and do not work > properly without necessary variables defined. > > Remove the explicit "test -n ..." from scripts/package/install-extmod-build. > > Both options are described in POSIX. [1] > > [1]: https://pubs.opengroup.org/onlinepubs/009604499/utilities/set.html > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > --- Setting -u needs more careful review and test. This patch will break 'make deb-pkg'. ./scripts/package/mkdebian: 150: KDEB_PKGVERSION: parameter not set To set -u, scripts/package/mkdebian needs code refactoring. I will keep scripts/package/mkdebian untouched.
On Mon, Jun 17, 2024 at 12:21:15AM +0900, Masahiro Yamada wrote: > On Wed, Jun 12, 2024 at 1:09 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > Set -e to make these scripts fail on the first error. > > > > Set -u because these scripts are invoked by Makefile, and do not work > > properly without necessary variables defined. > > > > Remove the explicit "test -n ..." from scripts/package/install-extmod-build. > > > > Both options are described in POSIX. [1] > > > > [1]: https://pubs.opengroup.org/onlinepubs/009604499/utilities/set.html > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > --- > > > > Setting -u needs more careful review and test. > > > This patch will break 'make deb-pkg'. > > > ./scripts/package/mkdebian: 150: KDEB_PKGVERSION: parameter not set > > > > > To set -u, scripts/package/mkdebian needs code refactoring. > > > > I will keep scripts/package/mkdebian untouched. uh, I missed that during the review. Do you want to refactor mkdebian in large scale, or is an explicit fallback definition possibly acceptable for you? diff --git a/scripts/package/mkdebian b/scripts/package/mkdebian index ecfeb34b99aa..7e3878197041 100755 --- a/scripts/package/mkdebian +++ b/scripts/package/mkdebian @@ -7,5 +7,17 @@ set -eu +# Optional user-specified environment variables + +# Set target Debian architecture (skip auto-detection) +: "${KBUILD_DEBARCH:=}" + +# Set target Debian distribution (skipping auto-detection) +: "${KDEB_CHANGELOG_DIST:=}" + +# Overwrite the automatically determined package version. +: ${KDEB_PKGVERSION:=} + + is_enabled() { grep -q "^$1=y" include/config/auto.conf } Kind regards, Nicolas
On Mon, Jun 17, 2024 at 12:56 AM Nicolas Schier <nicolas@fjasle.eu> wrote: > > On Mon, Jun 17, 2024 at 12:21:15AM +0900, Masahiro Yamada wrote: > > On Wed, Jun 12, 2024 at 1:09 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > Set -e to make these scripts fail on the first error. > > > > > > Set -u because these scripts are invoked by Makefile, and do not work > > > properly without necessary variables defined. > > > > > > Remove the explicit "test -n ..." from scripts/package/install-extmod-build. > > > > > > Both options are described in POSIX. [1] > > > > > > [1]: https://pubs.opengroup.org/onlinepubs/009604499/utilities/set.html > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > --- > > > > > > > > Setting -u needs more careful review and test. > > > > > > This patch will break 'make deb-pkg'. > > > > > > ./scripts/package/mkdebian: 150: KDEB_PKGVERSION: parameter not set > > > > > > > > > > To set -u, scripts/package/mkdebian needs code refactoring. > > > > > > > > I will keep scripts/package/mkdebian untouched. > > uh, I missed that during the review. Do you want to refactor mkdebian > in large scale, or is an explicit fallback definition possibly > acceptable for you? > > diff --git a/scripts/package/mkdebian b/scripts/package/mkdebian > index ecfeb34b99aa..7e3878197041 100755 > --- a/scripts/package/mkdebian > +++ b/scripts/package/mkdebian > @@ -7,5 +7,17 @@ > set -eu > > +# Optional user-specified environment variables > + > +# Set target Debian architecture (skip auto-detection) > +: "${KBUILD_DEBARCH:=}" > + > +# Set target Debian distribution (skipping auto-detection) > +: "${KDEB_CHANGELOG_DIST:=}" > + > +# Overwrite the automatically determined package version. > +: ${KDEB_PKGVERSION:=} > + > + > is_enabled() { > grep -q "^$1=y" include/config/auto.conf > } > It depends on the code. I would fix if [ -n "$KDEB_PKGVERSION" ]; then to if [ "${KDEB_PKGVERSION:+set}" = set ]; then or if [ "${KDEB_PKGVERSION:+set}" ]; then -- Best Regards Masahiro Yamada
On Mon, Jun 17, 2024 at 01:52:23AM +0900 Masahiro Yamada wrote: > On Mon, Jun 17, 2024 at 12:56 AM Nicolas Schier <nicolas@fjasle.eu> wrote: > > > > On Mon, Jun 17, 2024 at 12:21:15AM +0900, Masahiro Yamada wrote: > > > On Wed, Jun 12, 2024 at 1:09 AM Masahiro Yamada <masahiroy@kernel.org> wrote: > > > > > > > > Set -e to make these scripts fail on the first error. > > > > > > > > Set -u because these scripts are invoked by Makefile, and do not work > > > > properly without necessary variables defined. > > > > > > > > Remove the explicit "test -n ..." from scripts/package/install-extmod-build. > > > > > > > > Both options are described in POSIX. [1] > > > > > > > > [1]: https://pubs.opengroup.org/onlinepubs/009604499/utilities/set.html > > > > > > > > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> > > > > --- > > > > > > > > > > > > Setting -u needs more careful review and test. > > > > > > > > > This patch will break 'make deb-pkg'. > > > > > > > > > ./scripts/package/mkdebian: 150: KDEB_PKGVERSION: parameter not set > > > > > > > > > > > > > > > To set -u, scripts/package/mkdebian needs code refactoring. > > > > > > > > > > > > I will keep scripts/package/mkdebian untouched. > > > > uh, I missed that during the review. Do you want to refactor mkdebian > > in large scale, or is an explicit fallback definition possibly > > acceptable for you? > > > > diff --git a/scripts/package/mkdebian b/scripts/package/mkdebian > > index ecfeb34b99aa..7e3878197041 100755 > > --- a/scripts/package/mkdebian > > +++ b/scripts/package/mkdebian > > @@ -7,5 +7,17 @@ > > set -eu > > > > +# Optional user-specified environment variables > > + > > +# Set target Debian architecture (skip auto-detection) > > +: "${KBUILD_DEBARCH:=}" > > + > > +# Set target Debian distribution (skipping auto-detection) > > +: "${KDEB_CHANGELOG_DIST:=}" > > + > > +# Overwrite the automatically determined package version. > > +: ${KDEB_PKGVERSION:=} > > + > > + > > is_enabled() { > > grep -q "^$1=y" include/config/auto.conf > > } > > > > > > > It depends on the code. > > > > > I would fix > > > if [ -n "$KDEB_PKGVERSION" ]; then > > > to > > if [ "${KDEB_PKGVERSION:+set}" = set ]; then > > or > if [ "${KDEB_PKGVERSION:+set}" ]; then ah, yes that's probaly better than the default value assigments, Please ignore my patch. Kind regards, Nicolas
diff --git a/scripts/package/builddeb b/scripts/package/builddeb index e797ad360f7a..c1757db6aa8a 100755 --- a/scripts/package/builddeb +++ b/scripts/package/builddeb @@ -10,7 +10,7 @@ # specified in KDEB_HOOKDIR) that will be called on package install and # removal. -set -e +set -eu is_enabled() { grep -q "^$1=y" include/config/auto.conf diff --git a/scripts/package/buildtar b/scripts/package/buildtar index eb67787f8673..cc87a473c01f 100755 --- a/scripts/package/buildtar +++ b/scripts/package/buildtar @@ -11,7 +11,7 @@ # Wichert Akkerman <wichert@wiggy.net>. # -set -e +set -eu # # Some variables and settings used throughout the script diff --git a/scripts/package/gen-diff-patch b/scripts/package/gen-diff-patch index 8a98b7bb78a0..f272f7770ea3 100755 --- a/scripts/package/gen-diff-patch +++ b/scripts/package/gen-diff-patch @@ -1,6 +1,8 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0-only +set -eu + diff_patch=$1 mkdir -p "$(dirname "${diff_patch}")" diff --git a/scripts/package/install-extmod-build b/scripts/package/install-extmod-build index 76e0765dfcd6..8cc9e13403ae 100755 --- a/scripts/package/install-extmod-build +++ b/scripts/package/install-extmod-build @@ -1,13 +1,10 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0-only -set -e +set -eu destdir=${1} -test -n "${srctree}" -test -n "${SRCARCH}" - is_enabled() { grep -q "^$1=y" include/config/auto.conf } diff --git a/scripts/package/mkdebian b/scripts/package/mkdebian index 070149c985fe..ecfeb34b99aa 100755 --- a/scripts/package/mkdebian +++ b/scripts/package/mkdebian @@ -4,7 +4,7 @@ # # Simple script to generate a debian/ directory for a Linux kernel. -set -e +set -eu is_enabled() { grep -q "^$1=y" include/config/auto.conf diff --git a/scripts/package/mkspec b/scripts/package/mkspec index ce201bfa8377..cc046ac74bf9 100755 --- a/scripts/package/mkspec +++ b/scripts/package/mkspec @@ -9,6 +9,8 @@ # Patched for non-x86 by Opencon (L) 2002 <opencon@rio.skydome.net> # +set -eu + output=$1 mkdir -p "$(dirname "${output}")"
Set -e to make these scripts fail on the first error. Set -u because these scripts are invoked by Makefile, and do not work properly without necessary variables defined. Remove the explicit "test -n ..." from scripts/package/install-extmod-build. Both options are described in POSIX. [1] [1]: https://pubs.opengroup.org/onlinepubs/009604499/utilities/set.html Signed-off-by: Masahiro Yamada <masahiroy@kernel.org> --- scripts/package/builddeb | 2 +- scripts/package/buildtar | 2 +- scripts/package/gen-diff-patch | 2 ++ scripts/package/install-extmod-build | 5 +---- scripts/package/mkdebian | 2 +- scripts/package/mkspec | 2 ++ 6 files changed, 8 insertions(+), 7 deletions(-)