[wrynose][PATCH 0/4] spdx30: fix the source download enrichment

[wrynose][PATCH 0/4] spdx30: fix the source download enrichment

[Stefano Tondo]

Hi Yoann,

This series backports three spdx30 fixes from master to wrynose, plus
their tests. Patches 2 to 4 also go to blacksail in a separate series.
Blacksail already has patch 1.

Wrynose has 9f1825e74d ("spdx30: Add Git version and PURL to source
downloads"). It adds the commit hash as version and a purl to the Git
source downloads in the SPDX. That code has three bugs, and master
fixed all of them:

1. spdx30_tasks: Fix duplicated revision in Git PURLs (e31479ddba)

   On wrynose, every Git purl has the revision twice, for example
   pkg:github/fastfloat/fast_float@<rev>@<rev>. That purl is invalid,
   so scanners cannot match it. Master reformatted the file after this
   fix, so I applied it by hand. The change is the same.

2. spdx30: Make source download enrichment part of the task signature
   (e57ec14f8d)

   The enrichment code is in two functions whose names start with an
   underscore. bitbake leaves such functions out of the task
   signature. So do_create_spdx did not track them, or the
   SPDX_GIT_PURL_MAPPINGS variable that they read. After a change,
   bitbake restored the old SPDX from sstate. The fix renames the two
   functions. As a side effect, the do_create_spdx signature of every
   recipe changes once.

3. spdx30: Enrich gitsm source downloads like git ones (1e5146d8f9)

   This one can look like a feature, but I think it is a fix.
   9f1825e74d enriches Git source downloads, but it checks only
   fd.type == "git". gitsm is the Git fetcher with submodule support,
   and it has the same revision. So a recipe that changes from git://
   to gitsm:// loses the version and the purl of its main repository,
   with no warning. The fix is one line.

4. oeqa/selftest: Add tests for source download enrichment
   (3ca309cbd3)

   Two selftests for patches 2 and 3, which fail without them. On
   wrynose, test_git_purl_mappings_rerun_create_spdx also fails
   without patch 1. That is how I found that wrynose does not have it.
   If you take only fixes, you can drop this patch. Patchwork can show
   it as accepted already, because a March commit (9a622a2062) has the
   same subject.

I tested the series with oe-selftest -r spdx.SPDX30Check on wrynose
(bitbake 2.18). All 14 tests pass.

KAZUYOSHI AKIYAMA (秋山 和慶) (1):
  spdx30_tasks: Fix duplicated revision in Git PURLs

Stefano Tondo (3):
  spdx30: Make source download enrichment part of the task signature
  spdx30: Enrich gitsm source downloads like git ones
  oeqa/selftest: Add tests for source download enrichment

 meta/lib/oe/spdx30_tasks.py          | 17 +++++-----
 meta/lib/oeqa/selftest/cases/spdx.py | 47 ++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 7 deletions(-)

-- 
2.56.0

[wrynose][PATCH 1/4] spdx30_tasks: Fix duplicated revision in Git PURLs

[Stefano Tondo]

From: KAZUYOSHI AKIYAMA (秋山 和慶) <kazuyoshi_akiyama@jp.honda>

_generate_git_purl() generates invalid PURLs of the form
`pkg:github/<owner>/<repo>@<rev>@<rev>`.
purl-spec only allows a single `@<rev>`.

oe.spdx_common.fetch_data_to_uri() already appends "@<rev>" to the
download location, and urlparse() leaves that suffix in parsed.path.
Therefore, the parsed repository name(`{repo}`) includes `@<rev>`.
So, need to strip the revision suffix before splitting out
the owner(`{owner}`) and the repository name(`{repo}`).

While here, replace str.replace(".git", "") with str.removesuffix() so
repository names that contain ".git" elsewhere are preserved.

Signed-off-by: Kazuyoshi Akiyama <kazuyoshi_akiyama@jp.honda>
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit e31479ddbaf58d5c2eda2b0051e1a821d46e6f7c)
Signed-off-by: Stefano Tondo <stefano.tondo.ext@siemens.com>
---
 meta/lib/oe/spdx30_tasks.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/meta/lib/oe/spdx30_tasks.py b/meta/lib/oe/spdx30_tasks.py
index 1821dd7de4..d5128fff52 100644
--- a/meta/lib/oe/spdx30_tasks.py
+++ b/meta/lib/oe/spdx30_tasks.py
@@ -425,11 +425,14 @@ def _generate_git_purl(d, download_location, srcrev):
 
     for domain, purl_type in git_purl_handlers.items():
         if hostname == domain:
