Re: [PATCH 1/2] kernel-yocto: ensure sccs variable is set when using KBUILD_DEFCONFIG

[Saul Wold <sgw@linux.intel.com>]

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
> > > > > > 
> > > > > 
> > > > > 
> > > 
> > > 
> 
>