diff mbox series

kernel-yocto: enable fetching kernel metadata using file fetcher

Message ID 20240311091533.46869-1-christian.taedcke-oss@weidmueller.com
State New
Headers show
Series kernel-yocto: enable fetching kernel metadata using file fetcher | expand

Commit Message

Taedcke, Christian March 11, 2024, 9:15 a.m. UTC 
From: Christian Taedcke <christian.taedcke@weidmueller.com>

The file fetcher does not support the destsuffix parameter. It does
not rename the supplied folder during the fetch. Therefore the folder
name (i.e. basepath) is added to the kernel-meta search directories.

Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
---
 meta/classes-recipe/kernel-yocto.bbclass | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

Comments

Bruce Ashfield March 11, 2024, 12:46 p.m. UTC | #1 
On Mon, Mar 11, 2024 at 5:15 AM Taedcke, Christian
<christian.taedcke-oss@weidmueller.com> wrote:
>
> From: Christian Taedcke <christian.taedcke@weidmueller.com>
>
> The file fetcher does not support the destsuffix parameter. It does
> not rename the supplied folder during the fetch. Therefore the folder
> name (i.e. basepath) is added to the kernel-meta search directories.
>

What's the benefit ? I need to understand what you are trying to
do that requires this.

Because my first thought is "absolutely not" to changing the way
this works.

What I see below is possibly going to break out of tree BSP
definitions. The combination of kmeta and destsuffix really
isn't there to support the git fetcher, it is a keyword combination
that I used to indicate a meta-data repository that unfortunately
overlaps with the other fetcher parameters.

The reality is that I won't be able to fully review and test
this until after the upcoming release is done, since I need to
also see about pulling together some regression tests and
submitting them at the same time.

Bruce

> Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
> ---
>  meta/classes-recipe/kernel-yocto.bbclass | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/meta/classes-recipe/kernel-yocto.bbclass b/meta/classes-recipe/kernel-yocto.bbclass
> index 9a86616dad..3cbdf21df8 100644
> --- a/meta/classes-recipe/kernel-yocto.bbclass
> +++ b/meta/classes-recipe/kernel-yocto.bbclass
> @@ -62,8 +62,8 @@ def find_sccs(d):
>
>      return sources_list
>
> -# check the SRC_URI for "kmeta" type'd git repositories. Return the name of
> -# the repository as it will be found in WORKDIR
> +# check the SRC_URI for "kmeta" type'd git repositories and directories. Return
> +# the name of the repository or directory as it will be found in WORKDIR
>  def find_kernel_feature_dirs(d):
>      feature_dirs=[]
>      fetch = bb.fetch2.Fetch([], d)
> @@ -71,13 +71,20 @@ def find_kernel_feature_dirs(d):
>          urldata = fetch.ud[url]
>          parm = urldata.parm
>          type=""
> +        destdir = ""
>          if "type" in parm:
>              type = parm["type"]
> -        if "destsuffix" in parm:
> -            destdir = parm["destsuffix"]
> -            if type == "kmeta":
> -                feature_dirs.append(destdir)
> -
> +        if type == "kmeta":
> +            if urldata.type == "git":
> +                if "destsuffix" in parm:
> +                    destdir = parm["destsuffix"]
> +            elif urldata.type == "file":
> +                destdir = urldata.basepath
> +            else:
> +                bb.fatal( "unsupported fetcher '%s' used for kernel-metadata" % (urldata.type) )
> +        if destdir:
> +            feature_dirs.append(destdir)
> +
>      return feature_dirs
>
>  # find the master/machine source branch. In the same way that the fetcher proceses
> --
> 2.34.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#196928): https://lists.openembedded.org/g/openembedded-core/message/196928
> Mute This Topic: https://lists.openembedded.org/mt/104859707/1050810
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Taedcke, Christian March 11, 2024, 1:22 p.m. UTC | #2 
On 11.03.2024 13:46, Bruce Ashfield via lists.openembedded.org wrote:
> On Mon, Mar 11, 2024 at 5:15 AM Taedcke, Christian
> <christian.taedcke-oss@weidmueller.com> wrote:
>>
>> From: Christian Taedcke <christian.taedcke@weidmueller.com>
>>
>> The file fetcher does not support the destsuffix parameter. It does
>> not rename the supplied folder during the fetch. Therefore the folder
>> name (i.e. basepath) is added to the kernel-meta search directories.
>>
> 
> What's the benefit ? I need to understand what you are trying to
> do that requires this.

