[Buildroot] [PATCH 0/1] Fix Linux Version Probed

[Buildroot] [PATCH 0/1] Fix Linux Version Probed

[Giulio Benetti]

While trying to fix a bug on rtl8723ds I've found that
LINUX_VERSION_PROBED doesn't contain the x.y.z Linux version but
it contains instead the command to obtain it. I've found 2 errors
and 2 fix for this with the patch you find.
I've checked the LINUX_VERSION_PROBED even with make 4.1 and it was
wrong as well as make 4.3 and 4.3.91.

Let me know if someone else can give a try to this patch. It's mainly
used by drivers and by linux.mk to strip build/ and source/ folders from
target/ and I think that that part is failing.

Thank you
Giulio

Giulio Benetti (1):
  linux: fix LINUX_VERSION_PROBED content

 linux/linux.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.34.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH 1/1] linux: fix LINUX_VERSION_PROBED content

[Giulio Benetti]

Actually LINUX_VERSION_PROBED is set with the string sorrounded by the
backquotes including them. So when we evaluate $(LINUX_VERSION_PROBED) we
end up having the command itself and instead we only need the Linux version
probed in the style x.y.z.

Verifying it turns out that:
1) --no-print-directory and -s make flags must be passed before -C flag,
The error was not visible because errors were suppressed with
'2>/dev/null'
2) backquotes don't evaluate the expression they sorround but become part
of the string they sorround and get assigned to the variable

To fix this let's move --no-print-directory right after $(MAKE) and
use $(shell ) instead of backquotes.

Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
---
 linux/linux.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux/linux.mk b/linux/linux.mk
index 7645b5f507..23c169995a 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -183,7 +183,7 @@ endif
 # Get the real Linux version, which tells us where kernel modules are
 # going to be installed in the target filesystem.
 # Filter out 'w' from MAKEFLAGS, to workaround a bug in make 4.1 (#13141)
-LINUX_VERSION_PROBED = `MAKEFLAGS='$(filter-out w,$(MAKEFLAGS))' $(MAKE) $(LINUX_MAKE_FLAGS) -C $(LINUX_DIR) --no-print-directory -s kernelrelease 2>/dev/null`
+LINUX_VERSION_PROBED = $(shell MAKEFLAGS='$(filter-out w,$(MAKEFLAGS))' $(MAKE) --no-print-directory -s $(LINUX_MAKE_FLAGS) -C $(LINUX_DIR) kernelrelease 2>/dev/null)
 
 LINUX_DTS_NAME += $(call qstrip,$(BR2_LINUX_KERNEL_INTREE_DTS_NAME))
 
-- 
2.34.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [External] - [PATCH 1/1] linux: fix LINUX_VERSION_PROBED content

[Vincent Fazio]

> -----Original Message-----
> From: buildroot <buildroot-bounces@buildroot.org> On Behalf Of Giulio
> Benetti
> Sent: Tuesday, January 3, 2023 8:06 PM
> To: buildroot@buildroot.org
> Cc: Giulio Benetti <giulio.benetti@benettiengineering.com>
> Subject: [External] - [Buildroot] [PATCH 1/1] linux: fix
> LINUX_VERSION_PROBED content
> 
> Actually LINUX_VERSION_PROBED is set with the string sorrounded by the
> backquotes including them. So when we evaluate
> $(LINUX_VERSION_PROBED) we end up having the command itself and
> instead we only need the Linux version probed in the style x.y.z.
> 
> Verifying it turns out that:
> 1) --no-print-directory and -s make flags must be passed before -C flag, The
> error was not visible because errors were suppressed with '2>/dev/null'
> 2) backquotes don't evaluate the expression they sorround but become part
> of the string they sorround and get assigned to the variable
> 
> To fix this let's move --no-print-directory right after $(MAKE) and use $(shell )
> instead of backquotes.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>

Tested-by: Vincent Fazio <vfazio@xes-inc.com>

> ---
>  linux/linux.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux/linux.mk b/linux/linux.mk index 7645b5f507..23c169995a
> 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -183,7 +183,7 @@ endif
>  # Get the real Linux version, which tells us where kernel modules are  #
> going to be installed in the target filesystem.
>  # Filter out 'w' from MAKEFLAGS, to workaround a bug in make 4.1 (#13141)
> -LINUX_VERSION_PROBED = `MAKEFLAGS='$(filter-out w,$(MAKEFLAGS))'
> $(MAKE) $(LINUX_MAKE_FLAGS) -C $(LINUX_DIR) --no-print-directory -s
> kernelrelease 2>/dev/null`
> +LINUX_VERSION_PROBED = $(shell MAKEFLAGS='$(filter-out
> w,$(MAKEFLAGS))'
> +$(MAKE) --no-print-directory -s $(LINUX_MAKE_FLAGS) -C $(LINUX_DIR)
> +kernelrelease 2>/dev/null)
> 
>  LINUX_DTS_NAME += $(call
> qstrip,$(BR2_LINUX_KERNEL_INTREE_DTS_NAME))
> 
> --
> 2.34.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
> CAUTION: This email originated from outside of the organization. Do not click
> links or open attachments unless you recognize the sender and know the
> content is safe.

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] linux: fix LINUX_VERSION_PROBED content

