[OE-core][PATCH 0/3] Fix incomplete license expressions being preferred over complete ones

[OE-core][PATCH 0/3] Fix incomplete license expressions being preferred over complete ones

[Joshua Watt]

Fixes a bug where an incomplete (missing the actual license text) could
be preferred over the same expression that included the license text.

Joshua Watt (3):
  lib/oe/license.py: Fix documentation of parse_legacy_license()
  lib/oe/sbom.py: Fix partial licenses masking complete ones
  lib/oe/sbom.py: Split deduplication of CreationInfo

 meta/lib/oe/license.py |   4 +-
 meta/lib/oe/sbom30.py  | 107 +++++++++++++++++++++++++++++++++--------
 2 files changed, 88 insertions(+), 23 deletions(-)

-- 
2.56.0

[OE-core][PATCH 1/3] lib/oe/license.py: Fix documentation of parse_legacy_license()

[Joshua Watt]

The function documentation was incorrect in that it doesn't check if the
license text exists. This must be done with get_license_path()

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
---
 meta/lib/oe/license.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta/lib/oe/license.py b/meta/lib/oe/license.py
index bd1d85ad5f..e2a2bcc47d 100644
--- a/meta/lib/oe/license.py
+++ b/meta/lib/oe/license.py
@@ -425,8 +425,8 @@ def parse_legacy_license(d, licensestr):
     once legacy licenses are not longer allowed.
 
     Unknown license IDs in the string are automatically converted to
-    LicenseRefs, assuming that they are provided as either a common license
-    file or a NO_GENERIC_LICENSE
+    LicenseRefs. To check if a LicenseRef actually exists, use
+    get_license_path()
     """
     licensestr = _substitute_legacy_license(licensestr)
     if licensestr is None:
-- 
2.56.0

[OE-core][PATCH 2/3] lib/oe/sbom.py: Fix partial licenses masking complete ones

[Joshua Watt]

When scanning files for license expressions, an "incomplete" (or
partial) license object can be created, in that if the Licenses
Expression contains a LicenseRef- license, the text for the license will
is not linked. This usually happens when parsing recipe License
expressions, since the license files are not always present until
do_fetch/do_unpack.

Since license expressions are matched based on the expression, it is
possible for the partial license to be used instead of the complete one.
It is also possible for the partial license to be preferred when
de-duplicating license expressions while generating the final SBoM.

Fix this by identifying when a partial license is provided (that is, a
license with no text map and a LicenseRef-), and tagging them with an
internal extension that causes it to be less preferred than an complete
license.

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
---
 meta/lib/oe/sbom30.py | 106 +++++++++++++++++++++++++++++++++---------
 1 file changed, 85 insertions(+), 21 deletions(-)

diff --git a/meta/lib/oe/sbom30.py b/meta/lib/oe/sbom30.py
index b33860e4aa..713eec92d4 100644
--- a/meta/lib/oe/sbom30.py
+++ b/meta/lib/oe/sbom30.py
@@ -97,6 +97,32 @@ class OELicenseScannedExtension(oe.spdx30.extension_Extension):
     INTERNAL = True
 
 
+class OEIncompleteExtension(oe.spdx30.extension_Extension):
+    """
+    The presence of this extension means that the Element is incomplete, and
+    should not be preferred over a completely defined Element
+    """
+
+    TYPE = OE_SPDX_BASE + "incomplete"
+    CLOSED = True
+    INTERNAL = True
+
+    @classmethod
+    def is_incomplete(cls, obj):
+        for e in obj.extension:
+            if isinstance(e, cls):
+                return True
+        return False
+
+    @classmethod
+    def is_complete(cls, obj):
+        return not cls.is_incomplete(obj)
+
+    @classmethod
+    def set(cls, obj):
+        obj.extension.append(cls())
+
+
 class OEDocumentExtension(oe.spdx30.extension_Extension):
     """
     This extension is added to a SpdxDocument to indicate various useful bits
@@ -194,21 +220,35 @@ def to_list(l):
 
 class Dedup(object):
     def __init__(self, objset):
-        self.unique = set()
-        self.dedup = {}
+        self.unique = []
+        self.dup_idx = {}
         self.objset = objset
+        self.has_duplicates = False
 
-    def find_duplicates(self, cmp, typ, **kwargs):
+    def find_duplicates(self, cmp, typ, choose=None, **kwargs):
         for o in self.objset.foreach_filter(typ, **kwargs):
-            for u in self.unique:
+            for idx, u in enumerate(self.unique):
                 if cmp(u, o):
-                    self.dedup[o] = u
+                    self.has_duplicates = True
+                    self.dup_idx[o] = idx
+                    if choose:
+                        self.unique[idx] = choose(u, o)
                     break
             else:
-                self.unique.add(o)
+                self.dup_idx[o] = len(self.unique)
+                self.unique.append(o)
 
     def get(self, o):
-        return self.dedup.get(o, o)
+        idx = self.dup_idx.get(o, -1)
+        if idx < 0:
+            return o
+        return self.unique[idx]
+
+    def duplicates(self):
+        for o, idx in self.dup_idx.items():
+            d = self.unique[idx]
+            if d is not o:
+                yield o, d
 
 
 class ObjectSet(oe.spdx30.SHACLObjectSet):
@@ -582,23 +622,34 @@ class ObjectSet(oe.spdx30.SHACLObjectSet):
         if license_list_version.count(".") < 2:
             license_list_version += ".0"
 
+        license_text = sorted(license_text_map.items(), key=lambda t: t[0])
+
+        incomplete = not license_text and "licenseref-" in license_expression.lower()
+
         spdxid = [
-            "license",
+            "license" if not incomplete else "partial-license",
             license_list_version,
             re.sub(r"[^a-zA-Z0-9_-]", "_", license_expression),
         ]
 
-        license_text = sorted(license_text_map.items(), key=lambda t: t[0])
-
         if not license_text:
-            lic = self.find_filter(
+            incomplete_lic = None
+            for lic in self.foreach_filter(
                 oe.spdx30.simplelicensing_LicenseExpression,
                 simplelicensing_licenseExpression=license_expression,
                 simplelicensing_licenseListVersion=license_list_version,
-            )
-            if lic is not None:
-                return lic
+            ):
+                if OEIncompleteExtension.is_complete(lic):
+                    return lic
+
+                if not incomplete_lic:
+                    incomplete_lic = lic
+
+            if incomplete and incomplete_lic:
+                return incomplete_lic
+
         else:
+            # Note: This branch will never be entered for incomplete licenses
             spdxid.append(spdxid_hash(*(v for _, v in license_text)))
             lic = self.find_by_id(self.new_spdxid(*spdxid))
             if lic is not None:
@@ -618,6 +669,9 @@ class ObjectSet(oe.spdx30.SHACLObjectSet):
                 oe.spdx30.DictionaryEntry(key=key, value=value._id)
             )
 
+        if incomplete:
+            OEIncompleteExtension.set(lic)
+
         return lic
 
     def scan_declared_licenses(self, spdx_file, filepath, license_data):
@@ -953,16 +1007,16 @@ class ObjectSet(oe.spdx30.SHACLObjectSet):
 
             return True
 
-        if d.dedup:
+        if d.has_duplicates:
             for o in self.objects:
                 o.walk(visit)
 
-            for k, v in d.dedup.items():
+            for dup, replacement in d.duplicates():
                 bb.debug(
                     1,
-                    f"Removing duplicate {k.__class__.__name__} {k._id or id(k)} -> {v._id or id(v)}",
+                    f"Removing duplicate {dup.__class__.__name__} {dup._id or id(dup)} -> {replacement._id or id(replacement)}",
                 )
-                self.objects.discard(k)
+                self.objects.discard(dup)
 
             self.create_index()
 
@@ -1148,9 +1202,6 @@ def create_sbom(d, name, root_elements, add_objectsets=[]):
             + "\n  ".join(sorted(list(missing_spdxids)))
         )
 
-    # Filter out internal extensions from final SBoMs
-    objset.remove_internal_extensions()
-
     # SBoM should be the only root element of the document
     objset.doc.rootElement = [sbom]
 
@@ -1161,6 +1212,15 @@ def create_sbom(d, name, root_elements, add_objectsets=[]):
             == b.simplelicensing_licenseListVersion
         )
 
+    def choose_license_expressions(current, new):
+        if OEIncompleteExtension.is_complete(current):
+            return current
+
+        if OEIncompleteExtension.is_incomplete(new):
+            return current
+
+        return new
+
     def cmp_creation_info(a, b):
         data_a = {k: a[k] for k in a}
         data_b = {k: b[k] for k in b}
@@ -1173,9 +1233,13 @@ def create_sbom(d, name, root_elements, add_objectsets=[]):
         dedup.find_duplicates(
             cmp_license_expression,
             oe.spdx30.simplelicensing_LicenseExpression,
+            choose=choose_license_expressions,
         )
 
         # Deduplicate creation info
         dedup.find_duplicates(cmp_creation_info, oe.spdx30.CreationInfo)
 
+    # Filter out internal extensions from final SBoMs
+    objset.remove_internal_extensions()
+
     return objset, sbom
-- 
2.56.0

[OE-core][PATCH 3/3] lib/oe/sbom.py: Split deduplication of CreationInfo

[Joshua Watt]

The deduplicate logic has a logic flaw in that it was scanning over all
deduplicated items from previous calls to find_deduplicates(). This
doesn't make any sense because the types of objects already being
deduplicated might not match the new filter. In practice, this didn't
matter because the CreationInfo comparison function doesn't do anything
that cares if the type is actually a CreationInfo; so it didn't fail
when it was passed something else.

Fix this by creating a separate instance of the deduplication object to
start with a clean state

Signed-off-by: Joshua Watt <JPEWhacker@gmail.com>
---
 meta/lib/oe/sbom30.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meta/lib/oe/sbom30.py b/meta/lib/oe/sbom30.py
index 713eec92d4..e279cfc82b 100644
--- a/meta/lib/oe/sbom30.py
+++ b/meta/lib/oe/sbom30.py
@@ -1236,6 +1236,7 @@ def create_sbom(d, name, root_elements, add_objectsets=[]):
             choose=choose_license_expressions,
         )
 
+    with objset.deduplicate() as dedup:
         # Deduplicate creation info
         dedup.find_duplicates(cmp_creation_info, oe.spdx30.CreationInfo)
 
-- 
2.56.0