In a BSP layer for a custom machine we use the linux-yocto kernel.
In the BSP layer the file /recipes-kernel/linux/linux-yocto_6.1.bbappend 
adds a local folder containing kernel metatdata to the SRC_URI:

SRC_URI:append:my-machine = "file://my-meta;type=kmeta"

The folder my-meta is a subfolder: 
/recipes-kernel/linux/linux-yocto/my-machine/my-meta

Without this patch we would have to write:
SRC_URI:append:my-machine = " file://my-meta;type=kmeta;destsuffix=my-meta"

When using a file fetcher for kernel metatdata without this patch the 
destsuffix property would always have to be the same as the folder name 
itself. With this patch we do not need this duplication.

> 
> Because my first thought is "absolutely not" to changing the way
> this works.
> 
> What I see below is possibly going to break out of tree BSP
> definitions. The combination of kmeta and destsuffix really
> isn't there to support the git fetcher, it is a keyword combination
> that I used to indicate a meta-data repository that unfortunately
> overlaps with the other fetcher parameters.

Ok, that was not clear to me. The comment above the function 
find_kernel_feature_dirs() indicates that the returned value has 
something to do with how/where the fetcher stores the provided metadata:

-> Return the name of the repository as it will be found in WORKDIR

I think there are 2 ways forward:
1. Rewrite this patch so it does not affect out of tree BSPs (remove 
exception, still use destsuffix for other fetchers), or
2. Drop this patch completely

> 
> The reality is that I won't be able to fully review and test
> this until after the upcoming release is done, since I need to
> also see about pulling together some regression tests and
> submitting them at the same time.

No problem, i am just in the process of upstreaming our in-house 
patches. This is not time-critical. We can continue the discussion after 
the release.

Regards,
Christian

