[kocaemre]

Pull Request Description

What

• Keep wheel-only name, type, namespace, and global purl type settings out of the default upstream SBOM purl; explicit purl.upstream still takes precedence.
• Cover the four affected settings with SPDX-validating regressions and document the migration for non-PyPI sources.
• Clarify that SbomSettings.purl_type applies to wheel package entries.

Why

Closes #1383. Previously the GENERATED_FROM entry could identify a downstream-renamed wheel rather than the resolved PyPI source. This changes existing SBOM output for users overriding purl type/name/namespace without purl.upstream; set that field explicitly for a different source.

Verification

• RED: uvx --from hatch hatch run test:test tests/test_sbom.py::test_generate_sbom_upstream_ignores_wheel_purl_overrides -q --no-cov --tb=short → 4 expected upstream identity assertion failures (original test-first cycle).

• Follow-up RED: deterministic docstring assertion failed while SbomSettings.purl_type said “all packages”; GREEN: same check passed after making it wheel-only.

• GREEN: uvx --from hatch hatch run test:test tests/test_sbom.py tests/test_packagesettings.py -q --no-cov --tb=short → 112 passed.

• Reviewer follow-up RED (temporary wrong upstream name, restored before commit): uvx --from hatch hatch run test:test tests/test_sbom.py::test_generate_sbom_upstream_ignores_wheel_purl_overrides -q --no-cov --tb=short → 1 expected failure (test-pkg-corp vs test-pkg); GREEN: same command → 4 passed, and the two-file suite → 112 passed. uvx --from hatch hatch run lint:check, uvx --from hatch hatch run mypy:check tests/test_sbom.py, git diff --check passed.

• uvx --from hatch hatch run lint:check → passed; uvx --from hatch hatch run mypy:check src/fromager/sbom.py src/fromager/packagesettings/_models.py tests/test_sbom.py → 77 source, 51 test, 7 e2e files checked, no issues.

• uvx --from hatch hatch run docs:build → build succeeded; git diff --check HEAD~1..HEAD → passed. Full suite/e2e not run locally (repo guidance favors file-scoped tests for small changes). The original two signed-off commits, including the Sphinx heading correction, were squashed into one signed-off commit at maintainer request; CI is rerunning for the new head.

Significant AI assistance: Hermes Agent co-authored the patch; test failures and passing checks were run locally.

• PR follows CONTRIBUTING.md guidelines

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

When purl.upstream is absent, the upstream SBOM entry uses the resolved package name and version in a PyPI PURL. Per-package wheel PURL settings and global sbom.purl_type do not affect that entry. Documentation describes this behavior and directs users with non-PyPI sources to set purl.upstream. Tests cover name, type, namespace, and global type overrides.

Estimated code review effort: 2 (Simple) | ~10 minutes

Severity of issue fixed: Medium

Merge Risk: Minimal · up to 1cae5

The change separates upstream PyPI identity from wheel-specific settings while preserving explicit upstream configuration. No actionable merge-blocking risk is established by the supplied evidence.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: preventing wheel PURL settings from changing upstream identity.
Linked Issues check		Issue #1383 requires that, without purl.upstream, the upstream SBOM entry uses pkg:pypi/@. It also requires that per-package name, type, namespace, and version override…
Out of Scope Changes check		The whole-PR diff changes four areas. docs/reference/config-reference.rst adds migration documentation for SBOM upstream purls. src/fromager/packagesettings/_models.py updates the SbomSettings.purl_ty…
Description check		The description is specific and related to the changeset. It states that wheel-only name, type, namespace, and global purl type settings are excluded from the default upstream SBOM purl, while explici…

Full details: Linked Issues check

Explanation