-            path = parsed.path.strip('/')
+            # oe.spdx_common.fetch_data_to_uri() appends "@<rev>" to the
+            # download location, which must not become part of the
+            # repository name
+            path = parsed.path.strip('/').partition('@')[0]
             path_parts = path.split('/')
             if len(path_parts) >= 2:
                 owner = path_parts[0]
-                repo = path_parts[1].replace('.git', '')
+                repo = path_parts[1].removesuffix('.git')
                 return f"{purl_type}/{owner}/{repo}@{srcrev}"
             break
 
-- 
2.56.0

[wrynose][PATCH 2/4] spdx30: Make source download enrichment part of the task signature

[Stefano Tondo]

From: Stefano Tondo <stefano.tondo.ext@siemens.com>

bitbake does not track module functions whose names start with an
underscore when it computes task signatures. _enrich_source_package()
and _generate_git_purl() were therefore not part of the do_create_spdx
signature, and neither was SPDX_GIT_PURL_MAPPINGS, which
_generate_git_purl() reads. Changing the mappings or this code did not
rerun do_create_spdx, and the previous SPDX was restored from sstate.

Rename both functions so that bitbake tracks them. This changes the
do_create_spdx signature of every recipe once.

Signed-off-by: Stefano Tondo <stefano.tondo.ext@siemens.com>
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit e57ec14f8d9bbe2e789163739df5c991e2eebe5e)
Signed-off-by: Stefano Tondo <stefano.tondo.ext@siemens.com>
---
 meta/lib/oe/spdx30_tasks.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/meta/lib/oe/spdx30_tasks.py b/meta/lib/oe/spdx30_tasks.py
index d5128fff52..9fb2dcc36f 100644
--- a/meta/lib/oe/spdx30_tasks.py
+++ b/meta/lib/oe/spdx30_tasks.py
@@ -383,7 +383,7 @@ def collect_dep_sources(dep_objsets, dest):
 
 
 
-def _generate_git_purl(d, download_location, srcrev):
+def generate_git_purl(d, download_location, srcrev):
     """Generate a Package URL for a Git source from its download location.
 
     Parses the Git URL to identify the hosting service and generates the
@@ -439,7 +439,7 @@ def _generate_git_purl(d, download_location, srcrev):
     return None
 
 
-def _enrich_source_package(d, dl, fd, file_name, primary_purpose):
+def enrich_source_package(d, dl, fd, file_name, primary_purpose):
     """Enrich a Git source download package with version, PURL, and external refs.
 
     For Git sources, extracts the full SHA-1 from SRCREV as the version,
@@ -458,7 +458,7 @@ def _enrich_source_package(d, dl, fd, file_name, primary_purpose):
         # Generate PURL for Git hosting services
         download_location = getattr(dl, 'software_downloadLocation', None)
         if version and download_location:
-            purl = _generate_git_purl(d, download_location, version)
+            purl = generate_git_purl(d, download_location, version)
 
     if version:
         dl.software_packageVersion = version
@@ -547,7 +547,7 @@ def add_download_files(d, objset):
                 )
             )
 
-            _enrich_source_package(d, dl, fd, file_name, primary_purpose)
+            enrich_source_package(d, dl, fd, file_name, primary_purpose)
 
             if fd.method.supports_checksum(fd):
                 # TODO Need something better than hard coding this
-- 
2.56.0

[wrynose][PATCH 3/4] spdx30: Enrich gitsm source downloads like git ones

[Stefano Tondo]

From: Stefano Tondo <stefano.tondo.ext@siemens.com>

The source download enrichment only handled fd.type "git", so gitsm
downloads got the VCS external reference but neither the SRCREV as
version nor the PURL. The gitsm fetcher is a git fetcher with
submodule support and carries the same revision, so treat it the same.

Signed-off-by: Stefano Tondo <stefano.tondo.ext@siemens.com>
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit 1e5146d8f9111416c3fc9247b152d1d70b55c7fe)
Signed-off-by: Stefano Tondo <stefano.tondo.ext@siemens.com>
---
 meta/lib/oe/spdx30_tasks.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/lib/oe/spdx30_tasks.py b/meta/lib/oe/spdx30_tasks.py