> 
> Bruce
> 
>> Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
>> ---
>>   meta/classes-recipe/kernel-yocto.bbclass | 21 ++++++++++++++-------
>>   1 file changed, 14 insertions(+), 7 deletions(-)
>>
>> diff --git a/meta/classes-recipe/kernel-yocto.bbclass b/meta/classes-recipe/kernel-yocto.bbclass
>> index 9a86616dad..3cbdf21df8 100644
>> --- a/meta/classes-recipe/kernel-yocto.bbclass
>> +++ b/meta/classes-recipe/kernel-yocto.bbclass
>> @@ -62,8 +62,8 @@ def find_sccs(d):
>>
>>       return sources_list
>>
>> -# check the SRC_URI for "kmeta" type'd git repositories. Return the name of
>> -# the repository as it will be found in WORKDIR
>> +# check the SRC_URI for "kmeta" type'd git repositories and directories. Return
>> +# the name of the repository or directory as it will be found in WORKDIR
>>   def find_kernel_feature_dirs(d):
>>       feature_dirs=[]
>>       fetch = bb.fetch2.Fetch([], d)
>> @@ -71,13 +71,20 @@ def find_kernel_feature_dirs(d):
>>           urldata = fetch.ud[url]
>>           parm = urldata.parm
>>           type=""
>> +        destdir = ""
>>           if "type" in parm:
>>               type = parm["type"]
>> -        if "destsuffix" in parm:
>> -            destdir = parm["destsuffix"]
>> -            if type == "kmeta":
>> -                feature_dirs.append(destdir)
>> -
>> +        if type == "kmeta":
>> +            if urldata.type == "git":
>> +                if "destsuffix" in parm:
>> +                    destdir = parm["destsuffix"]
>> +            elif urldata.type == "file":
>> +                destdir = urldata.basepath
>> +            else:
>> +                bb.fatal( "unsupported fetcher '%s' used for kernel-metadata" % (urldata.type) )
>> +        if destdir:
>> +            feature_dirs.append(destdir)
>> +
>>       return feature_dirs
>>
>>   # find the master/machine source branch. In the same way that the fetcher proceses
>> --
>> 2.34.1
>>
>>
>>
>>
> 
> 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#196934): https://lists.openembedded.org/g/openembedded-core/message/196934
> Mute This Topic: https://lists.openembedded.org/mt/104859707/7654653
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [christian.taedcke-oss@weidmueller.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Bruce Ashfield March 11, 2024, 2:25 p.m. UTC | #3 
On Mon, Mar 11, 2024 at 9:22 AM Taedcke, Christian
<christian.taedcke-oss@weidmueller.com> wrote:
>
>
>
> On 11.03.2024 13:46, Bruce Ashfield via lists.openembedded.org wrote:
> > On Mon, Mar 11, 2024 at 5:15 AM Taedcke, Christian
> > <christian.taedcke-oss@weidmueller.com> wrote:
> >>
> >> From: Christian Taedcke <christian.taedcke@weidmueller.com>
> >>
> >> The file fetcher does not support the destsuffix parameter. It does
> >> not rename the supplied folder during the fetch. Therefore the folder
> >> name (i.e. basepath) is added to the kernel-meta search directories.
> >>
> >
> > What's the benefit ? I need to understand what you are trying to
> > do that requires this.
>
> In a BSP layer for a custom machine we use the linux-yocto kernel.
> In the BSP layer the file /recipes-kernel/linux/linux-yocto_6.1.bbappend
> adds a local folder containing kernel metatdata to the SRC_URI:

Definitely a valid use-case!

>
> SRC_URI:append:my-machine = "file://my-meta;type=kmeta"
>
> The folder my-meta is a subfolder:
> /recipes-kernel/linux/linux-yocto/my-machine/my-meta
>
> Without this patch we would have to write:
> SRC_URI:append:my-machine = " file://my-meta;type=kmeta;destsuffix=my-meta"
>
> When using a file fetcher for kernel metatdata without this patch the
> destsuffix property would always have to be the same as the folder name
> itself. With this patch we do not need this duplication.

That's the (non-obvious) intentional part of the way that I ended
up implementing the identification of secondary / out of tree
meta-data files or directories that should be treated as feature
directories.

Initially I had more strictly enforced that the destsuffix and
folder name had to match. When it became not as strictly
required, it stayed around to avoid picking up invalid
meta data collections that were unfortunately named in the
SRC_URI. (I'm the one with the unfortunately named
meta-data that wasn't supposed to be pulled in as a kernel
meta directory :))

>
> >
> > Because my first thought is "absolutely not" to changing the way
> > this works.
> >
> > What I see below is possibly going to break out of tree BSP
> > definitions. The combination of kmeta and destsuffix really
> > isn't there to support the git fetcher, it is a keyword combination
> > that I used to indicate a meta-data repository that unfortunately
> > overlaps with the other fetcher parameters.
>
> Ok, that was not clear to me. The comment above the function
> find_kernel_feature_dirs() indicates that the returned value has
> something to do with how/where the fetcher stores the provided metadata:
>
> -> Return the name of the repository as it will be found in WORKDIR

The comments may be out of sync with how it currently works,
there was quite a bit of churn on this during the "early days".

You can see this was also covered recently on the docs mailing
list under: [docs] [PATCH] manuals: kernel-dev: Fix BSP in recipe space

>
> I think there are 2 ways forward:
> 1. Rewrite this patch so it does not affect out of tree BSPs (remove
> exception, still use destsuffix for other fetchers), or

Ideally it needs to still process destsuffix in file mode, but add
a new path that it can work without it being explicitly supplied.

> 2. Drop this patch completely
>
> >
> > The reality is that I won't be able to fully review and test
> > this until after the upcoming release is done, since I need to
> > also see about pulling together some regression tests and
> > submitting them at the same time.
>
> No problem, i am just in the process of upstreaming our in-house
> patches. This is not time-critical. We can continue the discussion after
> the release.

I'll pick this up in a couple of weeks (feel free to ping again), since
I really do need to run quite a few tests and then make them into
some sort of sanity suite .. since I can't really comment that this
might break something, without a way to actually test and show that
it is, or isn't the case.

Bruce