Issue #1383 requires that, without purl.upstream, the upstream SBOM entry uses pkg:pypi/<package name>@<resolved version>. It also requires that per-package name, type, namespace, and version overrides and global sbom.purl_type do not affect that entry. The whole-PR diff meets these requirements. In src/fromager/sbom.py, _build_upstream_purl returns an explicit pc.upstream value when set. Otherwise it returns PackageURL(type='pypi', namespace=None, name=name, version=str(version)). The sbom_settings parameter is removed, so the global purl type no longer reaches the upstream entry. Tests in tests/test_sbom.py cover name, type, namespace, and global purl_type overrides through test_generate_sbom_upstream_ignores_wheel_purl_overrides. They validate the SPDX output. test_generate_sbom_purl_field_overrides now expects pkg:pypi/test-pkg@1.0.0 for the upstream entry. The issue asks for documentation of the migration. The PurlConfig.upstream docstring and docs/reference/config-reference.rst state the new default and tell non-PyPI sources to set purl.upstream. SbomSettings.purl_type now states that it applies to wheel entries. The earlier FAIL depended on a release-notes entry. Release notes are release coordination, which the coding-requirement scope excludes. No release-notes file exists in the reviewed tree, and this assessment cannot inspect external release notes. The release-notes item therefore does not change the verdict.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff changes four areas. docs/reference/config-reference.rst adds migration documentation for SBOM upstream purls. src/fromager/packagesettings/_models.py updates the SbomSettings.purl_type and PurlConfig.upstream docstrings. src/fromager/sbom.py implements the upstream identity change and removes the now-unused sbom_settings argument. tests/test_sbom.py adds regression coverage and updates the field-override expectation. Each change implements or verifies #1383. The diff contains no unrelated changes. No release-notes file is in the diff.

Full details: Description check

Explanation

The description is specific and related to the changeset. It states that wheel-only name, type, namespace, and global purl type settings are excluded from the default upstream SBOM purl, while explicit purl.upstream still takes precedence. It names the four regression tests, the documentation for non-PyPI migration, and the SbomSettings.purl_type docstring clarification, which match the raw summary's changed files. It also references issue #1383 and lists verification commands.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: python-wheel-build/fromager
• Branch: main
• File: .coderabbit.yaml
• Commit: f9cf031
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 30119774-80e9-41d7-8f68-ce1c8d1a1b3c

Commits

Reviewing files that changed from the base of the PR and between dba98b3 and 1cae5b0.

Files selected for processing: 1

• tests/test_sbom.py

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

[mprpic]

Validated with the following test data:

All scenarios use this overrides/settings.yaml, with per-package settings in overrides/settings/test-pkg.yaml, for test-pkg==0.5.5:

sbom:
  supplier: "Organization: ExampleCo"
  namespace: "https://sbom.example"

Scenario	Settings	Wheel purl (same before and after)	Upstream purl on main	Upstream purl on #1386
None	(no package file)	pkg:pypi/test-pkg@0.5.5	pkg:pypi/test-pkg@0.5.5	unchanged
Name	purl: {name: "test-pkg-corp"}	pkg:pypi/test-pkg-corp@0.5.5	pkg:pypi/test-pkg-corp@0.5.5	pkg:pypi/test-pkg@0.5.5
Type	purl: {type: "generic"}	pkg:generic/test-pkg@0.5.5	pkg:generic/test-pkg@0.5.5	pkg:pypi/test-pkg@0.5.5
Namespace	purl: {namespace: "acme"}	pkg:pypi/acme/test-pkg@0.5.5	pkg:pypi/acme/test-pkg@0.5.5	pkg:pypi/test-pkg@0.5.5
Global type	sbom: {purl_type: "generic"} in settings.yaml	pkg:generic/test-pkg@0.5.5	pkg:generic/test-pkg@0.5.5	pkg:pypi/test-pkg@0.5.5
Combined	purl: {name: "test-pkg-corp", version: "0.5.5+local.1", repository_url: "https://pkgs.example/simple/"}	pkg:pypi/test-pkg-corp@0.5.5%2Blocal.1?repository_url=https://pkgs.example/simple/	pkg:pypi/test-pkg-corp@0.5.5	pkg:pypi/test-pkg@0.5.5
Upstream set	purl: {type: "generic", upstream: "pkg:github/acme/test-pkg@v0.5.5"}	pkg:generic/test-pkg@0.5.5	pkg:github/acme/test-pkg@v0.5.5	unchanged

One small nit: the SbomSettings.purl_type docstring (src/fromager/packagesettings/_models.py:118) still says "Default purl type for all packages". With this change it only applies to the wheel entry, so it would be good to say that.

[mprpic]

Can you also please squash both of the commits since the second one just fixes the indentation of the header?

[kocaemre]

Thanks for checking the scenarios. I changed SbomSettings.purl_type to describe wheel package entries only and squashed the header fix into the signed-off commit (dba98b3). Local verification: uvx --from hatch hatch run test:test tests/test_sbom.py tests/test_packagesettings.py -q --no-cov --tb=short (112 passed); uvx --from hatch hatch run lint:check, uvx --from hatch hatch run mypy:check src/fromager/sbom.py src/fromager/packagesettings/_models.py tests/test_sbom.py, uvx --from hatch hatch run docs:build, and git diff --check HEAD~1..HEAD passed. CI is rerunning after the squash.

