Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] toolchain/toolchain-external: fix call to check_kernel_headers_version
@ 2020-03-21 13:52 Thomas Petazzoni
  2020-03-21 13:52 ` [Buildroot] [PATCH] support/scripts/check-kernel-headers.sh: do not print error for loose checks Thomas Petazzoni
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2020-03-21 13:52 UTC (permalink / raw)
  To: buildroot

The external toolchain configure step calls the
check_kernel_headers_version make function to compare the kernel
headers version declared in the configuration with the actual kernel
headers of the toolchain.

This function takes 4 arguments, but due to a missing comma what
should be the first two arguments are both passed into the first
argument. Due to this, when check_kernel_headers_version does:

	if ! support/scripts/check-kernel-headers.sh $(1) $(2) $(3) \
		$(if $(BR2_TOOLCHAIN_HEADERS_LATEST),$(4),strict); \

Then:

  $(1) contains "$(BUILD_DIR) $$(call toolchain_find_sysroot,$$(TOOLCHAIN_EXTERNAL_CC))"
  $(2) contains "$$(call qstrip,$$(BR2_TOOLCHAIN_HEADERS_AT_LEAST))"
  $(3) contains "$$(if $$(BR2_TOOLCHAIN_EXTERNAL_CUSTOM),loose,strict))"

So from the point of view of check-kernel-headers.sh, it already has
four arguments, and therefore the additional argument passed by:

   $(if $(BR2_TOOLCHAIN_HEADERS_LATEST),$(4),strict); \

is ignored, defeating the $(BR2_TOOLCHAIN_HEADERS_LATEST) test.

The practical consequence is that a toolchain that has 5.4 kernel
headers but declared as using 5.3 kernel headers does not abort the
build, because the check is considered "loose" while it should be
"strict".

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 toolchain/toolchain-external/pkg-toolchain-external.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk
index b01082aadd..8667d7ddf6 100644
--- a/toolchain/toolchain-external/pkg-toolchain-external.mk
+++ b/toolchain/toolchain-external/pkg-toolchain-external.mk
@@ -540,7 +540,7 @@ define $(2)_CONFIGURE_CMDS
 	$$(Q)$$(call check_unusable_toolchain,$$(TOOLCHAIN_EXTERNAL_CC))
 	$$(Q)SYSROOT_DIR="$$(call toolchain_find_sysroot,$$(TOOLCHAIN_EXTERNAL_CC))" ; \
 	$$(call check_kernel_headers_version,\
-		$$(BUILD_DIR)\
+		$$(BUILD_DIR),\
 		$$(call toolchain_find_sysroot,$$(TOOLCHAIN_EXTERNAL_CC)),\
 		$$(call qstrip,$$(BR2_TOOLCHAIN_HEADERS_AT_LEAST)),\
 		$$(if $$(BR2_TOOLCHAIN_EXTERNAL_CUSTOM),loose,strict)); \
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] support/scripts/check-kernel-headers.sh: do not print error for loose checks
  2020-03-21 13:52 [Buildroot] [PATCH] toolchain/toolchain-external: fix call to check_kernel_headers_version Thomas Petazzoni
@ 2020-03-21 13:52 ` Thomas Petazzoni
  2020-03-21 14:48   ` Yann E. MORIN
  2020-04-06 15:19   ` Peter Korsgaard
  2020-03-21 14:47 ` [Buildroot] [PATCH] toolchain/toolchain-external: fix call to check_kernel_headers_version Yann E. MORIN
  2020-04-06 15:16 ` Peter Korsgaard
  2 siblings, 2 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2020-03-21 13:52 UTC (permalink / raw)
  To: buildroot

The C program inside check-kernel-headers.sh has two checking mode: a
strict and a loose one.

In strict mode, we want the kernel headers version declared by the
user to match exactly the one of the toolchain.

In loose mode, we want the kernel headers version of the toolchain to
be greater than or equal to the one declared by the user: this is used
when we have a toolchain that has newer headers than the latest
version known by Buildroot.

However, in loose mode, we continue to show the "Incorrect kernel
headers version" message, even though we then return a zero error
code. This is very confusing: you see an error displayed on the
terminal, but the build goes on.

