Message ID | 20230117-kernel-kheaders-cpio-v1-1-058d3e1c8621@weissschuh.net (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | kheaders: explicitly validate existence of cpio command | expand |
On Tue, Jan 17, 2023 at 05:30:43AM +0000, Thomas Weißschuh wrote: > If the cpio command is not available the error emitted by > gen_kheaders.so is not clear as all output of the call to cpio is > discarded: > > GNU make 4.4: > > GEN kernel/kheaders_data.tar.xz > find: 'standard output': Broken pipe > find: write error > make[2]: *** [kernel/Makefile:157: kernel/kheaders_data.tar.xz] Error 127 > make[1]: *** [scripts/Makefile.build:504: kernel] Error 2 > > GNU make < 4.4: > > GEN kernel/kheaders_data.tar.xz > make[2]: *** [kernel/Makefile:157: kernel/kheaders_data.tar.xz] Error 127 > make[2]: *** Waiting for unfinished jobs.... > make[1]: *** [scripts/Makefile.build:504: kernel] Error 2 > > Add an explicit check that will trigger a clear message about the issue: > > CHK kernel/kheaders_data.tar.xz > ./kernel/gen_kheaders.sh: line 17: type: cpio: not found Thanks for the patch! What would you think about an even more verbose message? Perhaps something like: echo >&2 ' *** "cpio" is required for "CONFIG_IKHEADERS". >&2 Reviewed-by: Nicolas Schier <n.schier@avm.de> Kind regards, Nicolas
On Tue, Jan 17, 2023 at 09:46:02AM +0100, Nicolas Schier wrote: > On Tue, Jan 17, 2023 at 05:30:43AM +0000, Thomas Weißschuh wrote: > > If the cpio command is not available the error emitted by > > gen_kheaders.so is not clear as all output of the call to cpio is > > discarded: > > > > GNU make 4.4: > > > > GEN kernel/kheaders_data.tar.xz > > find: 'standard output': Broken pipe > > find: write error > > make[2]: *** [kernel/Makefile:157: kernel/kheaders_data.tar.xz] Error 127 > > make[1]: *** [scripts/Makefile.build:504: kernel] Error 2 > > > > GNU make < 4.4: > > > > GEN kernel/kheaders_data.tar.xz > > make[2]: *** [kernel/Makefile:157: kernel/kheaders_data.tar.xz] Error 127 > > make[2]: *** Waiting for unfinished jobs.... > > make[1]: *** [scripts/Makefile.build:504: kernel] Error 2 > > > > Add an explicit check that will trigger a clear message about the issue: > > > > CHK kernel/kheaders_data.tar.xz > > ./kernel/gen_kheaders.sh: line 17: type: cpio: not found > > Thanks for the patch! What would you think about an even more verbose > message? Perhaps something like: > > echo >&2 ' *** "cpio" is required for "CONFIG_IKHEADERS". >&2 Wouldn't hurt obviously. The other scripts used by the kernel don't seem to try to provide this kind of explicit message either, though. Having a line number, some sort of "not found" and the name of the command are enough, in my opinion. > Reviewed-by: Nicolas Schier <n.schier@avm.de> Thanks! Thomas
On Tue, Jan 17, 2023 at 10:19 PM Thomas Weißschuh <thomas@t-8ch.de> wrote: > > On Tue, Jan 17, 2023 at 09:46:02AM +0100, Nicolas Schier wrote: > > On Tue, Jan 17, 2023 at 05:30:43AM +0000, Thomas Weißschuh wrote: > > > If the cpio command is not available the error emitted by > > > gen_kheaders.so is not clear as all output of the call to cpio is > > > discarded: > > > > > > GNU make 4.4: > > > > > > GEN kernel/kheaders_data.tar.xz > > > find: 'standard output': Broken pipe > > > find: write error > > > make[2]: *** [kernel/Makefile:157: kernel/kheaders_data.tar.xz] Error 127 > > > make[1]: *** [scripts/Makefile.build:504: kernel] Error 2 > > > > > > GNU make < 4.4: > > > > > > GEN kernel/kheaders_data.tar.xz > > > make[2]: *** [kernel/Makefile:157: kernel/kheaders_data.tar.xz] Error 127 > > > make[2]: *** Waiting for unfinished jobs.... > > > make[1]: *** [scripts/Makefile.build:504: kernel] Error 2 > > > > > > Add an explicit check that will trigger a clear message about the issue: > > > > > > CHK kernel/kheaders_data.tar.xz > > > ./kernel/gen_kheaders.sh: line 17: type: cpio: not found > > > > Thanks for the patch! What would you think about an even more verbose > > message? Perhaps something like: > > > > echo >&2 ' *** "cpio" is required for "CONFIG_IKHEADERS". >&2 > > Wouldn't hurt obviously. > The other scripts used by the kernel don't seem to try to provide this > kind of explicit message either, though. > Having a line number, some sort of "not found" and the name of the > command are enough, in my opinion. Agree. I am fine with this patch. Just a side note. 'type' is defined in posix [1], so this code is portable. Kbuild often uses "command -v" to check the existence of a command because it is also posix compliant [2]. I just wondered why we did not use 'type' before... 'type' is more efficient because it is shell-builtin (at least for dash and bash) but 'command' is not. [1]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/type.html [2]: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html > > > Reviewed-by: Nicolas Schier <n.schier@avm.de> > > Thanks! > > Thomas
On Tue, Jan 17, 2023 at 10:19 PM Thomas Weißschuh <thomas@t-8ch.de> wrote: > > On Tue, Jan 17, 2023 at 09:46:02AM +0100, Nicolas Schier wrote: > > On Tue, Jan 17, 2023 at 05:30:43AM +0000, Thomas Weißschuh wrote: > > > If the cpio command is not available the error emitted by > > > gen_kheaders.so is not clear as all output of the call to cpio is > > > discarded: > > > > > > GNU make 4.4: > > > > > > GEN kernel/kheaders_data.tar.xz > > > find: 'standard output': Broken pipe > > > find: write error > > > make[2]: *** [kernel/Makefile:157: kernel/kheaders_data.tar.xz] Error 127 > > > make[1]: *** [scripts/Makefile.build:504: kernel] Error 2 > > > > > > GNU make < 4.4: > > > > > > GEN kernel/kheaders_data.tar.xz > > > make[2]: *** [kernel/Makefile:157: kernel/kheaders_data.tar.xz] Error 127 > > > make[2]: *** Waiting for unfinished jobs.... > > > make[1]: *** [scripts/Makefile.build:504: kernel] Error 2 > > > > > > Add an explicit check that will trigger a clear message about the issue: > > > > > > CHK kernel/kheaders_data.tar.xz > > > ./kernel/gen_kheaders.sh: line 17: type: cpio: not found > > > > Thanks for the patch! What would you think about an even more verbose > > message? Perhaps something like: > > > > echo >&2 ' *** "cpio" is required for "CONFIG_IKHEADERS". >&2 > > Wouldn't hurt obviously. > The other scripts used by the kernel don't seem to try to provide this > kind of explicit message either, though. > Having a line number, some sort of "not found" and the name of the > command are enough, in my opinion. > > > Reviewed-by: Nicolas Schier <n.schier@avm.de> > > Thanks! > > Thomas Applied to linux-kbuild. Thanks.
diff --git a/kernel/gen_kheaders.sh b/kernel/gen_kheaders.sh index 473036b43c83..81b97f0f6556 100755 --- a/kernel/gen_kheaders.sh +++ b/kernel/gen_kheaders.sh @@ -14,6 +14,8 @@ include/ arch/$SRCARCH/include/ " +type cpio > /dev/null + # Support incremental builds by skipping archive generation # if timestamps of files being archived are not changed.
If the cpio command is not available the error emitted by gen_kheaders.so is not clear as all output of the call to cpio is discarded: GNU make 4.4: GEN kernel/kheaders_data.tar.xz find: 'standard output': Broken pipe find: write error make[2]: *** [kernel/Makefile:157: kernel/kheaders_data.tar.xz] Error 127 make[1]: *** [scripts/Makefile.build:504: kernel] Error 2 GNU make < 4.4: GEN kernel/kheaders_data.tar.xz make[2]: *** [kernel/Makefile:157: kernel/kheaders_data.tar.xz] Error 127 make[2]: *** Waiting for unfinished jobs.... make[1]: *** [scripts/Makefile.build:504: kernel] Error 2 Add an explicit check that will trigger a clear message about the issue: CHK kernel/kheaders_data.tar.xz ./kernel/gen_kheaders.sh: line 17: type: cpio: not found The other commands executed by gen_kheaders.sh are part of a standard installation, so they are not checked. Reported-by: Amy Parker <apark0006@student.cerritos.edu> Link: https://lore.kernel.org/lkml/CAPOgqxFva=tOuh1UitCSN38+28q3BNXKq19rEsVNPRzRqKqZ+g@mail.gmail.com/ Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- kernel/gen_kheaders.sh | 2 ++ 1 file changed, 2 insertions(+) --- base-commit: d532dd102151cc69fcd00b13e5a9689b23c0c8d9 change-id: 20230117-kernel-kheaders-cpio-2f7c70bc9c19 Best regards,