[Thomas Petazzoni via buildroot]

Hello Giulio,

On Wed,  4 Jan 2023 03:05:58 +0100
Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:

> Actually LINUX_VERSION_PROBED is set with the string sorrounded by the
> backquotes including them. So when we evaluate $(LINUX_VERSION_PROBED) we
> end up having the command itself and instead we only need the Linux version
> probed in the style x.y.z.
> 
> Verifying it turns out that:
> 1) --no-print-directory and -s make flags must be passed before -C flag,
> The error was not visible because errors were suppressed with
> '2>/dev/null'  
> 2) backquotes don't evaluate the expression they sorround but become part
> of the string they sorround and get assigned to the variable
> 
> To fix this let's move --no-print-directory right after $(MAKE) and
> use $(shell ) instead of backquotes.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>

What is this fixing exactly? Do you have a specific problem that this
fixes? Or is this just fixing the fact that LINUX_VERSION_PROBED
contains `a very long command` instead of the result of the command
itself?

Indeed, switching back to $(shell ...) like you're proposing would
result in basically backporting:

commit 28f57bb863ba48df054cec0057c844bf23d5969f
Author: Arnout Vandecappelle <arnout@mind.be>
Date:   Sun Jul 12 16:35:27 2015 +0200

    linux: use backtick instead of $(shell ...) make function
    
    Verified that LINUX_VERSION_PROBED is only used in "-quoted commands
    (actually, usually it's not quoted).
    
    Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
    Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

which is not desirable, especially if you don't have a good
justification of why the current situation doesn't work.

So for now, I'll mark as Rejected, but if you have a good
justification, you can resend with a better commit log with this
justification.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] linux: fix LINUX_VERSION_PROBED content

[Giulio Benetti]

Hello Thomas, Vincent,

On 07/02/23 15:16, Thomas Petazzoni via buildroot wrote:
> Hello Giulio,
> 
> On Wed,  4 Jan 2023 03:05:58 +0100
> Giulio Benetti <giulio.benetti@benettiengineering.com> wrote:
> 
>> Actually LINUX_VERSION_PROBED is set with the string sorrounded by the
>> backquotes including them. So when we evaluate $(LINUX_VERSION_PROBED) we
>> end up having the command itself and instead we only need the Linux version
>> probed in the style x.y.z.
>>
>> Verifying it turns out that:
>> 1) --no-print-directory and -s make flags must be passed before -C flag,
>> The error was not visible because errors were suppressed with
>> '2>/dev/null'
>> 2) backquotes don't evaluate the expression they sorround but become part
>> of the string they sorround and get assigned to the variable
>>
>> To fix this let's move --no-print-directory right after $(MAKE) and
>> use $(shell ) instead of backquotes.
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>
> 
> What is this fixing exactly? Do you have a specific problem that this
> fixes? Or is this just fixing the fact that LINUX_VERSION_PROBED
> contains `a very long command` instead of the result of the command
> itself?

While building the various rtl* wifi drivers I've noticed KVER='a very 
long command` but it gets evaluated correctly, indeed the .ko module is
installed in the correct "lib/modules/$(LINUX_VERSION_PROBED)" path.

> Indeed, switching back to $(shell ...) like you're proposing would
> result in basically backporting:
> 
> commit 28f57bb863ba48df054cec0057c844bf23d5969f
> Author: Arnout Vandecappelle <arnout@mind.be>
> Date:   Sun Jul 12 16:35:27 2015 +0200
> 
>      linux: use backtick instead of $(shell ...) make function
>      
>      Verified that LINUX_VERSION_PROBED is only used in "-quoted commands
>      (actually, usually it's not quoted).
>      
>      Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>      Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> which is not desirable, especially if you don't have a good
> justification of why the current situation doesn't work.
> 
> So for now, I'll mark as Rejected, but if you have a good
> justification, you can resend with a better commit log with this
> justification.

Let's keep it as rejected, sorry for the noise!

Best regards
-- 
Giulio Benetti
Benetti Engineering sas
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot