diff mbox series

[RFC] lib/spdx30_tasks: support directories deployed by image recipes

Message ID 20250113123432.649645-1-igor.opaniuk@foundries.io
State New
Headers show
Series [RFC] lib/spdx30_tasks: support directories deployed by image recipes | expand

Commit Message

Igor Opaniuk Jan. 13, 2025, 12:34 p.m. UTC 
From: Igor Opaniuk <igor.opaniuk@foundries.io>

create_image_spdx() implementation assumes that image is indeed a file.
If image recipe deploys a directory (for example, which contains an
hierarchy of flash artifacts, that is used by SoC vendor-specific
flashing tool) which follows ${IMAGE_NAME}.${IMAGE_TYPE} naming scheme,
create_image_spdx() function will fail after trying to hash a directory:

*** 0002:do_create_image_spdx(d)
     0003:
File: '.../meta/classes-recipe/create-spdx-image-3.0.bbclass', lineno: 48, function: do_create_image_spdx
     0044:addtask do_create_rootfs_spdx_setscene
     0045:
     0046:python do_create_image_spdx() {
     0047:    import oe.spdx30_tasks
 *** 0048:    oe.spdx30_tasks.create_image_spdx(d)
     0049:}
     0050:addtask do_create_image_spdx after do_image_complete do_create_rootfs_spdx before do_build
     0051:SSTATETASKS += "do_create_image_spdx"
...
File: '.../bitbake/lib/bb/utils.py', lineno: 536, function: _hasher
     0532:
     0533:def _hasher(method, filename):
     0534:    import mmap
     0535:
 *** 0536:    with open(filename, "rb") as f:
     0537:        try:
     0538:            with mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ) as mm:
     0539:                for chunk in iter(lambda: mm.read(8192), b''):
     0540:                    method.update(chunk)
Exception: IsADirectoryError: [Errno 21] Is a directory: '...'

Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
---
 meta/lib/oe/spdx30_tasks.py | 33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

Comments

Joshua Watt Jan. 13, 2025, 2:58 p.m. UTC | #1 
On Mon, Jan 13, 2025 at 5:34 AM Igor Opaniuk via
lists.openembedded.org
<igor.opaniuk=foundries.io@lists.openembedded.org> wrote:
>
> From: Igor Opaniuk <igor.opaniuk@foundries.io>
>
> create_image_spdx() implementation assumes that image is indeed a file.
> If image recipe deploys a directory (for example, which contains an
> hierarchy of flash artifacts, that is used by SoC vendor-specific
> flashing tool) which follows ${IMAGE_NAME}.${IMAGE_TYPE} naming scheme,
> create_image_spdx() function will fail after trying to hash a directory:
>
> *** 0002:do_create_image_spdx(d)
>      0003:
> File: '.../meta/classes-recipe/create-spdx-image-3.0.bbclass', lineno: 48, function: do_create_image_spdx
>      0044:addtask do_create_rootfs_spdx_setscene
>      0045:
>      0046:python do_create_image_spdx() {
>      0047:    import oe.spdx30_tasks
>  *** 0048:    oe.spdx30_tasks.create_image_spdx(d)
>      0049:}
>      0050:addtask do_create_image_spdx after do_image_complete do_create_rootfs_spdx before do_build
>      0051:SSTATETASKS += "do_create_image_spdx"
> ...
> File: '.../bitbake/lib/bb/utils.py', lineno: 536, function: _hasher
>      0532:
>      0533:def _hasher(method, filename):
>      0534:    import mmap
>      0535:
>  *** 0536:    with open(filename, "rb") as f:
>      0537:        try:
>      0538:            with mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ) as mm:
>      0539:                for chunk in iter(lambda: mm.read(8192), b''):
>      0540:                    method.update(chunk)
> Exception: IsADirectoryError: [Errno 21] Is a directory: '...'
>
> Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
> ---
>  meta/lib/oe/spdx30_tasks.py | 33 +++++++++++++++++++++------------
>  1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/meta/lib/oe/spdx30_tasks.py b/meta/lib/oe/spdx30_tasks.py
> index c60c97896c..20fb5644e3 100644
> --- a/meta/lib/oe/spdx30_tasks.py
> +++ b/meta/lib/oe/spdx30_tasks.py
> @@ -1072,19 +1072,28 @@ def create_image_spdx(d):
>          for image in task["images"]:
>              image_filename = image["filename"]
>              image_path = image_deploy_dir / image_filename
> -            a = objset.add_root(
> -                oe.spdx30.software_File(
> -                    _id=objset.new_spdxid("image", image_filename),
> -                    creationInfo=objset.doc.creationInfo,
> -                    name=image_filename,
> -                    verifiedUsing=[
> -                        oe.spdx30.Hash(
> -                            algorithm=oe.spdx30.HashAlgorithm.sha256,
> -                            hashValue=bb.utils.sha256_file(image_path),
> -                        )
> -                    ],
> +            if os.path.isdir(image_path):
> +                a = objset.add_root(
> +                    oe.spdx30.software_Package(
> +                        _id=objset.new_spdxid("image-files", image_filename),
> +                        creationInfo=objset.doc.creationInfo,
> +                        name=image_filename,
> +                    )

A "package" is not the correct thing to use here. It should
recursively add all the files (with their checksums). I think reusing
add_package_files() would be best, do you have an example of a recipe
that does this?

> +                )
> +            else:
> +                a = objset.add_root(
> +                    oe.spdx30.software_File(
> +                        _id=objset.new_spdxid("image", image_filename),
> +                        creationInfo=objset.doc.creationInfo,
> +                        name=image_filename,
> +                        verifiedUsing=[
> +                            oe.spdx30.Hash(
> +                                algorithm=oe.spdx30.HashAlgorithm.sha256,
> +                                hashValue=bb.utils.sha256_file(image_path),
> +                            )
> +                        ],
> +                    )
>                  )
> -            )
>              set_purposes(
>                  d, a, "SPDX_IMAGE_PURPOSE:%s" % imagetype, "SPDX_IMAGE_PURPOSE"
>              )
> --
> 2.43.0
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#209717): https://lists.openembedded.org/g/openembedded-core/message/209717
> Mute This Topic: https://lists.openembedded.org/mt/110585648/3616693
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [JPEWhacker@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Igor Opaniuk Jan. 13, 2025, 4:15 p.m. UTC | #2 
Hello Joshua,

On Mon, Jan 13, 2025 at 3:58 PM Joshua Watt <jpewhacker@gmail.com> wrote:
>
> On Mon, Jan 13, 2025 at 5:34 AM Igor Opaniuk via
> lists.openembedded.org
> <igor.opaniuk=foundries.io@lists.openembedded.org> wrote:
> >
> > From: Igor Opaniuk <igor.opaniuk@foundries.io>
> >
> > create_image_spdx() implementation assumes that image is indeed a file.
> > If image recipe deploys a directory (for example, which contains an
> > hierarchy of flash artifacts, that is used by SoC vendor-specific
> > flashing tool) which follows ${IMAGE_NAME}.${IMAGE_TYPE} naming scheme,
> > create_image_spdx() function will fail after trying to hash a directory:
> >
> > *** 0002:do_create_image_spdx(d)
> >      0003:
> > File: '.../meta/classes-recipe/create-spdx-image-3.0.bbclass', lineno: 48, function: do_create_image_spdx
> >      0044:addtask do_create_rootfs_spdx_setscene
> >      0045:
> >      0046:python do_create_image_spdx() {
> >      0047:    import oe.spdx30_tasks
> >  *** 0048:    oe.spdx30_tasks.create_image_spdx(d)
> >      0049:}
> >      0050:addtask do_create_image_spdx after do_image_complete do_create_rootfs_spdx before do_build
> >      0051:SSTATETASKS += "do_create_image_spdx"
> > ...
> > File: '.../bitbake/lib/bb/utils.py', lineno: 536, function: _hasher
> >      0532:
> >      0533:def _hasher(method, filename):
> >      0534:    import mmap
> >      0535:
> >  *** 0536:    with open(filename, "rb") as f:
> >      0537:        try:
> >      0538:            with mmap.mmap(f.fileno(), 0, access=mmap.ACCESS_READ) as mm:
> >      0539:                for chunk in iter(lambda: mm.read(8192), b''):
> >      0540:                    method.update(chunk)
> > Exception: IsADirectoryError: [Errno 21] Is a directory: '...'
> >
> > Signed-off-by: Igor Opaniuk <igor.opaniuk@foundries.io>
> > ---
> >  meta/lib/oe/spdx30_tasks.py | 33 +++++++++++++++++++++------------
> >  1 file changed, 21 insertions(+), 12 deletions(-)
> >
> > diff --git a/meta/lib/oe/spdx30_tasks.py b/meta/lib/oe/spdx30_tasks.py
> > index c60c97896c..20fb5644e3 100644
> > --- a/meta/lib/oe/spdx30_tasks.py
> > +++ b/meta/lib/oe/spdx30_tasks.py
> > @@ -1072,19 +1072,28 @@ def create_image_spdx(d):
> >          for image in task["images"]:
> >              image_filename = image["filename"]
> >              image_path = image_deploy_dir / image_filename
> > -            a = objset.add_root(
> > -                oe.spdx30.software_File(
> > -                    _id=objset.new_spdxid("image", image_filename),
> > -                    creationInfo=objset.doc.creationInfo,
> > -                    name=image_filename,
> > -                    verifiedUsing=[
> > -                        oe.spdx30.Hash(
> > -                            algorithm=oe.spdx30.HashAlgorithm.sha256,
> > -                            hashValue=bb.utils.sha256_file(image_path),
> > -                        )
> > -                    ],
> > +            if os.path.isdir(image_path):
> > +                a = objset.add_root(
> > +                    oe.spdx30.software_Package(
> > +                        _id=objset.new_spdxid("image-files", image_filename),
> > +                        creationInfo=objset.doc.creationInfo,
> > +                        name=image_filename,
> > +                    )
>
> A "package" is not the correct thing to use here. It should
> recursively add all the files (with their checksums). I think reusing
> add_package_files() would be best, do you have an example of a recipe
> that does this?
Thanks for looking into this. Will rework with add_package_files() and send v2.

I stumbled upon this issue when I was reworking custom fstype implementation
in [1] meta-qcom-hwe layer, which creates a tarball with all boot
artifacts that are
"consumed" by QDL tool [2] for flashing a target board.

The idea was to create folder with all ready-to-flash
artifacts besides a tarball, so we can test local builds right away
avoiding the additional "unpack the tarball" step. I'm still not sure
if it's the right approach, as
the fundamental question is if image recipe output can be a directory
(is it even legit?).

[1] https://github.com/qualcomm-linux/meta-qcom-hwe/pull/122
[2] https://github.com/linux-msm/qdl

>
> > +                )
> > +            else:
> > +                a = objset.add_root(
> > +                    oe.spdx30.software_File(
> > +                        _id=objset.new_spdxid("image", image_filename),
> > +                        creationInfo=objset.doc.creationInfo,
> > +                        name=image_filename,
> > +                        verifiedUsing=[
> > +                            oe.spdx30.Hash(
> > +                                algorithm=oe.spdx30.HashAlgorithm.sha256,
> > +                                hashValue=bb.utils.sha256_file(image_path),
> > +                            )
> > +                        ],
> > +                    )
> >                  )
> > -            )
> >              set_purposes(
> >                  d, a, "SPDX_IMAGE_PURPOSE:%s" % imagetype, "SPDX_IMAGE_PURPOSE"
> >              )
> > --
> > 2.43.0
> >
> >
> > -=-=-=-=-=-=-=-=-=-=-=-
> > Links: You receive all messages sent to this group.
> > View/Reply Online (#209717): https://lists.openembedded.org/g/openembedded-core/message/209717
> > Mute This Topic: https://lists.openembedded.org/mt/110585648/3616693
> > Group Owner: openembedded-core+owner@lists.openembedded.org
> > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [JPEWhacker@gmail.com]
> > -=-=-=-=-=-=-=-=-=-=-=-
> >
diff mbox series

Patch

diff --git a/meta/lib/oe/spdx30_tasks.py b/meta/lib/oe/spdx30_tasks.py
index c60c97896c..20fb5644e3 100644
--- a/meta/lib/oe/spdx30_tasks.py
+++ b/meta/lib/oe/spdx30_tasks.py
@@ -1072,19 +1072,28 @@  def create_image_spdx(d):
         for image in task["images"]:
             image_filename = image["filename"]
             image_path = image_deploy_dir / image_filename
-            a = objset.add_root(
-                oe.spdx30.software_File(
-                    _id=objset.new_spdxid("image", image_filename),
-                    creationInfo=objset.doc.creationInfo,
-                    name=image_filename,
-                    verifiedUsing=[
-                        oe.spdx30.Hash(
-                            algorithm=oe.spdx30.HashAlgorithm.sha256,
-                            hashValue=bb.utils.sha256_file(image_path),
-                        )
-                    ],
+            if os.path.isdir(image_path):
+                a = objset.add_root(
+                    oe.spdx30.software_Package(
+                        _id=objset.new_spdxid("image-files", image_filename),
+                        creationInfo=objset.doc.creationInfo,
+                        name=image_filename,
+                    )
+                )
+            else:
+                a = objset.add_root(
+                    oe.spdx30.software_File(
+                        _id=objset.new_spdxid("image", image_filename),
+                        creationInfo=objset.doc.creationInfo,
+                        name=image_filename,
+                        verifiedUsing=[
+                            oe.spdx30.Hash(
+                                algorithm=oe.spdx30.HashAlgorithm.sha256,
+                                hashValue=bb.utils.sha256_file(image_path),
+                            )
+                        ],
+                    )
                 )
-            )
             set_purposes(
                 d, a, "SPDX_IMAGE_PURPOSE:%s" % imagetype, "SPDX_IMAGE_PURPOSE"
             )