[PATCH] kernel-yocto: checksum indirect cfg and scc files

[PATCH] kernel-yocto: checksum indirect cfg and scc files

[Zhaolong Zhang]

Currently, Yocto can not realize the modification of the cfg/scc files indirectly
introduced by scc files in custom layers.

Instead of introducing complicated scc parser code, this patch walks though
FILESEXTRAPATHS and takes all the cfg/scc files into account when calculating
checksums.

Signed-off-by: Zhaolong Zhang <zhangzl2013@126.com>
---
 meta/classes/kernel-yocto.bbclass | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/meta/classes/kernel-yocto.bbclass b/meta/classes/kernel-yocto.bbclass
index ed9bcfa57c..a9aac8a9d9 100644
--- a/meta/classes/kernel-yocto.bbclass
+++ b/meta/classes/kernel-yocto.bbclass
@@ -69,6 +69,24 @@ def get_machine_branch(d, default):
 	    
     return default
 
+def get_files_in_filesextrapaths(d):
+    extrapaths = []
+    extrafiles = []
+    extrapathsvalue = (d.getVar("FILESEXTRAPATHS") or "")
+    # Remove default flag which was used for checking
+    extrapathsvalue = extrapathsvalue.replace("__default:", "")
+    extrapaths = extrapathsvalue.split(":")
+    for path in extrapaths:
+        for root, dirs, files in os.walk(path):
+            for name in files:
+                base, ext = os.path.splitext(name)
+                if ext and ext in [".scc", ".cfg"]:
+                    filepath = os.path.join(root, name)
+                    extrafiles.append(filepath + ":" + str(os.path.exists(filepath)))
+    return " ".join(extrafiles)
+
+
+
 do_kernel_metadata() {
 	set +e
 	cd ${S}
@@ -296,6 +314,7 @@ do_kernel_checkout[dirs] = "${S}"
 addtask kernel_checkout before do_kernel_metadata after do_unpack
 addtask kernel_metadata after do_validate_branches do_unpack before do_patch
 do_kernel_metadata[depends] = "kern-tools-native:do_populate_sysroot"
+do_kernel_metadata[file-checksums] = " ${@get_files_in_filesextrapaths(d)}"
 do_validate_branches[depends] = "kern-tools-native:do_populate_sysroot"
 
 do_kernel_configme[depends] += "virtual/${TARGET_PREFIX}binutils:do_populate_sysroot"
-- 
2.19.1

Re: [PATCH] kernel-yocto: checksum indirect cfg and scc files

[Bruce Ashfield]

On Thu, Jul 4, 2019 at 7:02 AM Zhaolong Zhang <zhangzl2013@126.com> wrote:
>
> Currently, Yocto can not realize the modification of the cfg/scc files indirectly
> introduced by scc files in custom layers.
>
> Instead of introducing complicated scc parser code, this patch walks though
> FILESEXTRAPATHS and takes all the cfg/scc files into account when calculating
> checksums.

There used to be a bugzilla around for this .. but I can't find it now.

While the approach isn't wrong, I think it is too heavy, since it is
looking at *all* the .scc and .cfg files that can be located in the
search paths, not just the ones that are actually used.

I do have some old code from the existing bugzilla that I can try and
locate. The right approach is to have the kern-tools emit the list of
files, since that's where we know the includes, etc, and what is
actually going to be used. What you have will also conflict a bit with
some changes that I'm making to tweak the config handling.

Since I can't find the old bugzilla, can you open a new one, put the
patch there and I can find the code to dump the list of files from the
tools.

Bruce

>
> Signed-off-by: Zhaolong Zhang <zhangzl2013@126.com>
> ---
>  meta/classes/kernel-yocto.bbclass | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
>
> diff --git a/meta/classes/kernel-yocto.bbclass b/meta/classes/kernel-yocto.bbclass
> index ed9bcfa57c..a9aac8a9d9 100644
> --- a/meta/classes/kernel-yocto.bbclass
> +++ b/meta/classes/kernel-yocto.bbclass
> @@ -69,6 +69,24 @@ def get_machine_branch(d, default):
>
>      return default
>
> +def get_files_in_filesextrapaths(d):
> +    extrapaths = []
> +    extrafiles = []
> +    extrapathsvalue = (d.getVar("FILESEXTRAPATHS") or "")
> +    # Remove default flag which was used for checking
> +    extrapathsvalue = extrapathsvalue.replace("__default:", "")
> +    extrapaths = extrapathsvalue.split(":")
> +    for path in extrapaths:
> +        for root, dirs, files in os.walk(path):
> +            for name in files:
> +                base, ext = os.path.splitext(name)
> +                if ext and ext in [".scc", ".cfg"]:
> +                    filepath = os.path.join(root, name)
> +                    extrafiles.append(filepath + ":" + str(os.path.exists(filepath)))
> +    return " ".join(extrafiles)
> +
> +
> +
>  do_kernel_metadata() {
>         set +e
>         cd ${S}
> @@ -296,6 +314,7 @@ do_kernel_checkout[dirs] = "${S}"
>  addtask kernel_checkout before do_kernel_metadata after do_unpack
>  addtask kernel_metadata after do_validate_branches do_unpack before do_patch
>  do_kernel_metadata[depends] = "kern-tools-native:do_populate_sysroot"
> +do_kernel_metadata[file-checksums] = " ${@get_files_in_filesextrapaths(d)}"
>  do_validate_branches[depends] = "kern-tools-native:do_populate_sysroot"
>
>  do_kernel_configme[depends] += "virtual/${TARGET_PREFIX}binutils:do_populate_sysroot"
> --
> 2.19.1
>
> --
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core



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

Re: [PATCH] kernel-yocto: checksum indirect cfg and scc files

[Richard Purdie]

On Thu, 2019-07-04 at 08:48 -0400, Bruce Ashfield wrote:
> On Thu, Jul 4, 2019 at 7:02 AM Zhaolong Zhang <zhangzl2013@126.com>
> wrote:
> > Currently, Yocto can not realize the modification of the cfg/scc
> > files indirectly
> > introduced by scc files in custom layers.
> > 
> > Instead of introducing complicated scc parser code, this patch
> > walks though
> > FILESEXTRAPATHS and takes all the cfg/scc files into account when
> > calculating
> > checksums.
> 
> There used to be a bugzilla around for this .. but I can't find it
> now.
> 
> While the approach isn't wrong, I think it is too heavy, since it is
> looking at *all* the .scc and .cfg files that can be located in the
> search paths, not just the ones that are actually used.

That isn't quite right. With the checksums its important to know if a
new file appears at location X, we should reparse as it could change
the outcome.

We therefore have to account for files which doesn't exist as much as
the ones that do.

> I do have some old code from the existing bugzilla that I can try and
> locate. The right approach is to have the kern-tools emit the list of
> files, since that's where we know the includes, etc, and what is
> actually going to be used. What you have will also conflict a bit
> with
> some changes that I'm making to tweak the config handling.
> 
> Since I can't find the old bugzilla, can you open a new one, put the
> patch there and I can find the code to dump the list of files from
> the tools.

This doesn't work since we need to be able to predict the task hash
checksum at parse time. We don't have the kern-tools available then to
be able to know which ones it would actually use...

Cheers,

Richard

Re: [PATCH] kernel-yocto: checksum indirect cfg and scc files

[Bruce Ashfield]

On Thu, Jul 4, 2019 at 11:18 AM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> On Thu, 2019-07-04 at 08:48 -0400, Bruce Ashfield wrote:
> > On Thu, Jul 4, 2019 at 7:02 AM Zhaolong Zhang <zhangzl2013@126.com>
> > wrote:
> > > Currently, Yocto can not realize the modification of the cfg/scc
> > > files indirectly
> > > introduced by scc files in custom layers.
> > >
> > > Instead of introducing complicated scc parser code, this patch
> > > walks though
> > > FILESEXTRAPATHS and takes all the cfg/scc files into account when
> > > calculating
> > > checksums.
> >
> > There used to be a bugzilla around for this .. but I can't find it
> > now.
> >
> > While the approach isn't wrong, I think it is too heavy, since it is
> > looking at *all* the .scc and .cfg files that can be located in the
> > search paths, not just the ones that are actually used.
>
> That isn't quite right. With the checksums its important to know if a
> new file appears at location X, we should reparse as it could change
> the outcome.
>
> We therefore have to account for files which doesn't exist as much as
> the ones that do.

Maybe I'm misunderstanding what you are saying here, but these are
just sitting around (like unused patch files). They are not on the
SRC_URI and they are not necessarily used at all. Just because someone
drops a new file in those locations, we should not be re-running the
meta data task.

What that routine is currently doing is just wrong.

>
> > I do have some old code from the existing bugzilla that I can try and
> > locate. The right approach is to have the kern-tools emit the list of
> > files, since that's where we know the includes, etc, and what is
> > actually going to be used. What you have will also conflict a bit
> > with
> > some changes that I'm making to tweak the config handling.
> >
> > Since I can't find the old bugzilla, can you open a new one, put the
> > patch there and I can find the code to dump the list of files from
> > the tools.
>
> This doesn't work since we need to be able to predict the task hash
> checksum at parse time. We don't have the kern-tools available then to
> be able to know which ones it would actually use...

So there's only python code allowed in those hash routines ? If so,
what is there is still wrong, and needs to be reworked.

Bruce

>
> Cheers,
>
> Richard
>


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

Re: [PATCH] kernel-yocto: checksum indirect cfg and scc files

[richard.purdie]

On Thu, 2019-07-04 at 12:29 -0400, Bruce Ashfield wrote:
> On Thu, Jul 4, 2019 at 11:18 AM Richard Purdie
> <richard.purdie@linuxfoundation.org> wrote:
> > On Thu, 2019-07-04 at 08:48 -0400, Bruce Ashfield wrote:
> > > On Thu, Jul 4, 2019 at 7:02 AM Zhaolong Zhang <
> > > zhangzl2013@126.com>
> > > wrote:
> > > > Currently, Yocto can not realize the modification of the
> > > > cfg/scc
> > > > files indirectly
> > > > introduced by scc files in custom layers.
> > > > 
> > > > Instead of introducing complicated scc parser code, this patch
> > > > walks though
> > > > FILESEXTRAPATHS and takes all the cfg/scc files into account
> > > > when
> > > > calculating
> > > > checksums.
> > > 
> > > There used to be a bugzilla around for this .. but I can't find
> > > it
> > > now.
> > > 
> > > While the approach isn't wrong, I think it is too heavy, since it
> > > is
> > > looking at *all* the .scc and .cfg files that can be located in
> > > the
> > > search paths, not just the ones that are actually used.
> > 
> > That isn't quite right. With the checksums its important to know if
> > a
> > new file appears at location X, we should reparse as it could
> > change
> > the outcome.
> > 
> > We therefore have to account for files which doesn't exist as much
> > as
> > the ones that do.
> 
> Maybe I'm misunderstanding what you are saying here, but these are
> just sitting around (like unused patch files). They are not on the
> SRC_URI and they are not necessarily used at all. Just because
> someone
> drops a new file in those locations, we should not be re-running the
> meta data task.
> 
> What that routine is currently doing is just wrong.

Agreed, it is.

I'm just saying that this situation isn't as simple as files exist, we
also need to look at which files don't exist, but that would influence
the build if they did.

The patch doesn't do that either!


> > This doesn't work since we need to be able to predict the task hash
> > checksum at parse time. We don't have the kern-tools available then
> > to
> > be able to know which ones it would actually use...
> 
> So there's only python code allowed in those hash routines ? If so,
> what is there is still wrong, and needs to be reworked.

It has to be able to work on the information available to it at parse
time. In reality that does mean python code. There are performance
implications to anything too complex.

Cheers,

Richard

Re: [PATCH] kernel-yocto: checksum indirect cfg and scc files

[Bruce Ashfield]

On Thu, Jul 4, 2019 at 12:36 PM <richard.purdie@linuxfoundation.org> wrote:
>
> On Thu, 2019-07-04 at 12:29 -0400, Bruce Ashfield wrote:
> > On Thu, Jul 4, 2019 at 11:18 AM Richard Purdie
> > <richard.purdie@linuxfoundation.org> wrote:
> > > On Thu, 2019-07-04 at 08:48 -0400, Bruce Ashfield wrote:
> > > > On Thu, Jul 4, 2019 at 7:02 AM Zhaolong Zhang <
> > > > zhangzl2013@126.com>
> > > > wrote:
> > > > > Currently, Yocto can not realize the modification of the
> > > > > cfg/scc
> > > > > files indirectly
> > > > > introduced by scc files in custom layers.
> > > > >
> > > > > Instead of introducing complicated scc parser code, this patch
> > > > > walks though
> > > > > FILESEXTRAPATHS and takes all the cfg/scc files into account
> > > > > when
> > > > > calculating
> > > > > checksums.
> > > >
> > > > There used to be a bugzilla around for this .. but I can't find
> > > > it
> > > > now.
> > > >
> > > > While the approach isn't wrong, I think it is too heavy, since it
> > > > is
> > > > looking at *all* the .scc and .cfg files that can be located in
> > > > the
> > > > search paths, not just the ones that are actually used.
> > >
> > > That isn't quite right. With the checksums its important to know if
> > > a
> > > new file appears at location X, we should reparse as it could
> > > change
> > > the outcome.
> > >
> > > We therefore have to account for files which doesn't exist as much
> > > as
> > > the ones that do.
> >
> > Maybe I'm misunderstanding what you are saying here, but these are
> > just sitting around (like unused patch files). They are not on the
> > SRC_URI and they are not necessarily used at all. Just because
> > someone
> > drops a new file in those locations, we should not be re-running the
> > meta data task.
> >
> > What that routine is currently doing is just wrong.
>
> Agreed, it is.
>
> I'm just saying that this situation isn't as simple as files exist, we
> also need to look at which files don't exist, but that would influence
> the build if they did.

Aha.

>
> The patch doesn't do that either!
>
>
> > > This doesn't work since we need to be able to predict the task hash
> > > checksum at parse time. We don't have the kern-tools available then
> > > to
> > > be able to know which ones it would actually use...
> >
> > So there's only python code allowed in those hash routines ? If so,
> > what is there is still wrong, and needs to be reworked.
>
> It has to be able to work on the information available to it at parse
> time. In reality that does mean python code. There are performance
> implications to anything too complex.

Understood.

I'll think on this for a bit. This has been something that I looked
into several times, and didn't come up with anything I really liked.
Maybe now is the time to solve the issue :)

Bruce

>
> Cheers,
>
> Richard
>


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

Re: [PATCH] kernel-yocto: checksum indirect cfg and scc files

[Zhaolong Zhang]

At 2019-07-05 00:42:53, "Bruce Ashfield" <bruce.ashfield@gmail.com> wrote:
>On Thu, Jul 4, 2019 at 12:36 PM <richard.purdie@linuxfoundation.org> wrote:
>>
>> On Thu, 2019-07-04 at 12:29 -0400, Bruce Ashfield wrote:
>> > On Thu, Jul 4, 2019 at 11:18 AM Richard Purdie
>> > <richard.purdie@linuxfoundation.org> wrote:
>> > > On Thu, 2019-07-04 at 08:48 -0400, Bruce Ashfield wrote:
>> > > > On Thu, Jul 4, 2019 at 7:02 AM Zhaolong Zhang <
>> > > > zhangzl2013@126.com>
>> > > > wrote:
>> > > > > Currently, Yocto can not realize the modification of the
>> > > > > cfg/scc
>> > > > > files indirectly
>> > > > > introduced by scc files in custom layers.
>> > > > >
>> > > > > Instead of introducing complicated scc parser code, this patch
>> > > > > walks though
>> > > > > FILESEXTRAPATHS and takes all the cfg/scc files into account
>> > > > > when
>> > > > > calculating
>> > > > > checksums.
>> > > >
>> > > > There used to be a bugzilla around for this .. but I can't find
>> > > > it
>> > > > now.
>> > > >
>> > > > While the approach isn't wrong, I think it is too heavy, since it
>> > > > is
>> > > > looking at *all* the .scc and .cfg files that can be located in
>> > > > the
>> > > > search paths, not just the ones that are actually used.
>> > >
>> > > That isn't quite right. With the checksums its important to know if
>> > > a
>> > > new file appears at location X, we should reparse as it could
>> > > change
>> > > the outcome.
>> > >
>> > > We therefore have to account for files which doesn't exist as much
>> > > as
>> > > the ones that do.
>> >
>> > Maybe I'm misunderstanding what you are saying here, but these are
>> > just sitting around (like unused patch files). They are not on the
>> > SRC_URI and they are not necessarily used at all. Just because
>> > someone
>> > drops a new file in those locations, we should not be re-running the
>> > meta data task.
>> >
>> > What that routine is currently doing is just wrong.
>>
>> Agreed, it is.
>>
>> I'm just saying that this situation isn't as simple as files exist, we
>> also need to look at which files don't exist, but that would influence
>> the build if they did.
>
>Aha.
>
>>
>> The patch doesn't do that either!
>>
>>
>> > > This doesn't work since we need to be able to predict the task hash
>> > > checksum at parse time. We don't have the kern-tools available then
>> > > to
>> > > be able to know which ones it would actually use...
>> >
>> > So there's only python code allowed in those hash routines ? If so,
>> > what is there is still wrong, and needs to be reworked.
>>
>> It has to be able to work on the information available to it at parse
>> time. In reality that does mean python code. There are performance
>> implications to anything too complex.
>
>Understood.
>
>I'll think on this for a bit. This has been something that I looked
>into several times, and didn't come up with anything I really liked.
>Maybe now is the time to solve the issue :)