>
> Regards,
> Christian
>
> >
> > Bruce
> >
> >> Signed-off-by: Christian Taedcke <christian.taedcke@weidmueller.com>
> >> ---
> >>   meta/classes-recipe/kernel-yocto.bbclass | 21 ++++++++++++++-------
> >>   1 file changed, 14 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/meta/classes-recipe/kernel-yocto.bbclass b/meta/classes-recipe/kernel-yocto.bbclass
> >> index 9a86616dad..3cbdf21df8 100644
> >> --- a/meta/classes-recipe/kernel-yocto.bbclass
> >> +++ b/meta/classes-recipe/kernel-yocto.bbclass
> >> @@ -62,8 +62,8 @@ def find_sccs(d):
> >>
> >>       return sources_list
> >>
> >> -# check the SRC_URI for "kmeta" type'd git repositories. Return the name of
> >> -# the repository as it will be found in WORKDIR
> >> +# check the SRC_URI for "kmeta" type'd git repositories and directories. Return
> >> +# the name of the repository or directory as it will be found in WORKDIR
> >>   def find_kernel_feature_dirs(d):
> >>       feature_dirs=[]
> >>       fetch = bb.fetch2.Fetch([], d)
> >> @@ -71,13 +71,20 @@ def find_kernel_feature_dirs(d):
> >>           urldata = fetch.ud[url]
> >>           parm = urldata.parm
> >>           type=""
> >> +        destdir = ""
> >>           if "type" in parm:
> >>               type = parm["type"]
> >> -        if "destsuffix" in parm:
> >> -            destdir = parm["destsuffix"]
> >> -            if type == "kmeta":
> >> -                feature_dirs.append(destdir)
> >> -
> >> +        if type == "kmeta":
> >> +            if urldata.type == "git":
> >> +                if "destsuffix" in parm:
> >> +                    destdir = parm["destsuffix"]
> >> +            elif urldata.type == "file":
> >> +                destdir = urldata.basepath
> >> +            else:
> >> +                bb.fatal( "unsupported fetcher '%s' used for kernel-metadata" % (urldata.type) )
> >> +        if destdir:
> >> +            feature_dirs.append(destdir)
> >> +
> >>       return feature_dirs
> >>
> >>   # find the master/machine source branch. In the same way that the fetcher proceses
> >> --
> >> 2.34.1
> >>
> >>
> >>
> >>
> >
> >
> >
> >
> >
> >
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#196937): https://lists.openembedded.org/g/openembedded-core/message/196937
> Mute This Topic: https://lists.openembedded.org/mt/104859707/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
diff mbox series

Patch

diff --git a/meta/classes-recipe/kernel-yocto.bbclass b/meta/classes-recipe/kernel-yocto.bbclass
index 9a86616dad..3cbdf21df8 100644
--- a/meta/classes-recipe/kernel-yocto.bbclass
+++ b/meta/classes-recipe/kernel-yocto.bbclass
@@ -62,8 +62,8 @@  def find_sccs(d):
 
     return sources_list
 
-# check the SRC_URI for "kmeta" type'd git repositories. Return the name of
-# the repository as it will be found in WORKDIR
+# check the SRC_URI for "kmeta" type'd git repositories and directories. Return
+# the name of the repository or directory as it will be found in WORKDIR
 def find_kernel_feature_dirs(d):
     feature_dirs=[]
     fetch = bb.fetch2.Fetch([], d)
@@ -71,13 +71,20 @@  def find_kernel_feature_dirs(d):
         urldata = fetch.ud[url]
         parm = urldata.parm
         type=""
+        destdir = ""
         if "type" in parm:
             type = parm["type"]
-        if "destsuffix" in parm:
-            destdir = parm["destsuffix"]
-            if type == "kmeta":
-                feature_dirs.append(destdir)
-	    
+        if type == "kmeta":
+            if urldata.type == "git":
+                if "destsuffix" in parm:
+                    destdir = parm["destsuffix"]
+            elif urldata.type == "file":
+                destdir = urldata.basepath
+            else:
+                bb.fatal( "unsupported fetcher '%s' used for kernel-metadata" % (urldata.type) )
+        if destdir:
+            feature_dirs.append(destdir)
+
     return feature_dirs
 
 # find the master/machine source branch. In the same way that the fetcher proceses