index 9fb2dcc36f..dbaccd0e34 100644
--- a/meta/lib/oe/spdx30_tasks.py
+++ b/meta/lib/oe/spdx30_tasks.py
@@ -449,7 +449,7 @@ def enrich_source_package(d, dl, fd, file_name, primary_purpose):
     version = None
     purl = None
 
-    if fd.type == "git":
+    if fd.type in ("git", "gitsm"):
         # Use full SHA-1 from fd.revision
         srcrev = getattr(fd, 'revision', None)
         if srcrev and srcrev not in {'${AUTOREV}', 'AUTOINC', 'INVALID'}:
-- 
2.56.0

[wrynose][PATCH 4/4] oeqa/selftest: Add tests for source download enrichment

[Stefano Tondo]

From: Stefano Tondo <stefano.tondo.ext@siemens.com>

test_gitsm_source_version builds git-submodule-test, which uses gitsm,
and checks that its source download carries the SRCREV as version.

test_git_purl_mappings_rerun_create_spdx builds fastfloat twice, first
with the default mapping and then with github.com mapped to another
PURL type, and checks that the second SPDX carries the new PURL. Before
the enrichment became part of the task signature, the second build
restored the first SPDX from sstate.

Signed-off-by: Stefano Tondo <stefano.tondo.ext@siemens.com>
Signed-off-by: Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit 3ca309cbd38e7115705a9871c389b4641ec88c63)
Signed-off-by: Stefano Tondo <stefano.tondo.ext@siemens.com>
---
 meta/lib/oeqa/selftest/cases/spdx.py | 47 ++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/meta/lib/oeqa/selftest/cases/spdx.py b/meta/lib/oeqa/selftest/cases/spdx.py
index 8285189382..e65ab46f30 100644
--- a/meta/lib/oeqa/selftest/cases/spdx.py
+++ b/meta/lib/oeqa/selftest/cases/spdx.py
@@ -443,3 +443,50 @@ class SPDX30Check(SPDX3CheckBase, OESelftestTestCase):
                 r'\d',
                 f"Version '{version}' for package '{name}' should contain digits"
             )
+    def test_gitsm_source_version(self):
+        """Test that gitsm source downloads are enriched like git ones.
+
+        git-submodule-test is fetched with gitsm://, so its source download
+        must carry the SRCREV as version.
+        """
+        objset = self.check_recipe_spdx(
+            "git-submodule-test",
+            "{DEPLOY_DIR_SPDX}/{SSTATE_PKGARCH}/builds/build-git-submodule-test.spdx.json",
+        )
+
+        srcrev = get_bb_var("SRCREV", "git-submodule-test")
+        for pkg in objset.foreach_type(oe.spdx30.software_Package):
+            location = pkg.software_downloadLocation or ""
+            if location.startswith("git+https://git.yoctoproject.org/git-submodule-test"):
+                self.assertEqual(pkg.software_packageVersion, srcrev)
+                break
+        else:
+            self.fail("git-submodule-test gitsm source download not found")
+
+    def test_git_purl_mappings_rerun_create_spdx(self):
+        """Test that SPDX_GIT_PURL_MAPPINGS is part of the do_create_spdx signature.
+
+        fastfloat is fetched from github.com. Build it with the default
+        mapping, then map github.com to another PURL type: do_create_spdx
+        must run again instead of restoring the first result from sstate.
+        """
+        spdx_path = "{DEPLOY_DIR_SPDX}/{SSTATE_PKGARCH}/builds/build-fastfloat.spdx.json"
+        location_prefix = "git+https://github.com/fastfloat/fast_float"
+        srcrev = get_bb_var("SRCREV", "fastfloat")
+
+        def source_purl(objset):
+            for pkg in objset.foreach_type(oe.spdx30.software_Package):
+                if (pkg.software_downloadLocation or "").startswith(location_prefix):
+                    return pkg.software_packageUrl
+            self.fail("fastfloat git source download not found")
+
+        objset = self.check_recipe_spdx("fastfloat", spdx_path)
+        self.assertEqual(source_purl(objset), f"pkg:github/fastfloat/fast_float@{srcrev}")
+
+        objset = self.check_recipe_spdx(
+            "fastfloat",
+            spdx_path,
+            extraconf='SPDX_GIT_PURL_MAPPINGS = "github.com:pkg:selftest"\n',
+        )
+        self.assertEqual(source_purl(objset), f"pkg:selftest/fastfloat/fast_float@{srcrev}")
+
-- 
2.56.0