All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG
@ 2017-11-29  3:28 Saul Wold
  2017-11-29  3:28 ` [PATCH 2/2] kernel-yocto: Stop the build if defconfig is missing Saul Wold
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Saul Wold @ 2017-11-29  3:28 UTC (permalink / raw)
  To: openembedded-core, richard.purdie, bruce.ashfield

When using KBUILD_DEFCONFIG, $sccs should be set to the $WORKDIR/defconfig
regardless if it compares or is copied. Otherwise $sccs is not set and the
defconfig is not found correctly.

Part of
[YOCTO #12162]

Signed-off-by: Saul Wold <sgw@linux.intel.com>
---
 meta/classes/kernel-yocto.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/kernel-yocto.bbclass b/meta/classes/kernel-yocto.bbclass
index 1d447951c49..98ec78fb768 100644
--- a/meta/classes/kernel-yocto.bbclass
+++ b/meta/classes/kernel-yocto.bbclass
@@ -110,8 +110,8 @@ do_kernel_metadata() {
 				fi
 			else
 				cp -f ${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG} ${WORKDIR}/defconfig
-				sccs="${WORKDIR}/defconfig"
 			fi
+			sccs="${WORKDIR}/defconfig"
 		else
 			bbfatal "A KBUILD_DEFCONFIG '${KBUILD_DEFCONFIG}' was specified, but not present in the source tree"
 		fi
-- 
2.13.6



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] kernel-yocto: Stop the build if defconfig is missing
  2017-11-29  3:28 [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG Saul Wold
@ 2017-11-29  3:28 ` Saul Wold
  2017-11-29  3:46 ` [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG Bruce Ashfield
  2017-11-29 14:23 ` Bruce Ashfield
  2 siblings, 0 replies; 10+ messages in thread
From: Saul Wold @ 2017-11-29  3:28 UTC (permalink / raw)
  To: openembedded-core, richard.purdie, bruce.ashfield

The bberror does not stop the build correctly, this should be a
bbfatal_log to ensure the failure correctly stops the build and logs
the failure.

