* [Buildroot] [PATCH] toolchain/helpers: make sure we bail out when kernel headers check fails
@ 2019-11-12 20:21 Thomas Petazzoni
2019-11-12 21:21 ` Carlos Santos
2019-11-13 21:41 ` Yann E. MORIN
0 siblings, 2 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2019-11-12 20:21 UTC (permalink / raw)
To: buildroot
In commit 6136765b23abd9faba610dd54ed276a777811575 ("toolchain:
generate check-headers program under $(BUILD_DIR)"), the
check_kernel_headers_version function was simplified to not check the
return value of the check-kernel-headers.sh script, assuming that
"make" does bail out on the first failing command.
However, check_kernel_headers_version when used in $(2)_CONFIGURE_CMDS
from pkg-toolchain-external.mk, is called in a sequence of commands,
where the return value of each command is not checked. Therefore, a
failure of check-kernel-headers.sh no longer aborts the build.
Since all other macros are using this principle of calling "exit 1",
we revert back to the same for check_kernel_headers_version, as it was
done prior to 6136765b23abd9faba610dd54ed276a777811575.
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Carlos Santos <unixmania@gmail.com>
---
toolchain/helpers.mk | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
index 42e5522060..996cc70d44 100644
--- a/toolchain/helpers.mk
+++ b/toolchain/helpers.mk
@@ -163,7 +163,9 @@ copy_toolchain_sysroot = \
# $3: kernel version string, in the form: X.Y
#
check_kernel_headers_version = \
- support/scripts/check-kernel-headers.sh $(1) $(2) $(3)
+ if ! support/scripts/check-kernel-headers.sh $(1) $(2) $(3); then \
+ exit 1; \
+ fi
#
# Check the specific gcc version actually matches the version in the
--
2.23.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] toolchain/helpers: make sure we bail out when kernel headers check fails
2019-11-12 20:21 [Buildroot] [PATCH] toolchain/helpers: make sure we bail out when kernel headers check fails Thomas Petazzoni
@ 2019-11-12 21:21 ` Carlos Santos
2019-11-12 21:25 ` Thomas Petazzoni
2019-11-13 21:41 ` Yann E. MORIN
1 sibling, 1 reply; 6+ messages in thread
From: Carlos Santos @ 2019-11-12 21:21 UTC (permalink / raw)
To: buildroot
On Tue, Nov 12, 2019 at 5:21 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> In commit 6136765b23abd9faba610dd54ed276a777811575 ("toolchain:
> generate check-headers program under $(BUILD_DIR)"), the
> check_kernel_headers_version function was simplified to not check the
> return value of the check-kernel-headers.sh script, assuming that
> "make" does bail out on the first failing command.
>
> However, check_kernel_headers_version when used in $(2)_CONFIGURE_CMDS
> from pkg-toolchain-external.mk, is called in a sequence of commands,
> where the return value of each command is not checked. Therefore, a
> failure of check-kernel-headers.sh no longer aborts the build.
>
> Since all other macros are using this principle of calling "exit 1",
> we revert back to the same for check_kernel_headers_version, as it was
> done prior to 6136765b23abd9faba610dd54ed276a777811575.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Carlos Santos <unixmania@gmail.com>
> ---
> toolchain/helpers.mk | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
> index 42e5522060..996cc70d44 100644
> --- a/toolchain/helpers.mk
> +++ b/toolchain/helpers.mk
> @@ -163,7 +163,9 @@ copy_toolchain_sysroot = \
> # $3: kernel version string, in the form: X.Y
> #
> check_kernel_headers_version = \
> - support/scripts/check-kernel-headers.sh $(1) $(2) $(3)
> + if ! support/scripts/check-kernel-headers.sh $(1) $(2) $(3); then \
> + exit 1; \
> + fi
>
> #
> # Check the specific gcc version actually matches the version in the
> --
> 2.23.0
>
The function became a one-liner after commit 6136765b23. Wouldn't it
be simpler and more readable to run the script and test the result,
both in linux-headers.mk and pkg-toolchain-external.mk?
--
Carlos Santos <unixmania@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] toolchain/helpers: make sure we bail out when kernel headers check fails
2019-11-12 21:21 ` Carlos Santos
@ 2019-11-12 21:25 ` Thomas Petazzoni
2019-11-12 21:30 ` Carlos Santos
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Petazzoni @ 2019-11-12 21:25 UTC (permalink / raw)
To: buildroot
On Tue, 12 Nov 2019 18:21:15 -0300
Carlos Santos <unixmania@gmail.com> wrote:
> > diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
> > index 42e5522060..996cc70d44 100644
> > --- a/toolchain/helpers.mk
> > +++ b/toolchain/helpers.mk
> > @@ -163,7 +163,9 @@ copy_toolchain_sysroot = \
> > # $3: kernel version string, in the form: X.Y
> > #
> > check_kernel_headers_version = \
> > - support/scripts/check-kernel-headers.sh $(1) $(2) $(3)
> > + if ! support/scripts/check-kernel-headers.sh $(1) $(2) $(3); then \
> > + exit 1; \
> > + fi
> >
> > #
> > # Check the specific gcc version actually matches the version in the
> > --
> > 2.23.0
> >
>
> The function became a one-liner after commit 6136765b23. Wouldn't it
> be simpler and more readable to run the script and test the result,
> both in linux-headers.mk and pkg-toolchain-external.mk?
Yes, we could certainly do that. The aim of my patch was really just to
get back to where we were before, to fix the immediate issue. This of
course doesn't mean we can't improve things further.
I'll let Peter/Arnout/Yann decide what they want to do (i.e apply my
patch as-is, or have a more elaborate version).
Thanks!
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] toolchain/helpers: make sure we bail out when kernel headers check fails
2019-11-12 21:25 ` Thomas Petazzoni
@ 2019-11-12 21:30 ` Carlos Santos
0 siblings, 0 replies; 6+ messages in thread
From: Carlos Santos @ 2019-11-12 21:30 UTC (permalink / raw)
To: buildroot
On Tue, Nov 12, 2019 at 6:25 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Tue, 12 Nov 2019 18:21:15 -0300
> Carlos Santos <unixmania@gmail.com> wrote:
>
> > > diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
> > > index 42e5522060..996cc70d44 100644
> > > --- a/toolchain/helpers.mk
> > > +++ b/toolchain/helpers.mk
> > > @@ -163,7 +163,9 @@ copy_toolchain_sysroot = \
> > > # $3: kernel version string, in the form: X.Y
> > > #
> > > check_kernel_headers_version = \
> > > - support/scripts/check-kernel-headers.sh $(1) $(2) $(3)
> > > + if ! support/scripts/check-kernel-headers.sh $(1) $(2) $(3); then \
> > > + exit 1; \
> > > + fi
> > >
> > > #
> > > # Check the specific gcc version actually matches the version in the
> > > --
> > > 2.23.0
> > >
> >
> > The function became a one-liner after commit 6136765b23. Wouldn't it
> > be simpler and more readable to run the script and test the result,
> > both in linux-headers.mk and pkg-toolchain-external.mk?
>
> Yes, we could certainly do that. The aim of my patch was really just to
> get back to where we were before, to fix the immediate issue. This of
> course doesn't mean we can't improve things further.
>
> I'll let Peter/Arnout/Yann decide what they want to do (i.e apply my
> patch as-is, or have a more elaborate version).
>
> Thanks!
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Hum, perhaps we should keep the design pattern, considering that other
functions (e.g. check_gcc_version) are used the same way, so
Reviewed-by: Carlos Santos <unixmania@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] toolchain/helpers: make sure we bail out when kernel headers check fails
2019-11-12 20:21 [Buildroot] [PATCH] toolchain/helpers: make sure we bail out when kernel headers check fails Thomas Petazzoni
2019-11-12 21:21 ` Carlos Santos
@ 2019-11-13 21:41 ` Yann E. MORIN
2019-11-19 7:55 ` Peter Korsgaard
1 sibling, 1 reply; 6+ messages in thread
From: Yann E. MORIN @ 2019-11-13 21:41 UTC (permalink / raw)
To: buildroot
Thomas, All,
On 2019-11-12 21:21 +0100, Thomas Petazzoni spake thusly:
> In commit 6136765b23abd9faba610dd54ed276a777811575 ("toolchain:
> generate check-headers program under $(BUILD_DIR)"), the
> check_kernel_headers_version function was simplified to not check the
> return value of the check-kernel-headers.sh script, assuming that
> "make" does bail out on the first failing command.
>
> However, check_kernel_headers_version when used in $(2)_CONFIGURE_CMDS
> from pkg-toolchain-external.mk, is called in a sequence of commands,
> where the return value of each command is not checked. Therefore, a
> failure of check-kernel-headers.sh no longer aborts the build.
>
> Since all other macros are using this principle of calling "exit 1",
> we revert back to the same for check_kernel_headers_version, as it was
> done prior to 6136765b23abd9faba610dd54ed276a777811575.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Carlos Santos <unixmania@gmail.com>
Applied to master, thanks.
Regards,
Yann E. MORIN.
> ---
> toolchain/helpers.mk | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/toolchain/helpers.mk b/toolchain/helpers.mk
> index 42e5522060..996cc70d44 100644
> --- a/toolchain/helpers.mk
> +++ b/toolchain/helpers.mk
> @@ -163,7 +163,9 @@ copy_toolchain_sysroot = \
> # $3: kernel version string, in the form: X.Y
> #
> check_kernel_headers_version = \
> - support/scripts/check-kernel-headers.sh $(1) $(2) $(3)
> + if ! support/scripts/check-kernel-headers.sh $(1) $(2) $(3); then \
> + exit 1; \
> + fi
>
> #
> # Check the specific gcc version actually matches the version in the
> --
> 2.23.0
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] toolchain/helpers: make sure we bail out when kernel headers check fails
2019-11-13 21:41 ` Yann E. MORIN
@ 2019-11-19 7:55 ` Peter Korsgaard
0 siblings, 0 replies; 6+ messages in thread
From: Peter Korsgaard @ 2019-11-19 7:55 UTC (permalink / raw)
To: buildroot
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
Hi,
> On 2019-11-12 21:21 +0100, Thomas Petazzoni spake thusly:
>> In commit 6136765b23abd9faba610dd54ed276a777811575 ("toolchain:
>> generate check-headers program under $(BUILD_DIR)"), the
>> check_kernel_headers_version function was simplified to not check the
>> return value of the check-kernel-headers.sh script, assuming that
>> "make" does bail out on the first failing command.
>>
>> However, check_kernel_headers_version when used in $(2)_CONFIGURE_CMDS
>> from pkg-toolchain-external.mk, is called in a sequence of commands,
>> where the return value of each command is not checked. Therefore, a
>> failure of check-kernel-headers.sh no longer aborts the build.
>>
>> Since all other macros are using this principle of calling "exit 1",
>> we revert back to the same for check_kernel_headers_version, as it was
>> done prior to 6136765b23abd9faba610dd54ed276a777811575.
>>
>> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
>> Cc: Carlos Santos <unixmania@gmail.com>
> Applied to master, thanks.
Committed to 2019.02.x and 2019.08.x, thanks.
--
Bye, Peter Korsgaard
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-11-19 7:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-11-12 20:21 [Buildroot] [PATCH] toolchain/helpers: make sure we bail out when kernel headers check fails Thomas Petazzoni
2019-11-12 21:21 ` Carlos Santos
2019-11-12 21:25 ` Thomas Petazzoni
2019-11-12 21:30 ` Carlos Santos
2019-11-13 21:41 ` Yann E. MORIN
2019-11-19 7:55 ` Peter Korsgaard
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.