From ec1503c91c7826821ba0b4ac9a30247c93c648d9 Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Fri, 6 Nov 2026 23:30:54 +0530 Subject: [PATCH] fix: a component of `emit sbom` states the package, not the lock's spelling of it The bill of materials copied a lock entry's name into the component's name, bom-ref and purl, and that name is a spelling: the map key of the declaring manifest, or a qualified name for a package no manifest in view declares. `mcpplibs.cmdline` became pkg:mcpp/mcpplibs/mcpplibs.cmdline@0.0.1 while `cmdline` gave pkg:mcpp/mcpplibs/cmdline@0.0.1 for the same package, and compat.zlib became pkg:mcpp/compat/compat.zlib@1.3.2. A purl is what correlates a component with an advisory feed, so one package needs one. records.cpp already knew the identity through lock_short_name. It moves to mcpp.pm.lock_io as short_name so the two readers share it; the lock's text and the comparisons records.cpp makes do not change. A purl also percent-encodes its segments now, so `+` in a version's build metadata or in a variant's `+p` is written %2B. The `version` field stays as recorded. A variant keeps its digest in the name, since a patched library is another library. A lock written by 2026.9.29.1 to .5 can hold one identity under two spellings. Those would now share a bom-ref, which is unique in a document, so they give one component. Tests: tests/e2e/1016 writes the lock by hand and needs no toolchain or network. PmLockIo.TheShortNameIsTheSameWhicheverWayTheManifestSpelledIt covers the function. docs/51 states the rule in both languages. Closes #892 --- docs/51-supported-versions.md | 5 + docs/zh/51-supported-versions.md | 4 + src/build/prepare/records.cpp | 19 +-- src/cli/cmd_sbom.cppm | 46 +++++-- src/pm/lock_io.cppm | 17 +++ ...of_materials_states_a_packages_identity.sh | 126 ++++++++++++++++++ tests/unit/test_pm_lock_io.cpp | 19 +++ 7 files changed, 208 insertions(+), 28 deletions(-) create mode 100755 tests/e2e/1016_the_bill_of_materials_states_a_packages_identity.sh diff --git a/docs/51-supported-versions.md b/docs/51-supported-versions.md index fef07b562..2e8fb13c3 100644 --- a/docs/51-supported-versions.md +++ b/docs/51-supported-versions.md @@ -110,6 +110,11 @@ resolution: mcpp emit sbom -o sbom.json ``` +A component's `name`, `group` and `purl` state the package's identity, whichever +way the manifest that declared it spelled it: `cmdline` and `mcpplibs.cmdline` +are both `pkg:mcpp/mcpplibs/cmdline@`. A variant keeps its digest in +the name, and a purl percent-encodes its segments, so `+` is written `%2B`. + It reads `mcpp.lock` rather than resolving again, because a document describing a different graph from the one that was built is worse than no document. A component whose licence mcpp does not know is emitted as `NOASSERTION` rather diff --git a/docs/zh/51-supported-versions.md b/docs/zh/51-supported-versions.md index aefe888fb..bc396d018 100644 --- a/docs/zh/51-supported-versions.md +++ b/docs/zh/51-supported-versions.md @@ -95,6 +95,10 @@ error: --locked was given and this resolution differs from mcpp.lock: mcpp emit sbom -o sbom.json ``` +组件的 `name`、`group` 与 `purl` 表达包的身份，与声明它的 manifest 如何拼写无关： +`cmdline` 与 `mcpplibs.cmdline` 都是 `pkg:mcpp/mcpplibs/cmdline@`。变体在 +名字里保留其摘要；purl 的各段经过百分号编码，因此 `+` 写作 `%2B`。 + 它读取 `mcpp.lock` 而不是重新解析，因为一份描述了与实际构建不同的依赖图的 文档，比没有文档更糟。许可证 mcpp 无法识别的组件会被写成 `NOASSERTION` 而不是 省略：缺失的字段读起来像「未检查」。 diff --git a/src/build/prepare/records.cpp b/src/build/prepare/records.cpp index aea64a660..52fe93872 100644 --- a/src/build/prepare/records.cpp +++ b/src/build/prepare/records.cpp @@ -81,24 +81,9 @@ namespace mcpp::build { // runs, called from it in the same order, and are declared in the `:state` // partition (state.cppm) as the phases are. -// The short name of a lock entry. An index entry is written under the map key -// its declaring manifest used (`mcpplibs.cmdline` under `[dependencies.mcpplibs]`, -// `cmdline` under a bare key), so the spelled name may carry its namespace as a -// prefix; the identity is (namespace, short name) whichever way it was spelled. -// An entry without a namespace (a git entry, keyed by the root's map key) is its -// own short name. -static std::string lock_short_name(const mcpp::lockfile::LockedPackage& p) { - if (!p.namespace_.empty()) { - const std::string prefix = p.namespace_ + "."; - if (p.name.size() > prefix.size() && p.name.starts_with(prefix)) - return p.name.substr(prefix.size()); - } - return p.name; -} - // The identity two entries of one lock are compared by. static std::string lock_identity(const mcpp::lockfile::LockedPackage& p) { - return p.namespace_ + "\x1f" + lock_short_name(p); + return p.namespace_ + "\x1f" + mcpp::pm::short_name(p); } std::expected step13_lockfile(PrepareState& state, BuildContext& ctx) { @@ -435,7 +420,7 @@ std::expected step13_lockfile(PrepareState& state, BuildConte lockPath.string())); } auto key = [](const mcpp::lockfile::LockedPackage& p) { - const std::string sn = lock_short_name(p); + const std::string sn = mcpp::pm::short_name(p); return p.namespace_.empty() ? sn : p.namespace_ + "." + sn; }; diff --git a/src/cli/cmd_sbom.cppm b/src/cli/cmd_sbom.cppm index ad37d59df..6315db8fb 100644 --- a/src/cli/cmd_sbom.cppm +++ b/src/cli/cmd_sbom.cppm @@ -56,13 +56,35 @@ std::string esc(std::string_view s) { return o; } +// One segment of a package URL: the purl specification percent-encodes all but +// the unreserved characters and `:`. `+` is the one that occurs here, in a +// version's build metadata and in a variant's name. +std::string purl_segment(std::string_view s) { + std::string o; + o.reserve(s.size()); + for (char c : s) { + const auto u = static_cast(c); + if (std::isalnum(u) || c == '.' || c == '-' || c == '_' || c == '~' || c == ':') + o += c; + else + o += std::format("%{:02X}", static_cast(u)); + } + return o; +} + +std::string purl(std::string_view ns, std::string_view name, std::string_view version) { + return std::format("pkg:mcpp/{}/{}@{}", purl_segment(ns.empty() ? "mcpp" : ns), + purl_segment(name), purl_segment(version)); +} + // A package URL for an mcpp package. `pkg:` URLs are how every SBOM consumer // correlates a component with an advisory feed, so emitting one is what makes -// the document useful rather than merely well-formed. +// the document useful rather than merely well-formed. It therefore states the +// package's identity, not the spelling of the manifest that declared it: the +// lock may record `mcpplibs.cmdline` or `cmdline` for one package. A variant +// keeps its `+p` in the name: a patched library is another library. std::string purl(const mcpp::lockfile::LockedPackage& p) { - const std::string ns = p.namespace_.empty() ? std::string("mcpp") - : p.namespace_; - return std::format("pkg:mcpp/{}/{}@{}", ns, p.name, p.version); + return purl(p.namespace_, mcpp::pm::short_name(p), p.version); } // AN UNKNOWN LICENCE IS REPORTED AS UNKNOWN, NEVER OMITTED AND NEVER @@ -119,9 +141,13 @@ int cmd_sbom(const mcpplibs::cmdline::ParsedArgs& parsed) { "dependencies, build it once so the resolution is recorded."); } + // One component per package: a lock written by 2026.9.29.1 to .5 can hold + // one identity under two spellings, and a bom-ref is unique in a document. + std::set listed; + std::erase_if(pkgs, [&](const auto& d) { return !listed.insert(purl(d)).second; }); + const auto& p = man->package; - const std::string rootNs = p.namespace_.empty() ? std::string("mcpp") - : p.namespace_; + const std::string rootPurl = purl(p.namespace_, p.name, p.version); std::string out; out += "{\n"; @@ -133,13 +159,11 @@ int cmd_sbom(const mcpplibs::cmdline::ParsedArgs& parsed) { + esc(std::string(mcpp::MCPP_VERSION)) + "\" } ],\n"; out += " \"component\": {\n"; out += " \"type\": \"application\",\n"; - out += " \"bom-ref\": \"" + esc(std::format("pkg:mcpp/{}/{}@{}", - rootNs, p.name, p.version)) + "\",\n"; + out += " \"bom-ref\": \"" + esc(rootPurl) + "\",\n"; out += " \"name\": \"" + esc(p.name) + "\",\n"; out += " \"version\": \"" + esc(p.version) + "\",\n"; out += licence_block(p.license) + "\n"; - out += " \"purl\": \"" + esc(std::format("pkg:mcpp/{}/{}@{}", - rootNs, p.name, p.version)) + "\"\n"; + out += " \"purl\": \"" + esc(rootPurl) + "\"\n"; out += " }\n"; out += " },\n"; out += " \"components\": ["; @@ -151,7 +175,7 @@ int cmd_sbom(const mcpplibs::cmdline::ParsedArgs& parsed) { out += " {\n"; out += " \"type\": \"library\",\n"; out += " \"bom-ref\": \"" + esc(purl(d)) + "\",\n"; - out += " \"name\": \"" + esc(d.name) + "\",\n"; + out += " \"name\": \"" + esc(mcpp::pm::short_name(d)) + "\",\n"; out += " \"version\": \"" + esc(d.version) + "\",\n"; if (!d.namespace_.empty()) out += " \"group\": \"" + esc(d.namespace_) + "\",\n"; diff --git a/src/pm/lock_io.cppm b/src/pm/lock_io.cppm index 7e486ae9d..064d39e9f 100644 --- a/src/pm/lock_io.cppm +++ b/src/pm/lock_io.cppm @@ -67,6 +67,14 @@ struct LockedGitSource { std::optional parse_git_source(std::string_view source); +// The short name of a lock entry. An index entry is written under the map key +// its declaring manifest used (`mcpplibs.cmdline` under `[dependencies.mcpplibs]`, +// `cmdline` under a bare key), so the spelled name may carry its namespace as a +// prefix; the identity is (namespace, short name) whichever way it was spelled. +// An entry without a namespace (a git entry, keyed by the root's map key) is its +// own short name. +std::string short_name(const LockedPackage& p); + struct Lockfile { int schemaVersion = 2; std::vector indices; @@ -107,6 +115,15 @@ namespace mcpp::pm { namespace t = mcpp::libs::toml; +std::string short_name(const LockedPackage& p) { + if (!p.namespace_.empty()) { + const std::string prefix = p.namespace_ + "."; + if (p.name.size() > prefix.size() && p.name.starts_with(prefix)) + return p.name.substr(prefix.size()); + } + return p.name; +} + std::expected load(const std::filesystem::path& path) { if (!std::filesystem::exists(path)) { return Lockfile{}; // no lock yet diff --git a/tests/e2e/1016_the_bill_of_materials_states_a_packages_identity.sh b/tests/e2e/1016_the_bill_of_materials_states_a_packages_identity.sh new file mode 100755 index 000000000..e2469f12b --- /dev/null +++ b/tests/e2e/1016_the_bill_of_materials_states_a_packages_identity.sh @@ -0,0 +1,126 @@ +#!/usr/bin/env bash +# requires: python3 +# 1016 -- a component of `mcpp emit sbom` names the package, not the spelling +# of the manifest that declared it. The purl is what correlates a component with +# an advisory feed, so one package has one purl. +# +# The command reads the lock and nothing else, so the lock is written by hand +# here: no toolchain, no index and no network. +# +# Criteria: +# A. `cmdline` and `mcpplibs.cmdline` are one name and one purl, and a +# transitive `compat.zlib` is not `pkg:mcpp/compat/compat.zlib`. +# B. A variant keeps its digest and stays another component. +# C. A purl percent-encodes `+`; the `version` field stays as recorded. +# D. An entry with no namespace (a git dependency) keeps its name whole. +# E. A lock that holds one identity under two spellings gives one component. +set -e + +MCPP="${MCPP:-mcpp}" +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT +cd "$work" + +cat > mcpp.toml <<'TOML' +[package] +name = "app" +version = "0.1.0+7" +TOML + +cat > mcpp.lock <<'TOML' +version = 2 + +[package."mcpplibs.cmdline"] +namespace = "mcpplibs" +version = "0.0.1" +source = "index+mcpplibs@0.0.1" +hash = "fnv1a:0000000000000001" + +[package."compat.zlib"] +namespace = "compat" +version = "1.3.2" +source = "index+compat@1.3.2" +hash = "fnv1a:0000000000000002" + +[package."compat.zlib+pabcdef012345"] +namespace = "compat" +version = "1.3.2" +source = "index+compat@1.3.2" +hash = "fnv1a:0000000000000003" +variant = "abcdef0123456789" +patches = ["patches/0001-fix.patch#sha256:00"] + +[package."fmt"] +namespace = "mcpplibs" +version = "11.0.0+7" +source = "index+mcpplibs@11.0.0+7" +hash = "fnv1a:0000000000000004" + +[package."acme.tool"] +version = "1.0.0" +source = "git+https://example.invalid/acme/tool#tag=v1.0.0" +hash = "fnv1a:0000000000000005" +TOML + +"$MCPP" emit sbom -o sbom.json >/dev/null 2>&1 || { echo "FAIL: mcpp emit sbom"; exit 1; } +python3 - <<'PY' || exit 1 +import json +d = json.load(open("sbom.json")) +got = {(c.get("group", ""), c["name"]): c for c in d["components"]} +want = { + ("mcpplibs", "cmdline"): "pkg:mcpp/mcpplibs/cmdline@0.0.1", + ("compat", "zlib"): "pkg:mcpp/compat/zlib@1.3.2", + ("compat", "zlib+pabcdef012345"): "pkg:mcpp/compat/zlib%2Bpabcdef012345@1.3.2", + ("mcpplibs", "fmt"): "pkg:mcpp/mcpplibs/fmt@11.0.0%2B7", + ("", "acme.tool"): "pkg:mcpp/mcpp/acme.tool@1.0.0", +} +assert set(got) == set(want), sorted(got) +for key, purl in want.items(): + assert got[key]["purl"] == purl, (key, got[key]["purl"]) + assert got[key]["bom-ref"] == purl, (key, got[key]["bom-ref"]) +assert got[("mcpplibs", "fmt")]["version"] == "11.0.0+7", got[("mcpplibs", "fmt")] +assert "pedigree" in got[("compat", "zlib+pabcdef012345")] +root = d["metadata"]["component"] +assert root["purl"] == "pkg:mcpp/mcpp/app@0.1.0%2B7", root["purl"] +assert root["version"] == "0.1.0+7", root["version"] +PY +echo " ok a component states the package's identity, and its purl is encoded" + +# The bare spelling of the package gives the purl the qualified spelling gave. +cat > mcpp.lock <<'TOML' +version = 2 + +[package."cmdline"] +namespace = "mcpplibs" +version = "0.0.1" +source = "index+mcpplibs@0.0.1" +hash = "fnv1a:0000000000000001" +TOML +"$MCPP" emit sbom -o bare.json >/dev/null 2>&1 || { echo "FAIL: mcpp emit sbom (bare)"; exit 1; } +python3 - <<'PY' || exit 1 +import json +one = {c["purl"] for c in json.load(open("bare.json"))["components"]} +all_ = {c["purl"] for c in json.load(open("sbom.json"))["components"]} +assert one == {"pkg:mcpp/mcpplibs/cmdline@0.0.1"}, one +assert one <= all_, (one, all_) +PY +echo " ok a bare and a qualified spelling are one purl" + +# A lock from 2026.9.29.1 to .5 can hold one identity twice. +cat >> mcpp.lock <<'TOML' + +[package."mcpplibs.cmdline"] +namespace = "mcpplibs" +version = "0.0.1" +source = "index+mcpplibs@0.0.1" +hash = "fnv1a:0000000000000001" +TOML +"$MCPP" emit sbom -o twice.json >/dev/null 2>&1 || { echo "FAIL: mcpp emit sbom (twice)"; exit 1; } +python3 - <<'PY' || exit 1 +import json +refs = [c["bom-ref"] for c in json.load(open("twice.json"))["components"]] +assert refs == ["pkg:mcpp/mcpplibs/cmdline@0.0.1"], refs +PY +echo " ok one identity under two spellings is one component" + +echo "PASS: the bill of materials states a package's identity" diff --git a/tests/unit/test_pm_lock_io.cpp b/tests/unit/test_pm_lock_io.cpp index d0d3c146d..8e55c6b1c 100644 --- a/tests/unit/test_pm_lock_io.cpp +++ b/tests/unit/test_pm_lock_io.cpp @@ -216,6 +216,25 @@ TEST(PmLockIo, UpdatingALinkedLockKeepsTheLinkAndUpdatesItsTarget) { // An entry names the configurations that resolve it, and reads back as written // (SPEC-013 §15.1); a lock written before the field reads with none. +TEST(PmLockIo, TheShortNameIsTheSameWhicheverWayTheManifestSpelledIt) { + auto entry = [](std::string name, std::string ns) { + mcpp::pm::LockedPackage p; + p.name = std::move(name); + p.namespace_ = std::move(ns); + return p; + }; + EXPECT_EQ(mcpp::pm::short_name(entry("cmdline", "mcpplibs")), "cmdline"); + EXPECT_EQ(mcpp::pm::short_name(entry("mcpplibs.cmdline", "mcpplibs")), "cmdline"); + // A variant keeps its digest: it is another library. + EXPECT_EQ(mcpp::pm::short_name(entry("compat.zlib+pabc", "compat")), "zlib+pabc"); + // Only the entry's own namespace is a prefix. + EXPECT_EQ(mcpp::pm::short_name(entry("other.zlib", "compat")), "other.zlib"); + // A git entry has no namespace, and is its own short name. + EXPECT_EQ(mcpp::pm::short_name(entry("acme.tool", "")), "acme.tool"); + // The namespace alone is not a prefix of anything. + EXPECT_EQ(mcpp::pm::short_name(entry("compat.", "compat")), "compat."); +} + TEST(PmLockIo, TargetsRoundTrip) { const auto dir = std::filesystem::temp_directory_path() / std::format("mcpp-lock-{}", std::chrono::steady_clock::now().time_since_epoch().count());