Thank you Bruce, I will be waiting for your solution.

Regards,
Zhaolong

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

Re: [PATCH] kernel-yocto: checksum indirect cfg and scc files

[Robert Berger]

Hi everybody,

Let me throw in my 2 cents ;)

We have 2 cases (and combinations)

1) .scc, .cfg, .patch files just lying around not being used

2) .scc, .cfg, .patch files actually somehow being included in the 
kernel recipe and hence being used

Say, the kernel recipe includes something like this:

SRC_URI += "file://imx6q-phytec-mira-rdk-nand-bsp.scc \
             file://imx6q-phytec-mira-rdk-nand-bsp.cfg \
             file://imx6q-phytec-mira-rdk-nand-bsp-user-config.cfg \
             file://imx6q-phytec-mira-rdk-nand-bsp-user-patches.scc \
             file://imx6q-phytec-mira-rdk-nand-bsp-user-features.scc \
            "

The various files contain this:

== imx6q-phytec-mira-rdk-nand-bsp.scc ==

kconf hardware imx6q-phytec-mira-rdk-nand-bsp.cfg

== imx6q-phytec-mira-rdk-nand-bsp-user-config.cfg ==

CONFIG_MODULES=y

== imx6q-phytec-mira-rdk-nand-bsp-user-patches.scc ==

patch 0001-revert-patch-e7e73b10d690c5352cb11b1941a09e4f3dc4c8c.patch

== imx6q-phytec-mira-rdk-nand-bsp-user-features.scc ==
include <from somewhere>/linux-yocto-custom/cfg/ikconfig.scc
include <from somewhere>/linux-yocto-custom/features/hello-in-tree.scc

and so on...

So yes .scc .patch and .cfg files are being included, directly, or 
indirectly.

My question is:"What is different from including a .patch file here 
compared to including in from any other recipe?"

I guess the same behavior should apply to .patch, .scc and .cfg files.

Regards,

Robert