[mprpic]

I re-tested this on top of current main. A bootstrap of sqlparse==0.5.5 with name, namespace and global purl_type overrides now produces pkg:pypi/sqlparse@0.5.5 for the upstream entry, and an explicit purl.upstream is still used unchanged.

A few small non-blocking suggestions:

1. Use a versionchanged directive for the behavior change. The new section in config-reference.rst explains the change with "Previously, those settings also changed the upstream entry." A Sphinx directive would be easier to spot and matches how other changes are documented. For example:

   .. versionchanged:: 0.103.0
      ``purl.name``, ``purl.type``, ``purl.namespace``, and ``sbom.purl_type``
      no longer change the upstream entry.

   It could also go in the PurlConfig.upstream docstring, which says much the same thing as the new section.

2. Assert the upstream SPDX name in the new test. test_generate_sbom_upstream_ignores_wheel_purl_overrides only checks the upstream purl. Adding assert upstream["name"] == "test-pkg" would also cover the entry's name field, which currently comes from the purl. With the name override case, this would catch a regression that leaves the downstream name there.

3. Optional, unrelated to this PR: the SbomSettings.repository_url docstring says the qualifier is "added to every purl", but the upstream purl has never had it. You could change it to say "wheel purl" while you're editing that docstring, or leave it for a follow-up.

[EmilienM]

Needs a human read: 5 minor (4 inline, 1 below)

The purl derivation itself is correct and matches the design agreed in #1383; all findings are docs and test polish. Flagging for a human verdict because this is a deep-tier review of a contributor's change to SBOM output.

Outside the diff:

• Minor: src/fromager/packagesettings/_models.py:123, the repository_url docstring still says the qualifier is added to "every purl", but the upstream entry never carries it, the opposite of the wheel-only scoping this PR applies to purl_type right above it.

How this was reviewed

• Commit dba98b3, round 1
• Deep review, because the change alters SBOM purl generation in src/fromager/sbom.py and src/fromager/packagesettings/_models.py
• Passes: architecture, performance, security, scalability, then correctness
• Independent skeptics checked each candidate finding (1); 1 of 1 held up
• Repo rules read from main: AGENTS.md, CLAUDE.md, CONTRIBUTING.md
• Not covered: nothing skipped

[EmilienM]

Minor: When purl.type/namespace/name are set without purl.upstream, the upstream entry now changes to pkg:pypi/... with nothing in the logs to say the overrides stopped applying. A warning here would tell operators who don't read release notes:

     pc = pbi.purl_config
     if pc and pc.upstream:
         return PackageURL.from_string(pc.upstream)
 
+    if pc and (pc.type or pc.namespace or pc.name):
+        logger.warning(
+            "ignoring purl type/namespace/name overrides for the upstream"
+            " identity of %s; set purl.upstream to describe a non-PyPI source",
+            name,
+        )
     return PackageURL(

[EmilienM]

Minor: This is a user-facing semantics change, so both edited docstrings should carry a .. versionchanged:: marker like the rest of the codebase does (downloads.py, wheels.py); next release is 0.103.0. Same for the purl_type docstring above at :118.

[EmilienM]

Minor: purl.version is also wheel-only (since #1377), and the purl.upstream docstring added in this PR lists all four overrides, so the two docs disagree on the list:

Suggested change

	per-package ``purl.name``, ``purl.type``, and ``purl.namespace`` settings and
	per-package ``purl.name``, ``purl.type``, ``purl.namespace``, and
	``purl.version`` settings and

[EmilienM]

Minor: pypi purls take no namespace (the purl spec, and #1383 says so too), so this case locks a spec-invalid string in as expected output. Pairing the namespace with type: generic keeps the coverage without the invalid purl:

Suggested change

	({"namespace": "acme"}, "pypi", "pkg:pypi/acme/test-pkg@0.5.5"),
	(
	{"namespace": "acme", "type": "generic"},
	"pypi",
	"pkg:generic/acme/test-pkg@0.5.5",
	),

[kocaemre]

Thanks for re-testing and the suggestion. I added the upstream SPDX name assertion (commit 1cae5b0), so a wheel-only name override cannot silently change that field while leaving the source purl intact. I temporarily changed the upstream name to the downstream name to verify the guard: 1 expected failure (test-pkg-corp vs test-pkg); after restoring production code, 4 focused and 112 related tests passed, as did lint, mypy and git diff --check. The PR body has the exact commands. I left the optional unrelated docstring adjustment out of scope.