[PATCH RESEND] kernel-yocto: fix merge_config.sh flag set when KBUILD_DEFCONFIG is used

[PATCH RESEND] kernel-yocto: fix merge_config.sh flag set when KBUILD_DEFCONFIG is used

[Slawomir Stepien]

Based on the documentation of KCONFIG_MODE[1], when we use "in-tree"
defconfig (the KBUILD_DEFCONFIG is set), then the alldefconfig mode
should be used with merge_config.sh.

This commit fixes the logic behind setting the flag by checking if the
provided defconfig file exists in the linux source tree.

[1] https://docs.yoctoproject.org/ref-manual/variables.html#term-KCONFIG_MODE

Signed-off-by: Slawomir Stepien <sst@poczta.fm>
---
 meta/classes-recipe/kernel-yocto.bbclass | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/meta/classes-recipe/kernel-yocto.bbclass b/meta/classes-recipe/kernel-yocto.bbclass
index c45abf6ddc..ffda41ffc9 100644
--- a/meta/classes-recipe/kernel-yocto.bbclass
+++ b/meta/classes-recipe/kernel-yocto.bbclass
@@ -113,6 +113,14 @@ def get_dirs_with_fragments(d):
 
     return " ".join(extrafiles)
 
+kbuild_defconfig_exists() {
+	if [ -f "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}" ]; then
+		return 0
+	fi
+
+	return 1
+}
+
 do_kernel_metadata() {
 	set +e
 
@@ -155,7 +163,7 @@ do_kernel_metadata() {
 	# precendence.
 	#
 	if [ -n "${KBUILD_DEFCONFIG}" ]; then
-		if [ -f "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}" ]; then
+		if $(kbuild_defconfig_exists); then
 			if [ -f "${UNPACKDIR}/defconfig" ]; then
 				# If the two defconfig's are different, warn that we overwrote the
 				# one already placed in UNPACKDIR
@@ -478,8 +486,12 @@ do_kernel_configme() {
 			config_flags=""
 			;;
 		*)
-			if [ -f ${UNPACKDIR}/defconfig ]; then
-				config_flags="-n"
+			if $(kbuild_defconfig_exists); then
+				config_flags=""
+			else
+				if [ -f ${UNPACKDIR}/defconfig ]; then
+					config_flags="-n"
+				fi
 			fi
 			;;
 	esac
-- 
2.48.1

Re: [OE-core] [PATCH RESEND] kernel-yocto: fix merge_config.sh flag set when KBUILD_DEFCONFIG is used

[Jörg Sommer]

Slawomir Stepien via lists.openembedded.org schrieb am So 26. Jan, 13:43 (+0100):
> Based on the documentation of KCONFIG_MODE[1], when we use "in-tree"
> defconfig (the KBUILD_DEFCONFIG is set), then the alldefconfig mode
> should be used with merge_config.sh.
> 
> This commit fixes the logic behind setting the flag by checking if the
> provided defconfig file exists in the linux source tree.
> 
> [1] https://docs.yoctoproject.org/ref-manual/variables.html#term-KCONFIG_MODE
> 
> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
> ---
>  meta/classes-recipe/kernel-yocto.bbclass | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/meta/classes-recipe/kernel-yocto.bbclass b/meta/classes-recipe/kernel-yocto.bbclass
> index c45abf6ddc..ffda41ffc9 100644
> --- a/meta/classes-recipe/kernel-yocto.bbclass
> +++ b/meta/classes-recipe/kernel-yocto.bbclass
> @@ -113,6 +113,14 @@ def get_dirs_with_fragments(d):
>  
>      return " ".join(extrafiles)
>  
> +kbuild_defconfig_exists() {
> +	if [ -f "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}" ]; then
> +		return 0
> +	fi
> +
> +	return 1

Hi,

you don't need the branching logic here. The function always returns the
last return value. So

kbuild_defconfig_exists() {
    [ -f "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}" ]
}

is enough.

Is it really worth to have a single function for this command? Maybe, it's
more useful to define a variable S_KBUILD_DEFCONFIG_PATH and replace all
occurrences of `${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}` by this. But
even this might not help so much.

>  	if [ -n "${KBUILD_DEFCONFIG}" ]; then
> -		if [ -f "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}" ]; then
> +		if $(kbuild_defconfig_exists); then

I think you don't want the $(), because you don't want the output of the
command, but the return value. So `if kbuild_defconfig_exists; then`

>  			if [ -f "${UNPACKDIR}/defconfig" ]; then
>  				# If the two defconfig's are different, warn that we overwrote the
>  				# one already placed in UNPACKDIR
> @@ -478,8 +486,12 @@ do_kernel_configme() {
>  			config_flags=""
>  			;;
>  		*)
> -			if [ -f ${UNPACKDIR}/defconfig ]; then
> -				config_flags="-n"
> +			if $(kbuild_defconfig_exists); then

Is it save here to assume KBUILD_DEFCONFIG is set. This is checked in the
code fragment above.

> +				config_flags=""
> +			else
> +				if [ -f ${UNPACKDIR}/defconfig ]; then

You can use `elif [ …` to reduce the nesting.

> +					config_flags="-n"
> +				fi
>  			fi
>  			;;
>  	esac
> -- 
> 2.48.1
> 


Bye, Jörg

-- 
Manchmal denke ich, das sicherste Indiz dafür, daß anderswo im Universum
intelligentes Leben existiert, ist, daß niemand versucht hat, mit uns
Kontakt aufzunehmen.                           (Calvin und Hobbes)

Navimatix GmbH           T: 03641 - 327 99 0
Tatzendpromenade 2       F: 03641 - 526 306
07745 Jena               www.navimatix.de

Geschäftsführer: Steffen Späthe, Jan Rommeley
Registergericht: Amtsgericht Jena, HRB 501480

Re: [OE-core] [PATCH RESEND] kernel-yocto: fix merge_config.sh flag set when KBUILD_DEFCONFIG is used

[Slawomir Stepien]

On sty 26, 2025 16:51, Jörg Sommer wrote:
> Slawomir Stepien via lists.openembedded.org schrieb am So 26. Jan, 13:43 (+0100):
> > Based on the documentation of KCONFIG_MODE[1], when we use "in-tree"
> > defconfig (the KBUILD_DEFCONFIG is set), then the alldefconfig mode
> > should be used with merge_config.sh.
> > 
> > This commit fixes the logic behind setting the flag by checking if the
> > provided defconfig file exists in the linux source tree.
> > 
> > [1] https://docs.yoctoproject.org/ref-manual/variables.html#term-KCONFIG_MODE
> > 
> > Signed-off-by: Slawomir Stepien <sst@poczta.fm>
> > ---
> >  meta/classes-recipe/kernel-yocto.bbclass | 18 +++++++++++++++---
> >  1 file changed, 15 insertions(+), 3 deletions(-)
> > 
> > diff --git a/meta/classes-recipe/kernel-yocto.bbclass b/meta/classes-recipe/kernel-yocto.bbclass
> > index c45abf6ddc..ffda41ffc9 100644
> > --- a/meta/classes-recipe/kernel-yocto.bbclass
> > +++ b/meta/classes-recipe/kernel-yocto.bbclass
> > @@ -113,6 +113,14 @@ def get_dirs_with_fragments(d):
> >  
> >      return " ".join(extrafiles)
> >  
> > +kbuild_defconfig_exists() {
> > +	if [ -f "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}" ]; then
> > +		return 0
> > +	fi
> > +
> > +	return 1
> 
> Hi,

Hi Jörg,

Thanks for all these good comments. After Bruce's comment, I will not push further this approach.

> (...)

-- 
Slawomir Stepien

Re: [OE-core] [PATCH RESEND] kernel-yocto: fix merge_config.sh flag set when KBUILD_DEFCONFIG is used

[Bruce Ashfield]

[-- Attachment #1: Type: text/plain, Size: 4531 bytes --]

On Sun, Jan 26, 2025 at 8:20 AM Slawomir Stepien via lists.openembedded.org
<sst=poczta.fm@lists.openembedded.org> wrote:

> Based on the documentation of KCONFIG_MODE[1], when we use "in-tree"
> defconfig (the KBUILD_DEFCONFIG is set), then the alldefconfig mode
> should be used with merge_config.sh.
>
>
Please make sure to copy me directly on any kernel-yocto changes.



> This commit fixes the logic behind setting the flag by checking if the
> provided defconfig file exists in the linux source tree.


There's no sense referencing the documentation here, since it was
written long after the code and matches the logic in the class, not
the other way around. Meaning it would be the documentation that
is wrong, not the code if there's a mismatch.



> [1]
> https://docs.yoctoproject.org/ref-manual/variables.html#term-KCONFIG_MODE
>
> Signed-off-by: Slawomir Stepien <sst@poczta.fm>
> ---
>  meta/classes-recipe/kernel-yocto.bbclass | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/meta/classes-recipe/kernel-yocto.bbclass
> b/meta/classes-recipe/kernel-yocto.bbclass
> index c45abf6ddc..ffda41ffc9 100644
> --- a/meta/classes-recipe/kernel-yocto.bbclass
> +++ b/meta/classes-recipe/kernel-yocto.bbclass
> @@ -113,6 +113,14 @@ def get_dirs_with_fragments(d):
>
>      return " ".join(extrafiles)
>
> +kbuild_defconfig_exists() {
> +       if [ -f "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}" ]; then
> +               return 0
> +       fi
> +
> +       return 1
> +}
> +
>  do_kernel_metadata() {
>         set +e
>
> @@ -155,7 +163,7 @@ do_kernel_metadata() {
>         # precendence.
>         #
>         if [ -n "${KBUILD_DEFCONFIG}" ]; then
> -               if [ -f "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}" ];
> then
> +               if $(kbuild_defconfig_exists); then
>

replacing this with a function call doesn't make much sense. See below for
why.



>                         if [ -f "${UNPACKDIR}/defconfig" ]; then
>                                 # If the two defconfig's are different,
> warn that we overwrote the
>                                 # one already placed in UNPACKDIR
> @@ -478,8 +486,12 @@ do_kernel_configme() {
>                         config_flags=""
>                         ;;
>                 *)
> -                       if [ -f ${UNPACKDIR}/defconfig ]; then
> -                               config_flags="-n"
> +                       if $(kbuild_defconfig_exists); then
> +                               config_flags=""
> +                       else
> +                               if [ -f ${UNPACKDIR}/defconfig ]; then
> +                                       config_flags="-n"
> +                               fi
>

This is only a sanity fall back, and was something I added only when pushed
for it.  There should be
no mod setting / changing based on logic at this point in the build. Since
we really can't be sure
what type of defconfig is what or if it has somehow been patched or
modified in an intervening task / step.
There's 10+ years of logic that sits on top of the current behaviour.

Anything that relies on a specific type of processing should explicitly set
the KCONFIG_MODE variable
in their kernel recipe / where the defconfig variable is set.

It might make sense to just set the KCONFIG_MODE in the do_kernel_metadata
routine when the
in-tree defconfig is copied and then just use it here.  Then just delete
the entirety of the *) case, since
we don't need a fallback (the default of alldefconfig will just occur since
the config_flags aren't set to
-n).

But we should not be looking into the tree and making a secondary decision
here based on
the source of the defconfig.

Bruce



>                         fi
>                         ;;
>         esac
> --
> 2.48.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#210289):
> https://lists.openembedded.org/g/openembedded-core/message/210289
> Mute This Topic: https://lists.openembedded.org/mt/110821216/1050810
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>

-- 
- Thou shalt not follow the NULL pointer, for chaos and madness await thee
at its end
- "Use the force Harry" - Gandalf, Star Trek II

[-- Attachment #2: Type: text/html, Size: 8286 bytes --]