We fix that by first doing the loose check first, and returning 0 if
it succeeds. And then we move on with the strict check where we want
the version to be identical.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
 support/scripts/check-kernel-headers.sh | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/support/scripts/check-kernel-headers.sh b/support/scripts/check-kernel-headers.sh
index 841df98b64..4e6dce5487 100755
--- a/support/scripts/check-kernel-headers.sh
+++ b/support/scripts/check-kernel-headers.sh
@@ -49,18 +49,20 @@ ${HOSTCC} -imacros "${SYSROOT}/usr/include/linux/version.h" \
 int main(int argc __attribute__((unused)),
          char** argv __attribute__((unused)))
 {
-    int ret = 0;
     int l = LINUX_VERSION_CODE & ~0xFF;
     int h = KERNEL_VERSION(${HDR_M},${HDR_m},0);
 
+    if ((l >= h) && !strcmp("${CHECK}", "loose"))
+        return 0;
+
     if (l != h) {
         printf("Incorrect selection of kernel headers: ");
         printf("expected %d.%d.x, got %d.%d.x\n", ${HDR_M}, ${HDR_m},
                ((LINUX_VERSION_CODE>>16) & 0xFF),
                ((LINUX_VERSION_CODE>>8) & 0xFF));
-        ret = ((l >= h) && !strcmp("${CHECK}", "loose")) ? 0 : 1;
+        return 1;
     }
-    return ret;
+    return 0;
 }
 _EOF_
 
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] toolchain/toolchain-external: fix call to check_kernel_headers_version
  2020-03-21 13:52 [Buildroot] [PATCH] toolchain/toolchain-external: fix call to check_kernel_headers_version Thomas Petazzoni
  2020-03-21 13:52 ` [Buildroot] [PATCH] support/scripts/check-kernel-headers.sh: do not print error for loose checks Thomas Petazzoni
@ 2020-03-21 14:47 ` Yann E. MORIN
  2020-04-06 15:16 ` Peter Korsgaard
  2 siblings, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2020-03-21 14:47 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2020-03-21 14:52 +0100, Thomas Petazzoni spake thusly:
> The external toolchain configure step calls the
> check_kernel_headers_version make function to compare the kernel
> headers version declared in the configuration with the actual kernel
> headers of the toolchain.
> 
> This function takes 4 arguments, but due to a missing comma what
> should be the first two arguments are both passed into the first
> argument. Due to this, when check_kernel_headers_version does:
> 
> 	if ! support/scripts/check-kernel-headers.sh $(1) $(2) $(3) \
> 		$(if $(BR2_TOOLCHAIN_HEADERS_LATEST),$(4),strict); \
> 
> Then:
> 
>   $(1) contains "$(BUILD_DIR) $$(call toolchain_find_sysroot,$$(TOOLCHAIN_EXTERNAL_CC))"
>   $(2) contains "$$(call qstrip,$$(BR2_TOOLCHAIN_HEADERS_AT_LEAST))"
>   $(3) contains "$$(if $$(BR2_TOOLCHAIN_EXTERNAL_CUSTOM),loose,strict))"
> 
> So from the point of view of check-kernel-headers.sh, it already has
> four arguments, and therefore the additional argument passed by:
> 
>    $(if $(BR2_TOOLCHAIN_HEADERS_LATEST),$(4),strict); \
> 
> is ignored, defeating the $(BR2_TOOLCHAIN_HEADERS_LATEST) test.
> 
> The practical consequence is that a toolchain that has 5.4 kernel
> headers but declared as using 5.3 kernel headers does not abort the
> build, because the check is considered "loose" while it should be
> "strict".
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Applied to master, thanks.

Regards,
Yann E. MORIN.

> ---
>  toolchain/toolchain-external/pkg-toolchain-external.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/toolchain/toolchain-external/pkg-toolchain-external.mk b/toolchain/toolchain-external/pkg-toolchain-external.mk
> index b01082aadd..8667d7ddf6 100644
> --- a/toolchain/toolchain-external/pkg-toolchain-external.mk
> +++ b/toolchain/toolchain-external/pkg-toolchain-external.mk
> @@ -540,7 +540,7 @@ define $(2)_CONFIGURE_CMDS
>  	$$(Q)$$(call check_unusable_toolchain,$$(TOOLCHAIN_EXTERNAL_CC))
>  	$$(Q)SYSROOT_DIR="$$(call toolchain_find_sysroot,$$(TOOLCHAIN_EXTERNAL_CC))" ; \
>  	$$(call check_kernel_headers_version,\
> -		$$(BUILD_DIR)\
> +		$$(BUILD_DIR),\
>  		$$(call toolchain_find_sysroot,$$(TOOLCHAIN_EXTERNAL_CC)),\
>  		$$(call qstrip,$$(BR2_TOOLCHAIN_HEADERS_AT_LEAST)),\
>  		$$(if $$(BR2_TOOLCHAIN_EXTERNAL_CUSTOM),loose,strict)); \
> -- 
> 2.25.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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] support/scripts/check-kernel-headers.sh: do not print error for loose checks
  2020-03-21 13:52 ` [Buildroot] [PATCH] support/scripts/check-kernel-headers.sh: do not print error for loose checks Thomas Petazzoni
@ 2020-03-21 14:48   ` Yann E. MORIN
  2020-04-06 15:19   ` Peter Korsgaard
  1 sibling, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2020-03-21 14:48 UTC (permalink / raw)
  To: buildroot

Thomas, All,

On 2020-03-21 14:52 +0100, Thomas Petazzoni spake thusly:
> The C program inside check-kernel-headers.sh has two checking mode: a
> strict and a loose one.
> 
> In strict mode, we want the kernel headers version declared by the
> user to match exactly the one of the toolchain.
> 
> In loose mode, we want the kernel headers version of the toolchain to
> be greater than or equal to the one declared by the user: this is used
> when we have a toolchain that has newer headers than the latest
> version known by Buildroot.
> 
> However, in loose mode, we continue to show the "Incorrect kernel
> headers version" message, even though we then return a zero error
> code. This is very confusing: you see an error displayed on the
> terminal, but the build goes on.
> 
> We fix that by first doing the loose check first, and returning 0 if
> it succeeds. And then we move on with the strict check where we want
> the version to be identical.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

WhenVincent and I worked on this topic, I thought it would be better to
have the information about the version mismatch.

But I agree that the wording is confusing at best, so I applied to
master, thanks.

Regards,
Yann E. MORIN.

> ---
>  support/scripts/check-kernel-headers.sh | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/support/scripts/check-kernel-headers.sh b/support/scripts/check-kernel-headers.sh
> index 841df98b64..4e6dce5487 100755
> --- a/support/scripts/check-kernel-headers.sh
> +++ b/support/scripts/check-kernel-headers.sh
> @@ -49,18 +49,20 @@ ${HOSTCC} -imacros "${SYSROOT}/usr/include/linux/version.h" \
>  int main(int argc __attribute__((unused)),
>           char** argv __attribute__((unused)))
>  {
> -    int ret = 0;
>      int l = LINUX_VERSION_CODE & ~0xFF;
>      int h = KERNEL_VERSION(${HDR_M},${HDR_m},0);
>  
> +    if ((l >= h) && !strcmp("${CHECK}", "loose"))
> +        return 0;
> +
>      if (l != h) {
>          printf("Incorrect selection of kernel headers: ");
>          printf("expected %d.%d.x, got %d.%d.x\n", ${HDR_M}, ${HDR_m},
>                 ((LINUX_VERSION_CODE>>16) & 0xFF),
>                 ((LINUX_VERSION_CODE>>8) & 0xFF));
> -        ret = ((l >= h) && !strcmp("${CHECK}", "loose")) ? 0 : 1;
> +        return 1;
>      }
> -    return ret;
> +    return 0;
>  }
>  _EOF_
>  
> -- 
> 2.25.1
> 

-- 
.-----------------.--------------------.------------------.--------------------.
|  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/toolchain-external: fix call to check_kernel_headers_version
  2020-03-21 13:52 [Buildroot] [PATCH] toolchain/toolchain-external: fix call to check_kernel_headers_version Thomas Petazzoni
  2020-03-21 13:52 ` [Buildroot] [PATCH] support/scripts/check-kernel-headers.sh: do not print error for loose checks Thomas Petazzoni
  2020-03-21 14:47 ` [Buildroot] [PATCH] toolchain/toolchain-external: fix call to check_kernel_headers_version Yann E. MORIN
@ 2020-04-06 15:16 ` Peter Korsgaard
  2 siblings, 0 replies; 6+ messages in thread
From: Peter Korsgaard @ 2020-04-06 15:16 UTC (permalink / raw)
  To: buildroot

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

 > The external toolchain configure step calls the
 > check_kernel_headers_version make function to compare the kernel
 > headers version declared in the configuration with the actual kernel
 > headers of the toolchain.

 > This function takes 4 arguments, but due to a missing comma what
 > should be the first two arguments are both passed into the first
 > argument. Due to this, when check_kernel_headers_version does:

 > 	if ! support/scripts/check-kernel-headers.sh $(1) $(2) $(3) \
 > 		$(if $(BR2_TOOLCHAIN_HEADERS_LATEST),$(4),strict); \

 > Then:

 >   $(1) contains "$(BUILD_DIR) $$(call toolchain_find_sysroot,$$(TOOLCHAIN_EXTERNAL_CC))"
 >   $(2) contains "$$(call qstrip,$$(BR2_TOOLCHAIN_HEADERS_AT_LEAST))"
 >   $(3) contains "$$(if $$(BR2_TOOLCHAIN_EXTERNAL_CUSTOM),loose,strict))"

 > So from the point of view of check-kernel-headers.sh, it already has
 > four arguments, and therefore the additional argument passed by:

 >    $(if $(BR2_TOOLCHAIN_HEADERS_LATEST),$(4),strict); \

 > is ignored, defeating the $(BR2_TOOLCHAIN_HEADERS_LATEST) test.

 > The practical consequence is that a toolchain that has 5.4 kernel
 > headers but declared as using 5.3 kernel headers does not abort the
 > build, because the check is considered "loose" while it should be
 > "strict".

 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Committed to 2019.02.x, 2019.11.x and 2020.02.x, thanks (the first two
do not support the strict/loose match though)

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Buildroot] [PATCH] support/scripts/check-kernel-headers.sh: do not print error for loose checks
  2020-03-21 13:52 ` [Buildroot] [PATCH] support/scripts/check-kernel-headers.sh: do not print error for loose checks Thomas Petazzoni
  2020-03-21 14:48   ` Yann E. MORIN
@ 2020-04-06 15:19   ` Peter Korsgaard
  1 sibling, 0 replies; 6+ messages in thread
From: Peter Korsgaard @ 2020-04-06 15:19 UTC (permalink / raw)
  To: buildroot

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@bootlin.com> writes:

 > The C program inside check-kernel-headers.sh has two checking mode: a
 > strict and a loose one.

 > In strict mode, we want the kernel headers version declared by the
 > user to match exactly the one of the toolchain.

 > In loose mode, we want the kernel headers version of the toolchain to
 > be greater than or equal to the one declared by the user: this is used
 > when we have a toolchain that has newer headers than the latest
 > version known by Buildroot.

 > However, in loose mode, we continue to show the "Incorrect kernel
 > headers version" message, even though we then return a zero error
 > code. This is very confusing: you see an error displayed on the
 > terminal, but the build goes on.

 > We fix that by first doing the loose check first, and returning 0 if
 > it succeeds. And then we move on with the strict check where we want
 > the version to be identical.

 > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>

Committed to 2020.02.x, thanks.

-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-04-06 15:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-21 13:52 [Buildroot] [PATCH] toolchain/toolchain-external: fix call to check_kernel_headers_version Thomas Petazzoni
2020-03-21 13:52 ` [Buildroot] [PATCH] support/scripts/check-kernel-headers.sh: do not print error for loose checks Thomas Petazzoni
2020-03-21 14:48   ` Yann E. MORIN
2020-04-06 15:19   ` Peter Korsgaard
2020-03-21 14:47 ` [Buildroot] [PATCH] toolchain/toolchain-external: fix call to check_kernel_headers_version Yann E. MORIN
2020-04-06 15:16 ` Peter Korsgaard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox