diff mbox series

[5/6] create-spdx-{2.2,3.0}: support SPDX include source for work-share directory

Message ID 20241026032519.1968518-6-hongxu.jia@windriver.com
State Accepted, archived
Commit 64454b1956a9b50d6c89a3f3d7c594c1272cb289
Headers show
Series Support SPDX include source for work-share directory | expand

Commit Message

Hongxu Jia Oct. 26, 2024, 3:25 a.m. UTC
Originally, while SPDX_INCLUDE_SOURCES = "1" [1], there is bug in scan
for gcc, libgcc in which the sources locates in work-share directory.
Copy source from ${WORKDIR} to ${SPDXWORK} did not satisfy the situation
while ${S} was not included in ${WORKDIR}

This commit aim to support SPDX include source for work-share directory

1. If is_work_shared_spdx, Copy source from ${S} to ${SPDXWORK},
normally the dest dir in ${SPDXWORK} has the same basename dir of ${S};
but for kernel source, rename basename dir 'kernel-source' to ${BP} (${BPN}-${PV})

2. For SPDX source copy, do hard link copy to save copy time

3. Move do_patch to no work shared situation along with do_unpack

4. Tweak task do_create_spdx dependencies to assure the patched source
in work share is ready for SPDX source copy

5. Remove bb.data.inherits_class('kernel', d) from is_work_shared_spdx,
the kernel source locates in 'work-shared', test kernel.bbclass is not
necessary

[1] https://docs.yoctoproject.org/dev/ref-manual/variables.html#term-SPDX_INCLUDE_SOURCES

Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
---
 meta/classes/create-spdx-2.2.bbclass | 20 ++++++++++++++
 meta/classes/create-spdx-3.0.bbclass | 19 +++++++++++++
 meta/lib/oe/spdx_common.py           | 41 +++++++++++-----------------
 3 files changed, 55 insertions(+), 25 deletions(-)

Comments

Joshua Watt Oct. 28, 2024, 9:12 p.m. UTC | #1
On Fri, Oct 25, 2024 at 9:25 PM Hongxu Jia <hongxu.jia@windriver.com> wrote:
>
> Originally, while SPDX_INCLUDE_SOURCES = "1" [1], there is bug in scan
> for gcc, libgcc in which the sources locates in work-share directory.
> Copy source from ${WORKDIR} to ${SPDXWORK} did not satisfy the situation
> while ${S} was not included in ${WORKDIR}
>
> This commit aim to support SPDX include source for work-share directory
>
> 1. If is_work_shared_spdx, Copy source from ${S} to ${SPDXWORK},
> normally the dest dir in ${SPDXWORK} has the same basename dir of ${S};
> but for kernel source, rename basename dir 'kernel-source' to ${BP} (${BPN}-${PV})
>
> 2. For SPDX source copy, do hard link copy to save copy time
>
> 3. Move do_patch to no work shared situation along with do_unpack
>
> 4. Tweak task do_create_spdx dependencies to assure the patched source
> in work share is ready for SPDX source copy
>
> 5. Remove bb.data.inherits_class('kernel', d) from is_work_shared_spdx,
> the kernel source locates in 'work-shared', test kernel.bbclass is not
> necessary
>
> [1] https://docs.yoctoproject.org/dev/ref-manual/variables.html#term-SPDX_INCLUDE_SOURCES
>
> Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
> ---
>  meta/classes/create-spdx-2.2.bbclass | 20 ++++++++++++++
>  meta/classes/create-spdx-3.0.bbclass | 19 +++++++++++++
>  meta/lib/oe/spdx_common.py           | 41 +++++++++++-----------------
>  3 files changed, 55 insertions(+), 25 deletions(-)
>
> diff --git a/meta/classes/create-spdx-2.2.bbclass b/meta/classes/create-spdx-2.2.bbclass
> index cd1d6819bf..900595a7e9 100644
> --- a/meta/classes/create-spdx-2.2.bbclass
> +++ b/meta/classes/create-spdx-2.2.bbclass
> @@ -947,3 +947,23 @@ def combine_spdx(d, rootfs_name, rootfs_deploydir, rootfs_spdxid, packages, spdx
>              tar.addfile(info, fileobj=index_str)
>
>  combine_spdx[vardepsexclude] += "BB_NUMBER_THREADS SPDX_MULTILIB_SSTATE_ARCHS"
> +
> +python () {
> +    import oe.spdx_common
> +
> +    # Assure the patched source in work share is ready for SPDX source copy
> +    if oe.spdx_common.is_work_shared_spdx(d) and \
> +       d.getVar("SPDX_INCLUDE_SOURCES") == "1" and \
> +       oe.spdx_common.process_sources(d):
> +        pn = d.getVar('PN')
> +        # There is a corner case with "gcc-source-${PV}" recipes, they use "do_preconfigure"
> +        # to modify source file after do_patch
> +        if oe.spdx_common.has_task(d, "do_preconfigure"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_preconfigure' % pn)
> +        elif oe.spdx_common.has_task(d, "do_patch"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_patch' % pn)
> +        # There is a corner case with "gcc-cross-x86_64" recipes, it has "do_configure"
> +        # but no do_patch
> +        elif oe.spdx_common.has_task(d, "do_configure"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_configure' % pn)
> +}

For some reason, I think I'd prefer something like:

do_create_spdx[depends] = "${@extra_create_spdx_deps(d)}"

def extra_create_spdx_deps(d):
   if BLAH:
     return "do_configure"

  # ETC.

  return ""

But I don't know why. Perhaps Richard can weigh in on if one is better
than the other?

> diff --git a/meta/classes/create-spdx-3.0.bbclass b/meta/classes/create-spdx-3.0.bbclass
> index 5f0590198f..284e0ff91c 100644
> --- a/meta/classes/create-spdx-3.0.bbclass
> +++ b/meta/classes/create-spdx-3.0.bbclass
> @@ -190,3 +190,22 @@ python spdx30_build_started_handler () {
>  addhandler spdx30_build_started_handler
>  spdx30_build_started_handler[eventmask] = "bb.event.BuildStarted"
>
> +python () {
> +    import oe.spdx_common
> +
> +    # Assure the patched source in work share is ready for SPDX source copy
> +    if oe.spdx_common.is_work_shared_spdx(d) and \
> +       d.getVar("SPDX_INCLUDE_SOURCES") == "1" and \
> +       oe.spdx_common.process_sources(d):
> +        pn = d.getVar('PN')
> +        # There is a corner case with "gcc-source-${PV}" recipes, they use "do_preconfigure"
> +        # to modify source file after do_patch
> +        if oe.spdx_common.has_task(d, "do_preconfigure"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_preconfigure' % pn)
> +        elif oe.spdx_common.has_task(d, "do_patch"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_patch' % pn)
> +        # There is a corner case with "gcc-cross-x86_64" recipes, it has "do_configure"
> +        # but no do_patch
> +        elif oe.spdx_common.has_task(d, "do_configure"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_configure' % pn)
> +}
> diff --git a/meta/lib/oe/spdx_common.py b/meta/lib/oe/spdx_common.py
> index 1ea55419ae..ab037b4088 100644
> --- a/meta/lib/oe/spdx_common.py
> +++ b/meta/lib/oe/spdx_common.py
> @@ -38,7 +38,7 @@ def extract_licenses(filename):
>
>
>  def is_work_shared_spdx(d):
> -    return bb.data.inherits_class("kernel", d) or ("work-shared" in d.getVar("WORKDIR"))
> +    return 'work-shared' in d.getVar('S')

Should the "/" be included? (e.g. return "/work-shared/" in d.getVar("S") )

>
>
>  def load_spdx_license_data(d):
> @@ -74,11 +74,6 @@ def process_sources(d):
>      if d.getVar("PN") == "shadow-sysroot":
>          return False
>
> -    # We just archive gcc-source for all the gcc related recipes
> -    if d.getVar("BPN") in ["gcc", "libgcc"]:
> -        bb.debug(1, "spdx: There is bug in scan of %s is, do nothing" % pn)
> -        return False
> -
>      return True
>
>
> @@ -190,34 +185,28 @@ def get_patched_src(d):
>              bb.utils.mkdirhier(d.getVar("B"))
>
>              bb.build.exec_func("do_unpack", d)
> -        # Copy source of kernel to spdx_workdir
> +
> +            if d.getVar("SRC_URI") != "":
> +                bb.build.exec_func("do_patch", d)
> +
> +        # Copy source from work-share to spdx_workdir
>          if is_work_shared_spdx(d):
> -            share_src = d.getVar("WORKDIR")
> +            share_src = d.getVar('S')
>              d.setVar("WORKDIR", spdx_workdir)
>              d.setVar("STAGING_DIR_NATIVE", spdx_sysroot_native)
> +            # Copy source to ${SPDXWORK}, same basename dir of ${S};
>              src_dir = (
>                  spdx_workdir
>                  + "/"
> -                + d.getVar("PN")
> -                + "-"
> -                + d.getVar("PV")
> -                + "-"
> -                + d.getVar("PR")
> +                + os.path.basename(share_src)
>              )
> -            bb.utils.mkdirhier(src_dir)
> +            # For kernel souce, rename suffix dir 'kernel-source'
> +            # to ${BP} (${BPN}-${PV})
>              if bb.data.inherits_class("kernel", d):
> -                share_src = d.getVar("STAGING_KERNEL_DIR")
> -            cmd_copy_share = "cp -rf " + share_src + "/* " + src_dir + "/"
> -            cmd_copy_shared_res = os.popen(cmd_copy_share).read()
> -            bb.note("cmd_copy_shared_result = " + cmd_copy_shared_res)
> -
> -            git_path = src_dir + "/.git"
> -            if os.path.exists(git_path):
> -                shutils.rmtree(git_path)
> +                src_dir = spdx_workdir + "/" + d.getVar('BP')
>
> -        # Make sure gcc and kernel sources are patched only once
> -        if not (d.getVar("SRC_URI") == "" or is_work_shared_spdx(d)):
> -            bb.build.exec_func("do_patch", d)
> +            bb.note(f"copyhardlinktree {share_src} to {src_dir}")
> +            oe.path.copyhardlinktree(share_src, src_dir)
>
>          # Some userland has no source.
>          if not os.path.exists(spdx_workdir):
> @@ -225,6 +214,8 @@ def get_patched_src(d):
>      finally:
>          d.setVar("WORKDIR", workdir)
>
> +def has_task(d, task):
> +    return bool(d.getVarFlag(task, "task", False)) and not bool(d.getVarFlag(task, "noexec", False))
>
>  def fetch_data_to_uri(fd, name):
>      """
> --
> 2.25.1
>
Hongxu Jia Oct. 29, 2024, 2:59 a.m. UTC | #2
Base on your comments, I found we should use create_spdx_source_deps to maintain
the dependency of do_create_spdx for  SPDX_INCLUDE_SOURCES = "1"

Move create_spdx_source_deps to meta/lib/oe/spdx_common.py for  both
of SPDX 2.2 and SPDX 3.0, use ${@oe.spdx_common.create_spdx_source_deps(d)}
to instead of ${create_spdx_source_deps(d)}. Assure the patched source
in work share is ready for SPDX source copy, like following:

--- a/meta/classes/create-spdx-2.2.bbclass
+++ b/meta/classes/create-spdx-2.2.bbclass
@@ -588,8 +588,11 @@ python do_create_spdx() {
             oe.sbom.write_doc(d, package_doc, pkg_arch, "packages", indent=get_json_indent(d))
 }
 do_create_spdx[vardepsexclude] += "BB_NUMBER_THREADS"
-# NOTE: depending on do_unpack is a hack that is necessary to get it's dependencies for archive the source
-addtask do_create_spdx after do_package do_packagedata do_unpack do_collect_spdx_deps before do_populate_sdk do_build do_rm_work
+addtask do_create_spdx after \
+    do_package do_packagedata \
+    do_collect_spdx_deps \
+    ${@oe.spdx_common.create_spdx_source_deps(d)} \
+    before do_populate_sdk do_build do_rm_work

 SSTATETASKS += "do_create_spdx"
 do_create_spdx[sstate-inputdirs] = "${SPDXDEPLOY}"
diff --git a/meta/classes/create-spdx-3.0.bbclass b/meta/classes/create-spdx-3.0.bbclass
index 5f0590198f..d615edcf57 100644
--- a/meta/classes/create-spdx-3.0.bbclass
+++ b/meta/classes/create-spdx-3.0.bbclass
@@ -132,22 +132,9 @@ addtask do_create_spdx after \
     do_collect_spdx_deps \
     do_deploy_source_date_epoch \
     do_populate_sysroot do_package do_packagedata \
-    ${create_spdx_source_deps(d)} \
+    ${@oe.spdx_common.create_spdx_source_deps(d)} \
     before do_populate_sdk do_populate_sdk_ext do_build do_rm_work

-def create_spdx_source_deps(d):
-    deps = []
-    if d.getVar("SPDX_INCLUDE_SOURCES") == "1":
-        deps.extend([
-            # do_unpack is a hack for now; we only need it to get the
-            # dependencies do_unpack already has so we can extract the source
-            # ourselves
-            "do_unpack",
-            # For kernel source code
-            "do_shared_workdir",
-        ])
-    return " ".join(deps)

--- a/meta/lib/oe/spdx_common.py
+++ b/meta/lib/oe/spdx_common.py
@@ -37,8 +37,35 @@ def extract_licenses(filename):
     return []


+def create_spdx_source_deps(d):
+    deps = []
+    if d.getVar("SPDX_INCLUDE_SOURCES") == "1":
+        deps.extend([
+            # do_unpack is a hack for now; we only need it to get the
+            # dependencies do_unpack already has so we can extract the source
+            # ourselves
+            "do_unpack",
+        ])
+
+        # Assure the patched source in work share is ready for SPDX source copy
+        if is_work_shared_spdx(d) and  process_sources(d):
+            deps.extend([
+                # For kernel source code
+                "do_shared_workdir",
+                # For gcc-source-${PV}
+                "do_preconfigure",
+                # For gcc-cross-x86_64
+                "do_configure",
+                "do_patch",
+            ])
+
+
+    return " ".join(deps)

//Hongxu


On Fri, Oct 25, 2024 at 9:25 PM Hongxu Jia <hongxu.jia@windriver.com> wrote:
>
> Originally, while SPDX_INCLUDE_SOURCES = "1" [1], there is bug in scan
> for gcc, libgcc in which the sources locates in work-share directory.
> Copy source from ${WORKDIR} to ${SPDXWORK} did not satisfy the situation
> while ${S} was not included in ${WORKDIR}
>
> This commit aim to support SPDX include source for work-share directory
>
> 1. If is_work_shared_spdx, Copy source from ${S} to ${SPDXWORK},
> normally the dest dir in ${SPDXWORK} has the same basename dir of ${S};
> but for kernel source, rename basename dir 'kernel-source' to ${BP} (${BPN}-${PV})
>
> 2. For SPDX source copy, do hard link copy to save copy time
>
> 3. Move do_patch to no work shared situation along with do_unpack
>
> 4. Tweak task do_create_spdx dependencies to assure the patched source
> in work share is ready for SPDX source copy
>
> 5. Remove bb.data.inherits_class('kernel', d) from is_work_shared_spdx,
> the kernel source locates in 'work-shared', test kernel.bbclass is not
> necessary
>
> [1] https://docs.yoctoproject.org/dev/ref-manual/variables.html#term-SPDX_INCLUDE_SOURCES
>
> Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
> ---
>  meta/classes/create-spdx-2.2.bbclass | 20 ++++++++++++++
>  meta/classes/create-spdx-3.0.bbclass | 19 +++++++++++++
>  meta/lib/oe/spdx_common.py           | 41 +++++++++++-----------------
>  3 files changed, 55 insertions(+), 25 deletions(-)
>
> diff --git a/meta/classes/create-spdx-2.2.bbclass b/meta/classes/create-spdx-2.2.bbclass
> index cd1d6819bf..900595a7e9 100644
> --- a/meta/classes/create-spdx-2.2.bbclass
> +++ b/meta/classes/create-spdx-2.2.bbclass
> @@ -947,3 +947,23 @@ def combine_spdx(d, rootfs_name, rootfs_deploydir, rootfs_spdxid, packages, spdx
>              tar.addfile(info, fileobj=index_str)
>
>  combine_spdx[vardepsexclude] += "BB_NUMBER_THREADS SPDX_MULTILIB_SSTATE_ARCHS"
> +
> +python () {
> +    import oe.spdx_common
> +
> +    # Assure the patched source in work share is ready for SPDX source copy
> +    if oe.spdx_common.is_work_shared_spdx(d) and \
> +       d.getVar("SPDX_INCLUDE_SOURCES") == "1" and \
> +       oe.spdx_common.process_sources(d):
> +        pn = d.getVar('PN')
> +        # There is a corner case with "gcc-source-${PV}" recipes, they use "do_preconfigure"
> +        # to modify source file after do_patch
> +        if oe.spdx_common.has_task(d, "do_preconfigure"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_preconfigure' % pn)
> +        elif oe.spdx_common.has_task(d, "do_patch"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_patch' % pn)
> +        # There is a corner case with "gcc-cross-x86_64" recipes, it has "do_configure"
> +        # but no do_patch
> +        elif oe.spdx_common.has_task(d, "do_configure"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_configure' % pn)
> +}

For some reason, I think I'd prefer something like:

do_create_spdx[depends] = "${@extra_create_spdx_deps(d)}"

def extra_create_spdx_deps(d):
   if BLAH:
     return "do_configure"

  # ETC.

  return ""

But I don't know why. Perhaps Richard can weigh in on if one is better
than the other?

> diff --git a/meta/classes/create-spdx-3.0.bbclass b/meta/classes/create-spdx-3.0.bbclass
> index 5f0590198f..284e0ff91c 100644
> --- a/meta/classes/create-spdx-3.0.bbclass
> +++ b/meta/classes/create-spdx-3.0.bbclass
> @@ -190,3 +190,22 @@ python spdx30_build_started_handler () {
>  addhandler spdx30_build_started_handler
>  spdx30_build_started_handler[eventmask] = "bb.event.BuildStarted"
>
> +python () {
> +    import oe.spdx_common
> +
> +    # Assure the patched source in work share is ready for SPDX source copy
> +    if oe.spdx_common.is_work_shared_spdx(d) and \
> +       d.getVar("SPDX_INCLUDE_SOURCES") == "1" and \
> +       oe.spdx_common.process_sources(d):
> +        pn = d.getVar('PN')
> +        # There is a corner case with "gcc-source-${PV}" recipes, they use "do_preconfigure"
> +        # to modify source file after do_patch
> +        if oe.spdx_common.has_task(d, "do_preconfigure"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_preconfigure' % pn)
> +        elif oe.spdx_common.has_task(d, "do_patch"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_patch' % pn)
> +        # There is a corner case with "gcc-cross-x86_64" recipes, it has "do_configure"
> +        # but no do_patch
> +        elif oe.spdx_common.has_task(d, "do_configure"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_configure' % pn)
> +}
> diff --git a/meta/lib/oe/spdx_common.py b/meta/lib/oe/spdx_common.py
> index 1ea55419ae..ab037b4088 100644
> --- a/meta/lib/oe/spdx_common.py
> +++ b/meta/lib/oe/spdx_common.py
> @@ -38,7 +38,7 @@ def extract_licenses(filename):
>
>
>  def is_work_shared_spdx(d):
> -    return bb.data.inherits_class("kernel", d) or ("work-shared" in d.getVar("WORKDIR"))
> +    return 'work-shared' in d.getVar('S')

Should the "/" be included? (e.g. return "/work-shared/" in d.getVar("S") )

>
>
>  def load_spdx_license_data(d):
> @@ -74,11 +74,6 @@ def process_sources(d):
>      if d.getVar("PN") == "shadow-sysroot":
>          return False
>
> -    # We just archive gcc-source for all the gcc related recipes
> -    if d.getVar("BPN") in ["gcc", "libgcc"]:
> -        bb.debug(1, "spdx: There is bug in scan of %s is, do nothing" % pn)
> -        return False
> -
>      return True
>
>
> @@ -190,34 +185,28 @@ def get_patched_src(d):
>              bb.utils.mkdirhier(d.getVar("B"))
>
>              bb.build.exec_func("do_unpack", d)
> -        # Copy source of kernel to spdx_workdir
> +
> +            if d.getVar("SRC_URI") != "":
> +                bb.build.exec_func("do_patch", d)
> +
> +        # Copy source from work-share to spdx_workdir
>          if is_work_shared_spdx(d):
> -            share_src = d.getVar("WORKDIR")
> +            share_src = d.getVar('S')
>              d.setVar("WORKDIR", spdx_workdir)
>              d.setVar("STAGING_DIR_NATIVE", spdx_sysroot_native)
> +            # Copy source to ${SPDXWORK}, same basename dir of ${S};
>              src_dir = (
>                  spdx_workdir
>                  + "/"
> -                + d.getVar("PN")
> -                + "-"
> -                + d.getVar("PV")
> -                + "-"
> -                + d.getVar("PR")
> +                + os.path.basename(share_src)
>              )
> -            bb.utils.mkdirhier(src_dir)
> +            # For kernel souce, rename suffix dir 'kernel-source'
> +            # to ${BP} (${BPN}-${PV})
>              if bb.data.inherits_class("kernel", d):
> -                share_src = d.getVar("STAGING_KERNEL_DIR")
> -            cmd_copy_share = "cp -rf " + share_src + "/* " + src_dir + "/"
> -            cmd_copy_shared_res = os.popen(cmd_copy_share).read()
> -            bb.note("cmd_copy_shared_result = " + cmd_copy_shared_res)
> -
> -            git_path = src_dir + "/.git"
> -            if os.path.exists(git_path):
> -                shutils.rmtree(git_path)
> +                src_dir = spdx_workdir + "/" + d.getVar('BP')
>
> -        # Make sure gcc and kernel sources are patched only once
> -        if not (d.getVar("SRC_URI") == "" or is_work_shared_spdx(d)):
> -            bb.build.exec_func("do_patch", d)
> +            bb.note(f"copyhardlinktree {share_src} to {src_dir}")
> +            oe.path.copyhardlinktree(share_src, src_dir)
>
>          # Some userland has no source.
>          if not os.path.exists(spdx_workdir):
> @@ -225,6 +214,8 @@ def get_patched_src(d):
>      finally:
>          d.setVar("WORKDIR", workdir)
>
> +def has_task(d, task):
> +    return bool(d.getVarFlag(task, "task", False)) and not bool(d.getVarFlag(task, "noexec", False))
>
>  def fetch_data_to_uri(fd, name):
>      """
> --
> 2.25.1
>
Hongxu Jia Oct. 29, 2024, 4:02 a.m. UTC | #3
After apply the fix and build, I found call function  ${@oe.spdx_common.create_spdx_source_deps(d)} or  ${oe.spdx_common.create_spdx_source_deps(d)} along with addtask not working

+addtask do_create_spdx after \
+    do_package do_packagedata \
+    do_collect_spdx_deps \
+    ${@oe.spdx_common.create_spdx_source_deps(d)} \
+    before do_populate_sdk do_build do_rm_work

I will try to find a fix in v2

//Hongxu




________________________________
From: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org> on behalf of hongxu <hongxu.jia@eng.windriver.com>
Sent: Tuesday, October 29, 2024 10:59 AM
To: Joshua Watt <jpewhacker@gmail.com>
Cc: openembedded-core@lists.openembedded.org <openembedded-core@lists.openembedded.org>
Subject: Re: [oe-core][PATCH 5/6] create-spdx-{2.2,3.0}: support SPDX include source for work-share directory

Base on your comments, I found we should use create_spdx_source_deps to maintain
the dependency of do_create_spdx for  SPDX_INCLUDE_SOURCES = "1"

Move create_spdx_source_deps to meta/lib/oe/spdx_common.py<https://urldefense.com/v3/__http://spdx_common.py__;!!AjveYdw8EvQ!bmiw_miqypZcRLzkLoy9hfrLIwIcLFUR6PxuI9Xg7nmkvLhcGSKs8Gw9qp1emglEDbHWz5tsS9uqFcgYse8cIX_5MMVjV0bu4w$> for  both
of SPDX 2.2 and SPDX 3.0, use ${@oe.spdx_common.create_spdx_source_deps(d)}
to instead of ${create_spdx_source_deps(d)}. Assure the patched source
in work share is ready for SPDX source copy, like following:

--- a/meta/classes/create-spdx-2.2.bbclass
+++ b/meta/classes/create-spdx-2.2.bbclass
@@ -588,8 +588,11 @@ python do_create_spdx() {
             oe.sbom.write_doc(d, package_doc, pkg_arch, "packages", indent=get_json_indent(d))
 }
 do_create_spdx[vardepsexclude] += "BB_NUMBER_THREADS"
-# NOTE: depending on do_unpack is a hack that is necessary to get it's dependencies for archive the source
-addtask do_create_spdx after do_package do_packagedata do_unpack do_collect_spdx_deps before do_populate_sdk do_build do_rm_work
+addtask do_create_spdx after \
+    do_package do_packagedata \
+    do_collect_spdx_deps \
+    ${@oe.spdx_common.create_spdx_source_deps(d)} \
+    before do_populate_sdk do_build do_rm_work

 SSTATETASKS += "do_create_spdx"
 do_create_spdx[sstate-inputdirs] = "${SPDXDEPLOY}"
diff --git a/meta/classes/create-spdx-3.0.bbclass b/meta/classes/create-spdx-3.0.bbclass
index 5f0590198f..d615edcf57 100644
--- a/meta/classes/create-spdx-3.0.bbclass
+++ b/meta/classes/create-spdx-3.0.bbclass
@@ -132,22 +132,9 @@ addtask do_create_spdx after \
     do_collect_spdx_deps \
     do_deploy_source_date_epoch \
     do_populate_sysroot do_package do_packagedata \
-    ${create_spdx_source_deps(d)} \
+    ${@oe.spdx_common.create_spdx_source_deps(d)} \
     before do_populate_sdk do_populate_sdk_ext do_build do_rm_work

-def create_spdx_source_deps(d):
-    deps = []
-    if d.getVar("SPDX_INCLUDE_SOURCES") == "1":
-        deps.extend([
-            # do_unpack is a hack for now; we only need it to get the
-            # dependencies do_unpack already has so we can extract the source
-            # ourselves
-            "do_unpack",
-            # For kernel source code
-            "do_shared_workdir",
-        ])
-    return " ".join(deps)

--- a/meta/lib/oe/spdx_common.py<https://urldefense.com/v3/__http://spdx_common.py__;!!AjveYdw8EvQ!bmiw_miqypZcRLzkLoy9hfrLIwIcLFUR6PxuI9Xg7nmkvLhcGSKs8Gw9qp1emglEDbHWz5tsS9uqFcgYse8cIX_5MMVjV0bu4w$>
+++ b/meta/lib/oe/spdx_common.py<https://urldefense.com/v3/__http://spdx_common.py__;!!AjveYdw8EvQ!bmiw_miqypZcRLzkLoy9hfrLIwIcLFUR6PxuI9Xg7nmkvLhcGSKs8Gw9qp1emglEDbHWz5tsS9uqFcgYse8cIX_5MMVjV0bu4w$>
@@ -37,8 +37,35 @@ def extract_licenses(filename):
     return []


+def create_spdx_source_deps(d):
+    deps = []
+    if d.getVar("SPDX_INCLUDE_SOURCES") == "1":
+        deps.extend([
+            # do_unpack is a hack for now; we only need it to get the
+            # dependencies do_unpack already has so we can extract the source
+            # ourselves
+            "do_unpack",
+        ])
+
+        # Assure the patched source in work share is ready for SPDX source copy
+        if is_work_shared_spdx(d) and  process_sources(d):
+            deps.extend([
+                # For kernel source code
+                "do_shared_workdir",
+                # For gcc-source-${PV}
+                "do_preconfigure",
+                # For gcc-cross-x86_64
+                "do_configure",
+                "do_patch",
+            ])
+
+
+    return " ".join(deps)

//Hongxu


On Fri, Oct 25, 2024 at 9:25 PM Hongxu Jia <hongxu.jia@windriver.com> wrote:
>
> Originally, while SPDX_INCLUDE_SOURCES = "1" [1], there is bug in scan
> for gcc, libgcc in which the sources locates in work-share directory.
> Copy source from ${WORKDIR} to ${SPDXWORK} did not satisfy the situation
> while ${S} was not included in ${WORKDIR}
>
> This commit aim to support SPDX include source for work-share directory
>
> 1. If is_work_shared_spdx, Copy source from ${S} to ${SPDXWORK},
> normally the dest dir in ${SPDXWORK} has the same basename dir of ${S};
> but for kernel source, rename basename dir 'kernel-source' to ${BP} (${BPN}-${PV})
>
> 2. For SPDX source copy, do hard link copy to save copy time
>
> 3. Move do_patch to no work shared situation along with do_unpack
>
> 4. Tweak task do_create_spdx dependencies to assure the patched source
> in work share is ready for SPDX source copy
>
> 5. Remove bb.data.inherits_class('kernel', d) from is_work_shared_spdx,
> the kernel source locates in 'work-shared', test kernel.bbclass is not
> necessary
>
> [1] https://docs.yoctoproject.org/dev/ref-manual/variables.html#term-SPDX_INCLUDE_SOURCES<https://urldefense.com/v3/__https://docs.yoctoproject.org/dev/ref-manual/variables.html*term-SPDX_INCLUDE_SOURCES__;Iw!!AjveYdw8EvQ!bmiw_miqypZcRLzkLoy9hfrLIwIcLFUR6PxuI9Xg7nmkvLhcGSKs8Gw9qp1emglEDbHWz5tsS9uqFcgYse8cIX_5MMUft5bJ9A$>
>
> Signed-off-by: Hongxu Jia <hongxu.jia@windriver.com>
> ---
>  meta/classes/create-spdx-2.2.bbclass | 20 ++++++++++++++
>  meta/classes/create-spdx-3.0.bbclass | 19 +++++++++++++
>  meta/lib/oe/spdx_common.py<https://urldefense.com/v3/__http://spdx_common.py__;!!AjveYdw8EvQ!bmiw_miqypZcRLzkLoy9hfrLIwIcLFUR6PxuI9Xg7nmkvLhcGSKs8Gw9qp1emglEDbHWz5tsS9uqFcgYse8cIX_5MMVjV0bu4w$>           | 41 +++++++++++-----------------
>  3 files changed, 55 insertions(+), 25 deletions(-)
>
> diff --git a/meta/classes/create-spdx-2.2.bbclass b/meta/classes/create-spdx-2.2.bbclass
> index cd1d6819bf..900595a7e9 100644
> --- a/meta/classes/create-spdx-2.2.bbclass
> +++ b/meta/classes/create-spdx-2.2.bbclass
> @@ -947,3 +947,23 @@ def combine_spdx(d, rootfs_name, rootfs_deploydir, rootfs_spdxid, packages, spdx
>              tar.addfile(info, fileobj=index_str)
>
>  combine_spdx[vardepsexclude] += "BB_NUMBER_THREADS SPDX_MULTILIB_SSTATE_ARCHS"
> +
> +python () {
> +    import oe.spdx_common
> +
> +    # Assure the patched source in work share is ready for SPDX source copy
> +    if oe.spdx_common.is_work_shared_spdx(d) and \
> +       d.getVar("SPDX_INCLUDE_SOURCES") == "1" and \
> +       oe.spdx_common.process_sources(d):
> +        pn = d.getVar('PN')
> +        # There is a corner case with "gcc-source-${PV}" recipes, they use "do_preconfigure"
> +        # to modify source file after do_patch
> +        if oe.spdx_common.has_task(d, "do_preconfigure"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_preconfigure' % pn)
> +        elif oe.spdx_common.has_task(d, "do_patch"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_patch' % pn)
> +        # There is a corner case with "gcc-cross-x86_64" recipes, it has "do_configure"
> +        # but no do_patch
> +        elif oe.spdx_common.has_task(d, "do_configure"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_configure' % pn)
> +}

For some reason, I think I'd prefer something like:

do_create_spdx[depends] = "${@extra_create_spdx_deps(d)}"

def extra_create_spdx_deps(d):
   if BLAH:
     return "do_configure"

  # ETC.

  return ""

But I don't know why. Perhaps Richard can weigh in on if one is better
than the other?

> diff --git a/meta/classes/create-spdx-3.0.bbclass b/meta/classes/create-spdx-3.0.bbclass
> index 5f0590198f..284e0ff91c 100644
> --- a/meta/classes/create-spdx-3.0.bbclass
> +++ b/meta/classes/create-spdx-3.0.bbclass
> @@ -190,3 +190,22 @@ python spdx30_build_started_handler () {
>  addhandler spdx30_build_started_handler
>  spdx30_build_started_handler[eventmask] = "bb.event.BuildStarted"
>
> +python () {
> +    import oe.spdx_common
> +
> +    # Assure the patched source in work share is ready for SPDX source copy
> +    if oe.spdx_common.is_work_shared_spdx(d) and \
> +       d.getVar("SPDX_INCLUDE_SOURCES") == "1" and \
> +       oe.spdx_common.process_sources(d):
> +        pn = d.getVar('PN')
> +        # There is a corner case with "gcc-source-${PV}" recipes, they use "do_preconfigure"
> +        # to modify source file after do_patch
> +        if oe.spdx_common.has_task(d, "do_preconfigure"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_preconfigure' % pn)
> +        elif oe.spdx_common.has_task(d, "do_patch"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_patch' % pn)
> +        # There is a corner case with "gcc-cross-x86_64" recipes, it has "do_configure"
> +        # but no do_patch
> +        elif oe.spdx_common.has_task(d, "do_configure"):
> +            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_configure' % pn)
> +}
> diff --git a/meta/lib/oe/spdx_common.py<https://urldefense.com/v3/__http://spdx_common.py__;!!AjveYdw8EvQ!bmiw_miqypZcRLzkLoy9hfrLIwIcLFUR6PxuI9Xg7nmkvLhcGSKs8Gw9qp1emglEDbHWz5tsS9uqFcgYse8cIX_5MMVjV0bu4w$> b/meta/lib/oe/spdx_common.py<https://urldefense.com/v3/__http://spdx_common.py__;!!AjveYdw8EvQ!bmiw_miqypZcRLzkLoy9hfrLIwIcLFUR6PxuI9Xg7nmkvLhcGSKs8Gw9qp1emglEDbHWz5tsS9uqFcgYse8cIX_5MMVjV0bu4w$>
> index 1ea55419ae..ab037b4088 100644
> --- a/meta/lib/oe/spdx_common.py<https://urldefense.com/v3/__http://spdx_common.py__;!!AjveYdw8EvQ!bmiw_miqypZcRLzkLoy9hfrLIwIcLFUR6PxuI9Xg7nmkvLhcGSKs8Gw9qp1emglEDbHWz5tsS9uqFcgYse8cIX_5MMVjV0bu4w$>
> +++ b/meta/lib/oe/spdx_common.py<https://urldefense.com/v3/__http://spdx_common.py__;!!AjveYdw8EvQ!bmiw_miqypZcRLzkLoy9hfrLIwIcLFUR6PxuI9Xg7nmkvLhcGSKs8Gw9qp1emglEDbHWz5tsS9uqFcgYse8cIX_5MMVjV0bu4w$>
> @@ -38,7 +38,7 @@ def extract_licenses(filename):
>
>
>  def is_work_shared_spdx(d):
> -    return bb.data.inherits_class("kernel", d) or ("work-shared" in d.getVar("WORKDIR"))
> +    return 'work-shared' in d.getVar('S')

Should the "/" be included? (e.g. return "/work-shared/" in d.getVar("S") )

>
>
>  def load_spdx_license_data(d):
> @@ -74,11 +74,6 @@ def process_sources(d):
>      if d.getVar("PN") == "shadow-sysroot":
>          return False
>
> -    # We just archive gcc-source for all the gcc related recipes
> -    if d.getVar("BPN") in ["gcc", "libgcc"]:
> -        bb.debug(1, "spdx: There is bug in scan of %s is, do nothing" % pn)
> -        return False
> -
>      return True
>
>
> @@ -190,34 +185,28 @@ def get_patched_src(d):
>              bb.utils.mkdirhier(d.getVar("B"))
>
>              bb.build.exec_func("do_unpack", d)
> -        # Copy source of kernel to spdx_workdir
> +
> +            if d.getVar("SRC_URI") != "":
> +                bb.build.exec_func("do_patch", d)
> +
> +        # Copy source from work-share to spdx_workdir
>          if is_work_shared_spdx(d):
> -            share_src = d.getVar("WORKDIR")
> +            share_src = d.getVar('S')
>              d.setVar("WORKDIR", spdx_workdir)
>              d.setVar("STAGING_DIR_NATIVE", spdx_sysroot_native)
> +            # Copy source to ${SPDXWORK}, same basename dir of ${S};
>              src_dir = (
>                  spdx_workdir
>                  + "/"
> -                + d.getVar("PN")
> -                + "-"
> -                + d.getVar("PV")
> -                + "-"
> -                + d.getVar("PR")
> +                + os.path.basename(share_src)
>              )
> -            bb.utils.mkdirhier(src_dir)
> +            # For kernel souce, rename suffix dir 'kernel-source'
> +            # to ${BP} (${BPN}-${PV})
>              if bb.data.inherits_class("kernel", d):
> -                share_src = d.getVar("STAGING_KERNEL_DIR")
> -            cmd_copy_share = "cp -rf " + share_src + "/* " + src_dir + "/"
> -            cmd_copy_shared_res = os.popen(cmd_copy_share).read()
> -            bb.note("cmd_copy_shared_result = " + cmd_copy_shared_res)
> -
> -            git_path = src_dir + "/.git"
> -            if os.path.exists(git_path):
> -                shutils.rmtree(git_path)
> +                src_dir = spdx_workdir + "/" + d.getVar('BP')
>
> -        # Make sure gcc and kernel sources are patched only once
> -        if not (d.getVar("SRC_URI") == "" or is_work_shared_spdx(d)):
> -            bb.build.exec_func("do_patch", d)
> +            bb.note(f"copyhardlinktree {share_src} to {src_dir}")
> +            oe.path.copyhardlinktree(share_src, src_dir)
>
>          # Some userland has no source.
>          if not os.path.exists(spdx_workdir):
> @@ -225,6 +214,8 @@ def get_patched_src(d):
>      finally:
>          d.setVar("WORKDIR", workdir)
>
> +def has_task(d, task):
> +    return bool(d.getVarFlag(task, "task", False)) and not bool(d.getVarFlag(task, "noexec", False))
>
>  def fetch_data_to_uri(fd, name):
>      """
> --
> 2.25.1
>
diff mbox series

Patch

diff --git a/meta/classes/create-spdx-2.2.bbclass b/meta/classes/create-spdx-2.2.bbclass
index cd1d6819bf..900595a7e9 100644
--- a/meta/classes/create-spdx-2.2.bbclass
+++ b/meta/classes/create-spdx-2.2.bbclass
@@ -947,3 +947,23 @@  def combine_spdx(d, rootfs_name, rootfs_deploydir, rootfs_spdxid, packages, spdx
             tar.addfile(info, fileobj=index_str)
 
 combine_spdx[vardepsexclude] += "BB_NUMBER_THREADS SPDX_MULTILIB_SSTATE_ARCHS"
+
+python () {
+    import oe.spdx_common
+
+    # Assure the patched source in work share is ready for SPDX source copy
+    if oe.spdx_common.is_work_shared_spdx(d) and \
+       d.getVar("SPDX_INCLUDE_SOURCES") == "1" and \
+       oe.spdx_common.process_sources(d):
+        pn = d.getVar('PN')
+        # There is a corner case with "gcc-source-${PV}" recipes, they use "do_preconfigure"
+        # to modify source file after do_patch
+        if oe.spdx_common.has_task(d, "do_preconfigure"):
+            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_preconfigure' % pn)
+        elif oe.spdx_common.has_task(d, "do_patch"):
+            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_patch' % pn)
+        # There is a corner case with "gcc-cross-x86_64" recipes, it has "do_configure"
+        # but no do_patch
+        elif oe.spdx_common.has_task(d, "do_configure"):
+            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_configure' % pn)
+}
diff --git a/meta/classes/create-spdx-3.0.bbclass b/meta/classes/create-spdx-3.0.bbclass
index 5f0590198f..284e0ff91c 100644
--- a/meta/classes/create-spdx-3.0.bbclass
+++ b/meta/classes/create-spdx-3.0.bbclass
@@ -190,3 +190,22 @@  python spdx30_build_started_handler () {
 addhandler spdx30_build_started_handler
 spdx30_build_started_handler[eventmask] = "bb.event.BuildStarted"
 
+python () {
+    import oe.spdx_common
+
+    # Assure the patched source in work share is ready for SPDX source copy
+    if oe.spdx_common.is_work_shared_spdx(d) and \
+       d.getVar("SPDX_INCLUDE_SOURCES") == "1" and \
+       oe.spdx_common.process_sources(d):
+        pn = d.getVar('PN')
+        # There is a corner case with "gcc-source-${PV}" recipes, they use "do_preconfigure"
+        # to modify source file after do_patch
+        if oe.spdx_common.has_task(d, "do_preconfigure"):
+            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_preconfigure' % pn)
+        elif oe.spdx_common.has_task(d, "do_patch"):
+            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_patch' % pn)
+        # There is a corner case with "gcc-cross-x86_64" recipes, it has "do_configure"
+        # but no do_patch
+        elif oe.spdx_common.has_task(d, "do_configure"):
+            d.appendVarFlag('do_create_spdx', 'depends', ' %s:do_configure' % pn)
+}
diff --git a/meta/lib/oe/spdx_common.py b/meta/lib/oe/spdx_common.py
index 1ea55419ae..ab037b4088 100644
--- a/meta/lib/oe/spdx_common.py
+++ b/meta/lib/oe/spdx_common.py
@@ -38,7 +38,7 @@  def extract_licenses(filename):
 
 
 def is_work_shared_spdx(d):
-    return bb.data.inherits_class("kernel", d) or ("work-shared" in d.getVar("WORKDIR"))
+    return 'work-shared' in d.getVar('S')
 
 
 def load_spdx_license_data(d):
@@ -74,11 +74,6 @@  def process_sources(d):
     if d.getVar("PN") == "shadow-sysroot":
         return False
 
-    # We just archive gcc-source for all the gcc related recipes
-    if d.getVar("BPN") in ["gcc", "libgcc"]:
-        bb.debug(1, "spdx: There is bug in scan of %s is, do nothing" % pn)
-        return False
-
     return True
 
 
@@ -190,34 +185,28 @@  def get_patched_src(d):
             bb.utils.mkdirhier(d.getVar("B"))
 
             bb.build.exec_func("do_unpack", d)
-        # Copy source of kernel to spdx_workdir
+
+            if d.getVar("SRC_URI") != "":
+                bb.build.exec_func("do_patch", d)
+
+        # Copy source from work-share to spdx_workdir
         if is_work_shared_spdx(d):
-            share_src = d.getVar("WORKDIR")
+            share_src = d.getVar('S')
             d.setVar("WORKDIR", spdx_workdir)
             d.setVar("STAGING_DIR_NATIVE", spdx_sysroot_native)
+            # Copy source to ${SPDXWORK}, same basename dir of ${S};
             src_dir = (
                 spdx_workdir
                 + "/"
-                + d.getVar("PN")
-                + "-"
-                + d.getVar("PV")
-                + "-"
-                + d.getVar("PR")
+                + os.path.basename(share_src)
             )
-            bb.utils.mkdirhier(src_dir)
+            # For kernel souce, rename suffix dir 'kernel-source'
+            # to ${BP} (${BPN}-${PV})
             if bb.data.inherits_class("kernel", d):
-                share_src = d.getVar("STAGING_KERNEL_DIR")
-            cmd_copy_share = "cp -rf " + share_src + "/* " + src_dir + "/"
-            cmd_copy_shared_res = os.popen(cmd_copy_share).read()
-            bb.note("cmd_copy_shared_result = " + cmd_copy_shared_res)
-
-            git_path = src_dir + "/.git"
-            if os.path.exists(git_path):
-                shutils.rmtree(git_path)
+                src_dir = spdx_workdir + "/" + d.getVar('BP')
 
-        # Make sure gcc and kernel sources are patched only once
-        if not (d.getVar("SRC_URI") == "" or is_work_shared_spdx(d)):
-            bb.build.exec_func("do_patch", d)
+            bb.note(f"copyhardlinktree {share_src} to {src_dir}")
+            oe.path.copyhardlinktree(share_src, src_dir)
 
         # Some userland has no source.
         if not os.path.exists(spdx_workdir):
@@ -225,6 +214,8 @@  def get_patched_src(d):
     finally:
         d.setVar("WORKDIR", workdir)
 
+def has_task(d, task):
+    return bool(d.getVarFlag(task, "task", False)) and not bool(d.getVarFlag(task, "noexec", False))
 
 def fetch_data_to_uri(fd, name):
     """