| Message ID | 20230512172221.4062722-3-rs@ti.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fix: allow specification of dtb directory | expand |
On Fri, 2023-05-12 at 12:22 -0500, Randolph Sapp via lists.openembedded.org wrote: > From: Randolph Sapp <rs@ti.com> > > Upstream's dtb directory structure has no real standard. They just tend > to idle around the 2/3 directory depth. Recursively search for the > dtb/dtbo files instead of assuming anything. > > Fixes: 04ab57d200 (kernel-devicetree: allow specification of dtb > directory, 2023-05-02) > > Signed-off-by: Randolph Sapp <rs@ti.com> > --- > meta/classes-recipe/kernel-devicetree.bbclass | 21 +++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/meta/classes-recipe/kernel-devicetree.bbclass b/meta/classes-recipe/kernel-devicetree.bbclass > index 831fdd1527..fa6de7a140 100644 > --- a/meta/classes-recipe/kernel-devicetree.bbclass > +++ b/meta/classes-recipe/kernel-devicetree.bbclass > @@ -12,12 +12,21 @@ python () { > d.appendVar("PACKAGES", " ${KERNEL_PACKAGE_NAME}-image-zimage-bundle") > } > > -FILES:${KERNEL_PACKAGE_NAME}-devicetree = " \ > - /${KERNEL_DTBDEST}/*.dtb \ > - /${KERNEL_DTBDEST}/*.dtbo \ > - /${KERNEL_DTBDEST}/*/*.dtb \ > - /${KERNEL_DTBDEST}/*/*.dtbo \ > -" > +# recursivly search for devicetree files > +def find_device_trees(d): > + import glob > + import os > + > + file_paths = [] > + dest_dir = d.getVar('D') > + full_dtb_dir = dest_dir + '/' + d.getVar('KERNEL_DTBDEST') > + > + for file in glob.glob(full_dtb_dir + '/**/*.dtb*', recursive=True): > + file_paths.append('/' + os.path.relpath(file, dest_dir)) > + > + return ' '.join(file_paths) > + > +FILES:${KERNEL_PACKAGE_NAME}-devicetree = "${@find_device_trees(d)}" > FILES:${KERNEL_PACKAGE_NAME}-image-zimage-bundle = "/${KERNEL_IMAGEDEST}/zImage-*.dtb.bin" I suspect this still suffers from a determinism problem as the previous patch did but now it depends on whether the build had occurred or not. I also really don't like resorting to python functions like this, they're horrible for performance overhead. I did wonder if: FILES:${KERNEL_PACKAGE_NAME}-devicetree = "/${KERNEL_DTBDEST}/**/*.dtb*" would work? Also, are there automated tests we should be adding to catch bugs like this? Cheers, Richard
On 5/22/23 03:00, Richard Purdie wrote: > On Fri, 2023-05-12 at 12:22 -0500, Randolph Sapp via > lists.openembedded.org wrote: >> From: Randolph Sapp <rs@ti.com> >> >> Upstream's dtb directory structure has no real standard. They just tend >> to idle around the 2/3 directory depth. Recursively search for the >> dtb/dtbo files instead of assuming anything. >> >> Fixes: 04ab57d200 (kernel-devicetree: allow specification of dtb >> directory, 2023-05-02) >> >> Signed-off-by: Randolph Sapp <rs@ti.com> >> --- >> meta/classes-recipe/kernel-devicetree.bbclass | 21 +++++++++++++------ >> 1 file changed, 15 insertions(+), 6 deletions(-) >> >> diff --git a/meta/classes-recipe/kernel-devicetree.bbclass b/meta/classes-recipe/kernel-devicetree.bbclass >> index 831fdd1527..fa6de7a140 100644 >> --- a/meta/classes-recipe/kernel-devicetree.bbclass >> +++ b/meta/classes-recipe/kernel-devicetree.bbclass >> @@ -12,12 +12,21 @@ python () { >> d.appendVar("PACKAGES", " ${KERNEL_PACKAGE_NAME}-image-zimage-bundle") >> } >> >> -FILES:${KERNEL_PACKAGE_NAME}-devicetree = " \ >> - /${KERNEL_DTBDEST}/*.dtb \ >> - /${KERNEL_DTBDEST}/*.dtbo \ >> - /${KERNEL_DTBDEST}/*/*.dtb \ >> - /${KERNEL_DTBDEST}/*/*.dtbo \ >> -" >> +# recursivly search for devicetree files >> +def find_device_trees(d): >> + import glob >> + import os >> + >> + file_paths = [] >> + dest_dir = d.getVar('D') >> + full_dtb_dir = dest_dir + '/' + d.getVar('KERNEL_DTBDEST') >> + >> + for file in glob.glob(full_dtb_dir + '/**/*.dtb*', recursive=True): >> + file_paths.append('/' + os.path.relpath(file, dest_dir)) >> + >> + return ' '.join(file_paths) >> + >> +FILES:${KERNEL_PACKAGE_NAME}-devicetree = "${@find_device_trees(d)}" >> FILES:${KERNEL_PACKAGE_NAME}-image-zimage-bundle = "/${KERNEL_IMAGEDEST}/zImage-*.dtb.bin" > > I suspect this still suffers from a determinism problem as the previous > patch did but now it depends on whether the build had occurred or not. > > I also really don't like resorting to python functions like this, > they're horrible for performance overhead. I did wonder if: > > FILES:${KERNEL_PACKAGE_NAME}-devicetree = "/${KERNEL_DTBDEST}/**/*.dtb*" > > would work? Tried that first. Unfortunately bitbake doesn't enable the recursive option in it's path globs. That would be too easy... > > Also, are there automated tests we should be adding to catch bugs like > this? Unsure. I haven't looked into automated tests for yocto recipes yet. If you know of some docs for whatever OE-core is doing, feel free to shoot them my way. > > Cheers, > > Richard >
On Mon, 2023-05-22 at 11:22 -0500, Randolph Sapp wrote: > On 5/22/23 03:00, Richard Purdie wrote: > > On Fri, 2023-05-12 at 12:22 -0500, Randolph Sapp via > > lists.openembedded.org wrote: > > > From: Randolph Sapp <rs@ti.com> > > > > > > Upstream's dtb directory structure has no real standard. They just tend > > > to idle around the 2/3 directory depth. Recursively search for the > > > dtb/dtbo files instead of assuming anything. > > > > > > Fixes: 04ab57d200 (kernel-devicetree: allow specification of dtb > > > directory, 2023-05-02) > > > > > > Signed-off-by: Randolph Sapp <rs@ti.com> > > > --- > > > meta/classes-recipe/kernel-devicetree.bbclass | 21 +++++++++++++------ > > > 1 file changed, 15 insertions(+), 6 deletions(-) > > > > > > diff --git a/meta/classes-recipe/kernel-devicetree.bbclass b/meta/classes-recipe/kernel-devicetree.bbclass > > > index 831fdd1527..fa6de7a140 100644 > > > --- a/meta/classes-recipe/kernel-devicetree.bbclass > > > +++ b/meta/classes-recipe/kernel-devicetree.bbclass > > > @@ -12,12 +12,21 @@ python () { > > > d.appendVar("PACKAGES", " ${KERNEL_PACKAGE_NAME}-image-zimage-bundle") > > > } > > > > > > -FILES:${KERNEL_PACKAGE_NAME}-devicetree = " \ > > > - /${KERNEL_DTBDEST}/*.dtb \ > > > - /${KERNEL_DTBDEST}/*.dtbo \ > > > - /${KERNEL_DTBDEST}/*/*.dtb \ > > > - /${KERNEL_DTBDEST}/*/*.dtbo \ > > > -" > > > +# recursivly search for devicetree files > > > +def find_device_trees(d): > > > + import glob > > > + import os > > > + > > > + file_paths = [] > > > + dest_dir = d.getVar('D') > > > + full_dtb_dir = dest_dir + '/' + d.getVar('KERNEL_DTBDEST') > > > + > > > + for file in glob.glob(full_dtb_dir + '/**/*.dtb*', recursive=True): > > > + file_paths.append('/' + os.path.relpath(file, dest_dir)) > > > + > > > + return ' '.join(file_paths) > > > + > > > +FILES:${KERNEL_PACKAGE_NAME}-devicetree = "${@find_device_trees(d)}" > > > FILES:${KERNEL_PACKAGE_NAME}-image-zimage-bundle = "/${KERNEL_IMAGEDEST}/zImage-*.dtb.bin" > > > > I suspect this still suffers from a determinism problem as the previous > > patch did but now it depends on whether the build had occurred or not. > > > > I also really don't like resorting to python functions like this, > > they're horrible for performance overhead. I did wonder if: > > > > FILES:${KERNEL_PACKAGE_NAME}-devicetree = "/${KERNEL_DTBDEST}/**/*.dtb*" > > > > would work? > > Tried that first. Unfortunately bitbake doesn't enable the recursive > option in it's path globs. That would be too easy... I wonder if we should do something like: diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py index edb70daaf17..70040f09e7c 100644 --- a/meta/lib/oe/package.py +++ b/meta/lib/oe/package.py @@ -550,7 +550,7 @@ def files_from_filevars(filevars): f = '.' + f if not f.startswith("./"): f = './' + f - globbed = glob.glob(f) + globbed = glob.glob(f, recursive=True) if globbed: if [ f ] != globbed: files += globbed we now have the right minimum python version for that. My main concern would be potential any performance implications. > > Also, are there automated tests we should be adding to catch bugs like > > this? > > Unsure. I haven't looked into automated tests for yocto recipes yet. If > you know of some docs for whatever OE-core is doing, feel free to shoot > them my way. oe-selftest -r <test-section-name> where you can see the test cases in lib/oeqa/selftest/cases/ but I'm not seeing much for devicetree specifically. We should have some! Cheers, Richard
On 5/22/23 11:59, Richard Purdie wrote: > On Mon, 2023-05-22 at 11:22 -0500, Randolph Sapp wrote: >> On 5/22/23 03:00, Richard Purdie wrote: >>> On Fri, 2023-05-12 at 12:22 -0500, Randolph Sapp via >>> lists.openembedded.org wrote: >>>> From: Randolph Sapp <rs@ti.com> >>>> >>>> Upstream's dtb directory structure has no real standard. They just tend >>>> to idle around the 2/3 directory depth. Recursively search for the >>>> dtb/dtbo files instead of assuming anything. >>>> >>>> Fixes: 04ab57d200 (kernel-devicetree: allow specification of dtb >>>> directory, 2023-05-02) >>>> >>>> Signed-off-by: Randolph Sapp <rs@ti.com> >>>> --- >>>> meta/classes-recipe/kernel-devicetree.bbclass | 21 +++++++++++++------ >>>> 1 file changed, 15 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/meta/classes-recipe/kernel-devicetree.bbclass b/meta/classes-recipe/kernel-devicetree.bbclass >>>> index 831fdd1527..fa6de7a140 100644 >>>> --- a/meta/classes-recipe/kernel-devicetree.bbclass >>>> +++ b/meta/classes-recipe/kernel-devicetree.bbclass >>>> @@ -12,12 +12,21 @@ python () { >>>> d.appendVar("PACKAGES", " ${KERNEL_PACKAGE_NAME}-image-zimage-bundle") >>>> } >>>> >>>> -FILES:${KERNEL_PACKAGE_NAME}-devicetree = " \ >>>> - /${KERNEL_DTBDEST}/*.dtb \ >>>> - /${KERNEL_DTBDEST}/*.dtbo \ >>>> - /${KERNEL_DTBDEST}/*/*.dtb \ >>>> - /${KERNEL_DTBDEST}/*/*.dtbo \ >>>> -" >>>> +# recursivly search for devicetree files >>>> +def find_device_trees(d): >>>> + import glob >>>> + import os >>>> + >>>> + file_paths = [] >>>> + dest_dir = d.getVar('D') >>>> + full_dtb_dir = dest_dir + '/' + d.getVar('KERNEL_DTBDEST') >>>> + >>>> + for file in glob.glob(full_dtb_dir + '/**/*.dtb*', recursive=True): >>>> + file_paths.append('/' + os.path.relpath(file, dest_dir)) >>>> + >>>> + return ' '.join(file_paths) >>>> + >>>> +FILES:${KERNEL_PACKAGE_NAME}-devicetree = "${@find_device_trees(d)}" >>>> FILES:${KERNEL_PACKAGE_NAME}-image-zimage-bundle = "/${KERNEL_IMAGEDEST}/zImage-*.dtb.bin" >>> >>> I suspect this still suffers from a determinism problem as the previous >>> patch did but now it depends on whether the build had occurred or not. >>> >>> I also really don't like resorting to python functions like this, >>> they're horrible for performance overhead. I did wonder if: >>> >>> FILES:${KERNEL_PACKAGE_NAME}-devicetree = "/${KERNEL_DTBDEST}/**/*.dtb*" >>> >>> would work? >> >> Tried that first. Unfortunately bitbake doesn't enable the recursive >> option in it's path globs. That would be too easy... > > I wonder if we should do something like: > > diff --git a/meta/lib/oe/package.py b/meta/lib/oe/package.py > index edb70daaf17..70040f09e7c 100644 > --- a/meta/lib/oe/package.py > +++ b/meta/lib/oe/package.py > @@ -550,7 +550,7 @@ def files_from_filevars(filevars): > f = '.' + f > if not f.startswith("./"): > f = './' + f > - globbed = glob.glob(f) > + globbed = glob.glob(f, recursive=True) > if globbed: > if [ f ] != globbed: > files += globbed > > we now have the right minimum python version for that. My main concern > would be potential any performance implications. This should not be an issue. Recursive=true just enables the interpretation of **, it *shouldn't* add extra recursion to any of the existing recipes... That being said, I've been wrong before. > >>> Also, are there automated tests we should be adding to catch bugs like >>> this? >> >> Unsure. I haven't looked into automated tests for yocto recipes yet. If >> you know of some docs for whatever OE-core is doing, feel free to shoot >> them my way. > > oe-selftest -r <test-section-name> > > where you can see the test cases in lib/oeqa/selftest/cases/ > > but I'm not seeing much for devicetree specifically. We should have > some! > > Cheers, > > Richard > > >
diff --git a/meta/classes-recipe/kernel-devicetree.bbclass b/meta/classes-recipe/kernel-devicetree.bbclass index 831fdd1527..fa6de7a140 100644 --- a/meta/classes-recipe/kernel-devicetree.bbclass +++ b/meta/classes-recipe/kernel-devicetree.bbclass @@ -12,12 +12,21 @@ python () { d.appendVar("PACKAGES", " ${KERNEL_PACKAGE_NAME}-image-zimage-bundle") } -FILES:${KERNEL_PACKAGE_NAME}-devicetree = " \ - /${KERNEL_DTBDEST}/*.dtb \ - /${KERNEL_DTBDEST}/*.dtbo \ - /${KERNEL_DTBDEST}/*/*.dtb \ - /${KERNEL_DTBDEST}/*/*.dtbo \ -" +# recursivly search for devicetree files +def find_device_trees(d): + import glob + import os + + file_paths = [] + dest_dir = d.getVar('D') + full_dtb_dir = dest_dir + '/' + d.getVar('KERNEL_DTBDEST') + + for file in glob.glob(full_dtb_dir + '/**/*.dtb*', recursive=True): + file_paths.append('/' + os.path.relpath(file, dest_dir)) + + return ' '.join(file_paths) + +FILES:${KERNEL_PACKAGE_NAME}-devicetree = "${@find_device_trees(d)}" FILES:${KERNEL_PACKAGE_NAME}-image-zimage-bundle = "/${KERNEL_IMAGEDEST}/zImage-*.dtb.bin" # Generate kernel+devicetree bundle