Part of
[YOCTO #12162]

Signed-off-by: Saul Wold <sgw@linux.intel.com>
---
 meta/classes/kernel-yocto.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/kernel-yocto.bbclass b/meta/classes/kernel-yocto.bbclass
index 98ec78fb768..2edf0fd1578 100644
--- a/meta/classes/kernel-yocto.bbclass
+++ b/meta/classes/kernel-yocto.bbclass
@@ -146,7 +146,7 @@ do_kernel_metadata() {
 	if [ -z "$bsp_definition" ]; then
 		echo "$sccs" | grep -q defconfig
 		if [ $? -ne 0 ]; then
-			bberror "Could not locate BSP definition for ${KMACHINE}/${LINUX_KERNEL_TYPE} and no defconfig was provided"
+			bbfatal_log "Could not locate BSP definition for ${KMACHINE}/${LINUX_KERNEL_TYPE} and no defconfig was provided"
 		fi
 	fi
 	meta_dir=$(kgit --meta)
-- 
2.13.6



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG
  2017-11-29  3:28 [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG Saul Wold
  2017-11-29  3:28 ` [PATCH 2/2] kernel-yocto: Stop the build if defconfig is missing Saul Wold
@ 2017-11-29  3:46 ` Bruce Ashfield
  2017-11-29 14:23 ` Bruce Ashfield
  2 siblings, 0 replies; 10+ messages in thread
From: Bruce Ashfield @ 2017-11-29  3:46 UTC (permalink / raw)
  To: Saul Wold, openembedded-core, richard.purdie

Let me take both these into my queue for some extra testing.

I have some other changes that could interact with them.

But they do look fine to me .. I'd just like to test them and
put them in my pull request.

Bruce

On 2017-11-28 10:28 PM, Saul Wold wrote:
> When using KBUILD_DEFCONFIG, $sccs should be set to the $WORKDIR/defconfig
> regardless if it compares or is copied. Otherwise $sccs is not set and the
> defconfig is not found correctly.
> 
> Part of
> [YOCTO #12162]
> 
> Signed-off-by: Saul Wold <sgw@linux.intel.com>
> ---
>   meta/classes/kernel-yocto.bbclass | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/classes/kernel-yocto.bbclass b/meta/classes/kernel-yocto.bbclass
> index 1d447951c49..98ec78fb768 100644
> --- a/meta/classes/kernel-yocto.bbclass
> +++ b/meta/classes/kernel-yocto.bbclass
> @@ -110,8 +110,8 @@ do_kernel_metadata() {
>   				fi
>   			else
>   				cp -f ${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG} ${WORKDIR}/defconfig
> -				sccs="${WORKDIR}/defconfig"
>   			fi
> +			sccs="${WORKDIR}/defconfig"
>   		else
>   			bbfatal "A KBUILD_DEFCONFIG '${KBUILD_DEFCONFIG}' was specified, but not present in the source tree"
>   		fi
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG
  2017-11-29  3:28 [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG Saul Wold
  2017-11-29  3:28 ` [PATCH 2/2] kernel-yocto: Stop the build if defconfig is missing Saul Wold
  2017-11-29  3:46 ` [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG Bruce Ashfield
@ 2017-11-29 14:23 ` Bruce Ashfield
  2017-11-29 16:30   ` Saul Wold
  2 siblings, 1 reply; 10+ messages in thread
From: Bruce Ashfield @ 2017-11-29 14:23 UTC (permalink / raw)
  To: Saul Wold, openembedded-core, richard.purdie

On 11/28/2017 10:28 PM, Saul Wold wrote:
> When using KBUILD_DEFCONFIG, $sccs should be set to the $WORKDIR/defconfig
> regardless if it compares or is copied. Otherwise $sccs is not set and the
> defconfig is not found correctly.

Actually, looking at this more today, and this morning in my testing.
This shouldn't be necessary .. it doesn't hurt anything (well, actually
it could end up with two defconfigs in the variable, but that also
should be ok).

> 
> Part of
> [YOCTO #12162]
> 
> Signed-off-by: Saul Wold <sgw@linux.intel.com>
> ---
>   meta/classes/kernel-yocto.bbclass | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/classes/kernel-yocto.bbclass b/meta/classes/kernel-yocto.bbclass
> index 1d447951c49..98ec78fb768 100644
> --- a/meta/classes/kernel-yocto.bbclass
> +++ b/meta/classes/kernel-yocto.bbclass
> @@ -110,8 +110,8 @@ do_kernel_metadata() {
>   				fi
>   			else
>   				cp -f ${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG} ${WORKDIR}/defconfig
> -				sccs="${WORKDIR}/defconfig"
>   			fi
> +			sccs="${WORKDIR}/defconfig"

The test that was protecting this assignment is:

    if [ -f "${WORKDIR}/defconfig" ]; then

and then:

   cmp "${WORKDIR}/defconfig" 
"${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}"

The only way that a defconfig can be in ${WORKDIR}/defconfig by
the time this runs, is if the fetcher puts it there. Which means
it is on the SRC_URI and comes from the recipe writer's layer.

There is existing code that already picks this up and adds it
to the configuration queue:

         sccs="$sccs ${@" ".join(find_sccs(d))}"

So that defconfig, is already going to be picked up directly
from the SRC_URI.

Bruce


>   		else
>   			bbfatal "A KBUILD_DEFCONFIG '${KBUILD_DEFCONFIG}' was specified, but not present in the source tree"
>   		fi
> 



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG
  2017-11-29 14:23 ` Bruce Ashfield
@ 2017-11-29 16:30   ` Saul Wold
  2017-11-29 16:56     ` Bruce Ashfield
  0 siblings, 1 reply; 10+ messages in thread
From: Saul Wold @ 2017-11-29 16:30 UTC (permalink / raw)
  To: Bruce Ashfield, openembedded-core, richard.purdie

On Wed, 2017-11-29 at 09:23 -0500, Bruce Ashfield wrote:
> On 11/28/2017 10:28 PM, Saul Wold wrote:
> > When using KBUILD_DEFCONFIG, $sccs should be set to the
> > $WORKDIR/defconfig
> > regardless if it compares or is copied. Otherwise $sccs is not set
> > and the
> > defconfig is not found correctly.
> 
> Actually, looking at this more today, and this morning in my testing.
> This shouldn't be necessary .. it doesn't hurt anything (well,
> actually
> it could end up with two defconfigs in the variable, but that also
> should be ok).
> 
Ok, I understand, it's in find_sccs() where if you have "defconfig" in
the SRC_URI then with my change you could end up with two defconfigs.

The problem I saw was if one just sets KBUILD_DEFCONFIG and does not
set any config info on the SRC_URI then it's possible for $sccs to be
empty, which was bad.

Maybe a cleaning of multiple "defconfig" entries on $sccs is needed?

Sau!


> > 
> > Part of
> > [YOCTO #12162]
> > 
> > Signed-off-by: Saul Wold <sgw@linux.intel.com>
> > ---
> >   meta/classes/kernel-yocto.bbclass | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/meta/classes/kernel-yocto.bbclass
> > b/meta/classes/kernel-yocto.bbclass
> > index 1d447951c49..98ec78fb768 100644
> > --- a/meta/classes/kernel-yocto.bbclass
> > +++ b/meta/classes/kernel-yocto.bbclass
> > @@ -110,8 +110,8 @@ do_kernel_metadata() {
> >   				fi
> >   			else
> >   				cp -f
> > ${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG} ${WORKDIR}/defconfig
> > -				sccs="${WORKDIR}/defconfig"
> >   			fi
> > +			sccs="${WORKDIR}/defconfig"
> 
> The test that was protecting this assignment is:
> 
>     if [ -f "${WORKDIR}/defconfig" ]; then
> 
> and then:
> 
>    cmp "${WORKDIR}/defconfig" 
> "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}"
> 
> The only way that a defconfig can be in ${WORKDIR}/defconfig by
> the time this runs, is if the fetcher puts it there. Which means
> it is on the SRC_URI and comes from the recipe writer's layer.
> 
> There is existing code that already picks this up and adds it
> to the configuration queue:
> 
>          sccs="$sccs ${@" ".join(find_sccs(d))}"
> 
> So that defconfig, is already going to be picked up directly
> from the SRC_URI.
> 
> Bruce
> 
> 
> >   		else
> >   			bbfatal "A KBUILD_DEFCONFIG
> > '${KBUILD_DEFCONFIG}' was specified, but not present in the source
> > tree"
> >   		fi
> > 
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG
  2017-11-29 16:30   ` Saul Wold
@ 2017-11-29 16:56     ` Bruce Ashfield
  2017-11-29 17:52       ` Saul Wold
  0 siblings, 1 reply; 10+ messages in thread
From: Bruce Ashfield @ 2017-11-29 16:56 UTC (permalink / raw)
  To: Saul Wold, openembedded-core, richard.purdie

On 11/29/2017 11:30 AM, Saul Wold wrote:
> On Wed, 2017-11-29 at 09:23 -0500, Bruce Ashfield wrote:
>> On 11/28/2017 10:28 PM, Saul Wold wrote:
>>> When using KBUILD_DEFCONFIG, $sccs should be set to the
>>> $WORKDIR/defconfig
>>> regardless if it compares or is copied. Otherwise $sccs is not set
>>> and the
>>> defconfig is not found correctly.
>>
>> Actually, looking at this more today, and this morning in my testing.
>> This shouldn't be necessary .. it doesn't hurt anything (well,
>> actually
>> it could end up with two defconfigs in the variable, but that also
>> should be ok).
>>
> Ok, I understand, it's in find_sccs() where if you have "defconfig" in
> the SRC_URI then with my change you could end up with two defconfigs.
> 
> The problem I saw was if one just sets KBUILD_DEFCONFIG and does not
> set any config info on the SRC_URI then it's possible for $sccs to be
> empty, which was bad.

I took a look at the conditions again, and I can't see that
path. But that doesn't mean it isn't there, is this a configuration
that I can build and see myself ?

> 
> Maybe a cleaning of multiple "defconfig" entries on $sccs is needed?
> 

Yah, regardless of my above statement, it certainly wouldn't hurt
and would be a good safeguard, since two defconfigs would trigger
a M x N configuration audit of options (where M and N could be
in the thousands) .. and hence, take a long time.

Bruce

> Sau!
> 
> 
>>>
>>> Part of
>>> [YOCTO #12162]
>>>
>>> Signed-off-by: Saul Wold <sgw@linux.intel.com>
>>> ---
>>>    meta/classes/kernel-yocto.bbclass | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/meta/classes/kernel-yocto.bbclass
>>> b/meta/classes/kernel-yocto.bbclass
>>> index 1d447951c49..98ec78fb768 100644
>>> --- a/meta/classes/kernel-yocto.bbclass
>>> +++ b/meta/classes/kernel-yocto.bbclass
>>> @@ -110,8 +110,8 @@ do_kernel_metadata() {
>>>    				fi
>>>    			else
>>>    				cp -f
>>> ${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG} ${WORKDIR}/defconfig
>>> -				sccs="${WORKDIR}/defconfig"
>>>    			fi
>>> +			sccs="${WORKDIR}/defconfig"
>>
>> The test that was protecting this assignment is:
>>
>>      if [ -f "${WORKDIR}/defconfig" ]; then
>>
>> and then:
>>
>>     cmp "${WORKDIR}/defconfig"
>> "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}"
>>
>> The only way that a defconfig can be in ${WORKDIR}/defconfig by
>> the time this runs, is if the fetcher puts it there. Which means
>> it is on the SRC_URI and comes from the recipe writer's layer.
>>
>> There is existing code that already picks this up and adds it
>> to the configuration queue:
>>
>>           sccs="$sccs ${@" ".join(find_sccs(d))}"
>>
>> So that defconfig, is already going to be picked up directly
>> from the SRC_URI.
>>
>> Bruce
>>
>>
>>>    		else
>>>    			bbfatal "A KBUILD_DEFCONFIG
>>> '${KBUILD_DEFCONFIG}' was specified, but not present in the source
>>> tree"
>>>    		fi
>>>
>>
>>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG
  2017-11-29 16:56     ` Bruce Ashfield
@ 2017-11-29 17:52       ` Saul Wold
  2017-11-29 18:13         ` Bruce Ashfield
  0 siblings, 1 reply; 10+ messages in thread
From: Saul Wold @ 2017-11-29 17:52 UTC (permalink / raw)
  To: Bruce Ashfield, openembedded-core, richard.purdie

On Wed, 2017-11-29 at 11:56 -0500, Bruce Ashfield wrote:
> On 11/29/2017 11:30 AM, Saul Wold wrote:
> > On Wed, 2017-11-29 at 09:23 -0500, Bruce Ashfield wrote:
> > > On 11/28/2017 10:28 PM, Saul Wold wrote:
> > > > When using KBUILD_DEFCONFIG, $sccs should be set to the
> > > > $WORKDIR/defconfig
> > > > regardless if it compares or is copied. Otherwise $sccs is not
> > > > set
> > > > and the
> > > > defconfig is not found correctly.
> > > 
> > > Actually, looking at this more today, and this morning in my
> > > testing.
> > > This shouldn't be necessary .. it doesn't hurt anything (well,
> > > actually
> > > it could end up with two defconfigs in the variable, but that
> > > also
> > > should be ok).
> > > 
> > 
> > Ok, I understand, it's in find_sccs() where if you have "defconfig"
> > in
> > the SRC_URI then with my change you could end up with two
> > defconfigs.
> > 
> > The problem I saw was if one just sets KBUILD_DEFCONFIG and does
> > not
> > set any config info on the SRC_URI then it's possible for $sccs to
> > be
> > empty, which was bad.
> 
> I took a look at the conditions again, and I can't see that
> path. But that doesn't mean it isn't there, is this a configuration
> that I can build and see myself ?
> 
Set KBUILD_DEFCONFIG to some kernel defconfig, ensure SRC_URI does not
have any defconfig, .scc or .cfg files.  Build once that will populate
the $WORKDIR/defconfig with vai the else path of the first if and the
cp path in the do_kernel_metadata() code.  Build a second time and the
$WORKDIR/defconfig exists now and the first part of the if with the cmp
will occur and the defconfig will not be found because $sccs does not
get set in the original code when the cmp is equal.

Sau!

> > 
> > Maybe a cleaning of multiple "defconfig" entries on $sccs is
> > needed?
> > 
> 
> Yah, regardless of my above statement, it certainly wouldn't hurt
> and would be a good safeguard, since two defconfigs would trigger
> a M x N configuration audit of options (where M and N could be
> in the thousands) .. and hence, take a long time.
> 
> Bruce
> 
> > Sau!
> > 
> > 
> > > > 
> > > > Part of
> > > > [YOCTO #12162]
> > > > 
> > > > Signed-off-by: Saul Wold <sgw@linux.intel.com>
> > > > ---
> > > >    meta/classes/kernel-yocto.bbclass | 2 +-
> > > >    1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/meta/classes/kernel-yocto.bbclass
> > > > b/meta/classes/kernel-yocto.bbclass
> > > > index 1d447951c49..98ec78fb768 100644
> > > > --- a/meta/classes/kernel-yocto.bbclass
> > > > +++ b/meta/classes/kernel-yocto.bbclass
> > > > @@ -110,8 +110,8 @@ do_kernel_metadata() {
> > > >    				fi
> > > >    			else
> > > >    				cp -f
> > > > ${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}
> > > > ${WORKDIR}/defconfig
> > > > -				sccs="${WORKDIR}/defconfig"
> > > >    			fi
> > > > +			sccs="${WORKDIR}/defconfig"
> > > 
> > > The test that was protecting this assignment is:
> > > 
> > >      if [ -f "${WORKDIR}/defconfig" ]; then
> > > 
> > > and then:
> > > 
> > >     cmp "${WORKDIR}/defconfig"
> > > "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}"
> > > 
> > > The only way that a defconfig can be in ${WORKDIR}/defconfig by
> > > the time this runs, is if the fetcher puts it there. Which means
> > > it is on the SRC_URI and comes from the recipe writer's layer.
> > > 
> > > There is existing code that already picks this up and adds it
> > > to the configuration queue:
> > > 
> > >           sccs="$sccs ${@" ".join(find_sccs(d))}"
> > > 
> > > So that defconfig, is already going to be picked up directly
> > > from the SRC_URI.
> > > 
> > > Bruce
> > > 
> > > 
> > > >    		else
> > > >    			bbfatal "A KBUILD_DEFCONFIG
> > > > '${KBUILD_DEFCONFIG}' was specified, but not present in the
> > > > source
> > > > tree"
> > > >    		fi
> > > > 
> > > 
> > > 
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG
  2017-11-29 17:52       ` Saul Wold
@ 2017-11-29 18:13         ` Bruce Ashfield
  2017-11-29 18:24           ` Saul Wold
  0 siblings, 1 reply; 10+ messages in thread
From: Bruce Ashfield @ 2017-11-29 18:13 UTC (permalink / raw)
  To: Saul Wold, openembedded-core, richard.purdie

On 11/29/2017 12:52 PM, Saul Wold wrote:
> On Wed, 2017-11-29 at 11:56 -0500, Bruce Ashfield wrote:
>> On 11/29/2017 11:30 AM, Saul Wold wrote:
>>> On Wed, 2017-11-29 at 09:23 -0500, Bruce Ashfield wrote:
>>>> On 11/28/2017 10:28 PM, Saul Wold wrote:
>>>>> When using KBUILD_DEFCONFIG, $sccs should be set to the
>>>>> $WORKDIR/defconfig
>>>>> regardless if it compares or is copied. Otherwise $sccs is not
>>>>> set
>>>>> and the
>>>>> defconfig is not found correctly.
>>>>
>>>> Actually, looking at this more today, and this morning in my
>>>> testing.
>>>> This shouldn't be necessary .. it doesn't hurt anything (well,
>>>> actually
>>>> it could end up with two defconfigs in the variable, but that
>>>> also
>>>> should be ok).
>>>>
>>>
>>> Ok, I understand, it's in find_sccs() where if you have "defconfig"
>>> in
>>> the SRC_URI then with my change you could end up with two
>>> defconfigs.
>>>
>>> The problem I saw was if one just sets KBUILD_DEFCONFIG and does
>>> not
>>> set any config info on the SRC_URI then it's possible for $sccs to
>>> be
>>> empty, which was bad.
>>
>> I took a look at the conditions again, and I can't see that
>> path. But that doesn't mean it isn't there, is this a configuration
>> that I can build and see myself ?
>>
> Set KBUILD_DEFCONFIG to some kernel defconfig, ensure SRC_URI does not
> have any defconfig, .scc or .cfg files.  Build once that will populate
> the $WORKDIR/defconfig with vai the else path of the first if and the
> cp path in the do_kernel_metadata() code.  Build a second time and the
> $WORKDIR/defconfig exists now and the first part of the if with the cmp
> will occur and the defconfig will not be found because $sccs does not
> get set in the original code when the cmp is equal.

That's the ticket .. I didn't think of a 2nd pass through the
build.

So with that extra assignment + a duplicate removal, it should
be safe for all the paths.

Did you want to send a v2, or did you want me to make that tweak?

Either way, I'll continue testing here.

Bruce

> 
> Sau!
> 
>>>
>>> Maybe a cleaning of multiple "defconfig" entries on $sccs is
>>> needed?
>>>
>>
>> Yah, regardless of my above statement, it certainly wouldn't hurt
>> and would be a good safeguard, since two defconfigs would trigger
>> a M x N configuration audit of options (where M and N could be
>> in the thousands) .. and hence, take a long time.
>>
>> Bruce
>>
>>> Sau!
>>>
>>>
>>>>>
>>>>> Part of
>>>>> [YOCTO #12162]
>>>>>
>>>>> Signed-off-by: Saul Wold <sgw@linux.intel.com>
>>>>> ---
>>>>>     meta/classes/kernel-yocto.bbclass | 2 +-
>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/meta/classes/kernel-yocto.bbclass
>>>>> b/meta/classes/kernel-yocto.bbclass
>>>>> index 1d447951c49..98ec78fb768 100644
>>>>> --- a/meta/classes/kernel-yocto.bbclass
>>>>> +++ b/meta/classes/kernel-yocto.bbclass
>>>>> @@ -110,8 +110,8 @@ do_kernel_metadata() {
>>>>>     				fi
>>>>>     			else
>>>>>     				cp -f
>>>>> ${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}
>>>>> ${WORKDIR}/defconfig
>>>>> -				sccs="${WORKDIR}/defconfig"
>>>>>     			fi
>>>>> +			sccs="${WORKDIR}/defconfig"
>>>>
>>>> The test that was protecting this assignment is:
>>>>
>>>>       if [ -f "${WORKDIR}/defconfig" ]; then
>>>>
>>>> and then:
>>>>
>>>>      cmp "${WORKDIR}/defconfig"
>>>> "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}"
>>>>
>>>> The only way that a defconfig can be in ${WORKDIR}/defconfig by
>>>> the time this runs, is if the fetcher puts it there. Which means
>>>> it is on the SRC_URI and comes from the recipe writer's layer.
>>>>
>>>> There is existing code that already picks this up and adds it
>>>> to the configuration queue:
>>>>
>>>>            sccs="$sccs ${@" ".join(find_sccs(d))}"
>>>>
>>>> So that defconfig, is already going to be picked up directly
>>>> from the SRC_URI.
>>>>
>>>> Bruce
>>>>
>>>>
>>>>>     		else
>>>>>     			bbfatal "A KBUILD_DEFCONFIG
>>>>> '${KBUILD_DEFCONFIG}' was specified, but not present in the
>>>>> source
>>>>> tree"
>>>>>     		fi
>>>>>
>>>>
>>>>
>>
>>



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG
  2017-11-29 18:13         ` Bruce Ashfield
@ 2017-11-29 18:24           ` Saul Wold
  2017-12-01  5:38             ` Bruce Ashfield
  0 siblings, 1 reply; 10+ messages in thread
From: Saul Wold @ 2017-11-29 18:24 UTC (permalink / raw)
  To: Bruce Ashfield, openembedded-core, richard.purdie

On Wed, 2017-11-29 at 13:13 -0500, Bruce Ashfield wrote:
> On 11/29/2017 12:52 PM, Saul Wold wrote:
> > On Wed, 2017-11-29 at 11:56 -0500, Bruce Ashfield wrote:
> > > On 11/29/2017 11:30 AM, Saul Wold wrote:
> > > > On Wed, 2017-11-29 at 09:23 -0500, Bruce Ashfield wrote:
> > > > > On 11/28/2017 10:28 PM, Saul Wold wrote:
> > > > > > When using KBUILD_DEFCONFIG, $sccs should be set to the
> > > > > > $WORKDIR/defconfig
> > > > > > regardless if it compares or is copied. Otherwise $sccs is
> > > > > > not
> > > > > > set
> > > > > > and the
> > > > > > defconfig is not found correctly.
> > > > > 
> > > > > Actually, looking at this more today, and this morning in my
> > > > > testing.
> > > > > This shouldn't be necessary .. it doesn't hurt anything
> > > > > (well,
> > > > > actually
> > > > > it could end up with two defconfigs in the variable, but that
> > > > > also
> > > > > should be ok).
> > > > > 
> > > > 
> > > > Ok, I understand, it's in find_sccs() where if you have
> > > > "defconfig"
> > > > in
> > > > the SRC_URI then with my change you could end up with two
> > > > defconfigs.
> > > > 
> > > > The problem I saw was if one just sets KBUILD_DEFCONFIG and
> > > > does
> > > > not
> > > > set any config info on the SRC_URI then it's possible for $sccs
> > > > to
> > > > be
> > > > empty, which was bad.
> > > 
> > > I took a look at the conditions again, and I can't see that
> > > path. But that doesn't mean it isn't there, is this a
> > > configuration
> > > that I can build and see myself ?
> > > 
> > 
> > Set KBUILD_DEFCONFIG to some kernel defconfig, ensure SRC_URI does
> > not
> > have any defconfig, .scc or .cfg files.  Build once that will
> > populate
> > the $WORKDIR/defconfig with vai the else path of the first if and
> > the
> > cp path in the do_kernel_metadata() code.  Build a second time and
> > the
> > $WORKDIR/defconfig exists now and the first part of the if with the
> > cmp
> > will occur and the defconfig will not be found because $sccs does
> > not
> > get set in the original code when the cmp is equal.
> 
> That's the ticket .. I didn't think of a 2nd pass through the
> build.
> 
> So with that extra assignment + a duplicate removal, it should
> be safe for all the paths.
> 
> Did you want to send a v2, or did you want me to make that tweak?
> 
If you have the tweak in mind already, might be better for you to make
it.

Thanks
 Sau!

> Either way, I'll continue testing here.
> 
> Bruce
> 
> > 
> > Sau!
> > 
> > > > 
> > > > Maybe a cleaning of multiple "defconfig" entries on $sccs is
> > > > needed?
> > > > 
> > > 
> > > Yah, regardless of my above statement, it certainly wouldn't hurt
> > > and would be a good safeguard, since two defconfigs would trigger
> > > a M x N configuration audit of options (where M and N could be
> > > in the thousands) .. and hence, take a long time.
> > > 
> > > Bruce
> > > 
> > > > Sau!
> > > > 
> > > > 
> > > > > > 
> > > > > > Part of
> > > > > > [YOCTO #12162]
> > > > > > 
> > > > > > Signed-off-by: Saul Wold <sgw@linux.intel.com>
> > > > > > ---
> > > > > >     meta/classes/kernel-yocto.bbclass | 2 +-
> > > > > >     1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/meta/classes/kernel-yocto.bbclass
> > > > > > b/meta/classes/kernel-yocto.bbclass
> > > > > > index 1d447951c49..98ec78fb768 100644
> > > > > > --- a/meta/classes/kernel-yocto.bbclass
> > > > > > +++ b/meta/classes/kernel-yocto.bbclass
> > > > > > @@ -110,8 +110,8 @@ do_kernel_metadata() {
> > > > > >     				fi
> > > > > >     			else
> > > > > >     				cp -f
> > > > > > ${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}
> > > > > > ${WORKDIR}/defconfig
> > > > > > -				sccs="${WORKDIR}/defconfig
> > > > > > "
> > > > > >     			fi
> > > > > > +			sccs="${WORKDIR}/defconfig"
> > > > > 
> > > > > The test that was protecting this assignment is:
> > > > > 
> > > > >       if [ -f "${WORKDIR}/defconfig" ]; then
> > > > > 
> > > > > and then:
> > > > > 
> > > > >      cmp "${WORKDIR}/defconfig"
> > > > > "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}"
> > > > > 
> > > > > The only way that a defconfig can be in ${WORKDIR}/defconfig
> > > > > by
> > > > > the time this runs, is if the fetcher puts it there. Which
> > > > > means
> > > > > it is on the SRC_URI and comes from the recipe writer's
> > > > > layer.
> > > > > 
> > > > > There is existing code that already picks this up and adds it
> > > > > to the configuration queue:
> > > > > 
> > > > >            sccs="$sccs ${@" ".join(find_sccs(d))}"
> > > > > 
> > > > > So that defconfig, is already going to be picked up directly
> > > > > from the SRC_URI.
> > > > > 
> > > > > Bruce
> > > > > 
> > > > > 
> > > > > >     		else
> > > > > >     			bbfatal "A KBUILD_DEFCONFIG
> > > > > > '${KBUILD_DEFCONFIG}' was specified, but not present in the
> > > > > > source
> > > > > > tree"
> > > > > >     		fi
> > > > > > 
> > > > > 
> > > > > 
> > > 
> > > 
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG
  2017-11-29 18:24           ` Saul Wold
@ 2017-12-01  5:38             ` Bruce Ashfield
  0 siblings, 0 replies; 10+ messages in thread
From: Bruce Ashfield @ 2017-12-01  5:38 UTC (permalink / raw)
  To: Saul Wold, openembedded-core, richard.purdie

On 2017-11-29 1:24 PM, Saul Wold wrote:
> On Wed, 2017-11-29 at 13:13 -0500, Bruce Ashfield wrote:
>> On 11/29/2017 12:52 PM, Saul Wold wrote:
>>> On Wed, 2017-11-29 at 11:56 -0500, Bruce Ashfield wrote:
>>>> On 11/29/2017 11:30 AM, Saul Wold wrote:
>>>>> On Wed, 2017-11-29 at 09:23 -0500, Bruce Ashfield wrote:
>>>>>> On 11/28/2017 10:28 PM, Saul Wold wrote:
>>>>>>> When using KBUILD_DEFCONFIG, $sccs should be set to the
>>>>>>> $WORKDIR/defconfig
>>>>>>> regardless if it compares or is copied. Otherwise $sccs is
>>>>>>> not
>>>>>>> set
>>>>>>> and the
>>>>>>> defconfig is not found correctly.
>>>>>>
>>>>>> Actually, looking at this more today, and this morning in my
>>>>>> testing.
>>>>>> This shouldn't be necessary .. it doesn't hurt anything
>>>>>> (well,
>>>>>> actually
>>>>>> it could end up with two defconfigs in the variable, but that
>>>>>> also
>>>>>> should be ok).
>>>>>>
>>>>>
>>>>> Ok, I understand, it's in find_sccs() where if you have
>>>>> "defconfig"
>>>>> in
>>>>> the SRC_URI then with my change you could end up with two
>>>>> defconfigs.
>>>>>
>>>>> The problem I saw was if one just sets KBUILD_DEFCONFIG and
>>>>> does
>>>>> not
>>>>> set any config info on the SRC_URI then it's possible for $sccs
>>>>> to
>>>>> be
>>>>> empty, which was bad.
>>>>
>>>> I took a look at the conditions again, and I can't see that
>>>> path. But that doesn't mean it isn't there, is this a
>>>> configuration
>>>> that I can build and see myself ?
>>>>
>>>
>>> Set KBUILD_DEFCONFIG to some kernel defconfig, ensure SRC_URI does
>>> not
>>> have any defconfig, .scc or .cfg files.  Build once that will
>>> populate
>>> the $WORKDIR/defconfig with vai the else path of the first if and
>>> the
>>> cp path in the do_kernel_metadata() code.  Build a second time and
>>> the
>>> $WORKDIR/defconfig exists now and the first part of the if with the
>>> cmp
>>> will occur and the defconfig will not be found because $sccs does
>>> not
>>> get set in the original code when the cmp is equal.
>>
>> That's the ticket .. I didn't think of a 2nd pass through the
>> build.
>>
>> So with that extra assignment + a duplicate removal, it should
>> be safe for all the paths.
>>
>> Did you want to send a v2, or did you want me to make that tweak?
>>
> If you have the tweak in mind already, might be better for you to make
> it.
> 

One more question, since I'm still not able to trigger the issue
here.

When you are getting the failure mode, is the:

   bbwarn "defconfig detected in WORKDIR ..." being triggered ?

I actually think it would be better if I flip that warning around
and always overwrite what is in WORKDIR if someone is using the
KBUILD_DEFCONFIG functionality.

But for now, I will just remove the potential duplicate defconfig
and ponder that change in functionality more.

Bruce

> Thanks
>   Sau!
> 
>> Either way, I'll continue testing here.
>>
>> Bruce
>>
>>>
>>> Sau!
>>>
>>>>>
>>>>> Maybe a cleaning of multiple "defconfig" entries on $sccs is
>>>>> needed?
>>>>>
>>>>
>>>> Yah, regardless of my above statement, it certainly wouldn't hurt
>>>> and would be a good safeguard, since two defconfigs would trigger
>>>> a M x N configuration audit of options (where M and N could be
>>>> in the thousands) .. and hence, take a long time.
>>>>
>>>> Bruce
>>>>
>>>>> Sau!
>>>>>
>>>>>
>>>>>>>
>>>>>>> Part of
>>>>>>> [YOCTO #12162]
>>>>>>>
>>>>>>> Signed-off-by: Saul Wold <sgw@linux.intel.com>
>>>>>>> ---
>>>>>>>      meta/classes/kernel-yocto.bbclass | 2 +-
>>>>>>>      1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/meta/classes/kernel-yocto.bbclass
>>>>>>> b/meta/classes/kernel-yocto.bbclass
>>>>>>> index 1d447951c49..98ec78fb768 100644
>>>>>>> --- a/meta/classes/kernel-yocto.bbclass
>>>>>>> +++ b/meta/classes/kernel-yocto.bbclass
>>>>>>> @@ -110,8 +110,8 @@ do_kernel_metadata() {
>>>>>>>      				fi
>>>>>>>      			else
>>>>>>>      				cp -f
>>>>>>> ${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}
>>>>>>> ${WORKDIR}/defconfig
>>>>>>> -				sccs="${WORKDIR}/defconfig
>>>>>>> "
>>>>>>>      			fi
>>>>>>> +			sccs="${WORKDIR}/defconfig"
>>>>>>
>>>>>> The test that was protecting this assignment is:
>>>>>>
>>>>>>        if [ -f "${WORKDIR}/defconfig" ]; then
>>>>>>
>>>>>> and then:
>>>>>>
>>>>>>       cmp "${WORKDIR}/defconfig"
>>>>>> "${S}/arch/${ARCH}/configs/${KBUILD_DEFCONFIG}"
>>>>>>
>>>>>> The only way that a defconfig can be in ${WORKDIR}/defconfig
>>>>>> by
>>>>>> the time this runs, is if the fetcher puts it there. Which
>>>>>> means
>>>>>> it is on the SRC_URI and comes from the recipe writer's
>>>>>> layer.
>>>>>>
>>>>>> There is existing code that already picks this up and adds it
>>>>>> to the configuration queue:
>>>>>>
>>>>>>             sccs="$sccs ${@" ".join(find_sccs(d))}"
>>>>>>
>>>>>> So that defconfig, is already going to be picked up directly
>>>>>> from the SRC_URI.
>>>>>>
>>>>>> Bruce
>>>>>>
>>>>>>
>>>>>>>      		else
>>>>>>>      			bbfatal "A KBUILD_DEFCONFIG
>>>>>>> '${KBUILD_DEFCONFIG}' was specified, but not present in the
>>>>>>> source
>>>>>>> tree"
>>>>>>>      		fi
>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>



^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-12-01  5:38 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-29  3:28 [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG Saul Wold
2017-11-29  3:28 ` [PATCH 2/2] kernel-yocto: Stop the build if defconfig is missing Saul Wold
2017-11-29  3:46 ` [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG Bruce Ashfield
2017-11-29 14:23 ` Bruce Ashfield
2017-11-29 16:30   ` Saul Wold
2017-11-29 16:56     ` Bruce Ashfield
2017-11-29 17:52       ` Saul Wold
2017-11-29 18:13         ` Bruce Ashfield
2017-11-29 18:24           ` Saul Wold
2017-12-01  5:38             ` Bruce Ashfield

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.