Conversation
…eStrategy Foundational types for a strategy-driven BOM merge engine: a marker interface (IBomEntity) plus IMergeable<T>/IEquivalent<T>, each with a default interface method body that falls back to the type's own IEquatable<T> equality. Most model classes will need nothing more than declaring the interface (zero-body opt-in); only types with real reconciliation logic override the defaults. Compiled for net8.0+/net10.0 only (default interface methods require an ABI this library's netstandard2.0 target cannot provide); netstandard2.0 consumers keep today's merge behavior unchanged. MergeStrategy configures the merge: independent on/off toggles for subset-dependency merging, dependency-as-extra-property treatment, and metadata refresh, plus a ComponentConflictResolution enum (rather than more booleans) so new resolution algorithms can be added as new cases without reshaping this type or any call site that reads it. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
This was referenced Sep 15, 2026
Declares the new interfaces on every model type the merge engine needs to process generically as list elements (Tool, Service, ExternalReference, Dependency, Composition, Vulnerability, Annotation, Standard, Assessor, Attestation, Claim, OrganizationalEntity, Component, plus nested-list element types Hash, Property, OrganizationalContact, LicenseChoice, PatentAssertion). For types that already implement IEquatable<T>, this is a one-line, zero-body change (the interface defaults handle it). Property, OrganizationalContact, LicenseChoice and PatentAssertion had no equality implementation at all; they gain the same JSON-hash-based Equals(object)/ Equals(T)/GetHashCode pattern already used by Component/Annotation/etc. elsewhere in this codebase (all three together, not just Equals(T)/ GetHashCode, to keep the object.Equals/GetHashCode contract consistent for callers that only see the non-generic Equals), so the interface defaults have something real to fall back on. Hash already had Equals(T)/GetHashCode but was missing the same Equals(object) override; added here too since this commit already touches the file. Hash gets a real (non-default) Equivalent/MergeWith: two hashes of the same algorithm should carry the same content; if one side is missing content, fill it in from the other, and treat a genuine content mismatch for the same algorithm as an unmergeable conflict. Component's real merge logic follows in a separate commit. Signed-off-by: Jim Klimov <jimklimov@gmail.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Generic-constrained (T : IEquatable<T>, IEquivalent<T>, IMergeable<T>) list merge: exact-equal items dedupe as before, equivalent-but-unequal items attempt MergeWith, everything else is kept as a separate entry. Falls back to the existing exact-match ListMergeHelper<T> when a strategy disables entity merging. One implementation serves every mergeable list field in a Bom (components, services, hashes, external references, authors, ...) via real interface dispatch -- no reflection, no per-type branching. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
Implements Component's merge logic as explicit per-field code instead of a PropertyInfo/switch(Type) walk: Equivalent() checks type/name required-equal, version/group/purl equal-if-both-present (bom-ref deliberately excluded -- that's the merge orchestration's concern, not a per-field check). MergeWith() folds scalar fields via ??, list fields via the new MergeableListHelper.Merge (lives in CycloneDX.Core rather than CycloneDX.Utils so Component can call it directly -- Utils depends on Core, not the reverse), and Scope via TryMergeScope, which handles the both-Excluded case explicitly: two components that are both Excluded-scope but differ in some unrelated field should still merge, not be treated as an unresolvable scope conflict just because neither side is Optional. MergeableListHelper also gains small single-value/string-list/ nullable-bool merge helpers used by Component's scalar and simple-list fields. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
BomRefWalker.RewriteRefs(bom, rewrite) generalizes the per-type ref-
rewriting CycloneDXUtils.HierarchicalMerge already hand-rolls for
bom-ref namespacing (NamespaceComponentBomRefs, NamespaceDependencyBomRefs,
NamespaceCompositions, NamespaceVulnerabilitiesRefs,
NamespaceAnnotationsBomRefs) into one reusable entry point parameterized
on an arbitrary rewrite function instead of a fixed namespace prefix.
Namespacing becomes RewriteRefs(bom, r => $"{ns}:{r}"); a single manual
rename becomes RewriteRefs(bom, r => r == oldRef ? newRef : r).
Covers Metadata.Component, Components, Services, Dependencies,
Compositions, Vulnerabilities, and Annotations; the newer (CycloneDX
1.6) Declarations/Definitions sections are not yet walked -- a
mechanical follow-up, not an architectural one.
Bom.RenameRef builds on the walker to rename a bom-ref and every
back-reference to it throughout a document in one pass.
BomMetadataReferThisToolkit/BomMetadataUpdate stamp this library's
(and the entry assembly's) Tool reference into Metadata.Tools, and
refresh Version/SerialNumber/Timestamp.
Signed-off-by: Jim Klimov <jimklimov@gmail.com>
New FlatMerge(bom1, bom2, MergeStrategy) and HierarchicalMerge(boms, bomSubject, MergeStrategy) overloads sit alongside the existing fixed-behavior ones (which are untouched and keep today's exact behavior for existing callers). FlatMerge's overload swaps every ListMergeHelper<T> call for MergeableListHelper.Merge(..., strategy), so Components/Services/Tools/Hashes/etc. attempt real reconciliation (Component's Scope-aware squash, Hash's content fill-in) instead of only deduping exact matches, and applies RenameConflictingComponents (pre-merge bom-ref collision detection via BomRefWalker) and the metadata-update toggles. Known gaps, called out rather than silently dropped: MergeSubsetDependencies isn't wired to real subset-detection logic yet (Dependency still merges via its IMergeable<T> default), and HierarchicalMerge's strategy overload only adds the metadata-update toggles so far (namespacing already avoids the collisions FlatMerge's squash logic exists to resolve, so it needed less new behavior here). Signed-off-by: Jim Klimov <jimklimov@gmail.com>
Unit tests (not full-BOM snapshots, for faster feedback) covering: Component.Equivalent's type/name/version-if-both-present rules and its deliberate bom-ref exclusion; Component.MergeWith's scope squash, excluded-vs-required conflict refusal, and the both-Excluded case (two components that are both Excluded-scope but differ in some unrelated field still merge); Hash.MergeWith's content fill-in/mismatch cases; FlatMerge(bom1, bom2, strategy) actually squashing equivalent components across two BOMs; and Bom.RenameRef rewriting both an identifier and its back-reference (plus the not-found no-op case). Signed-off-by: Jim Klimov <jimklimov@gmail.com>
The earlier commit only added the 2-BOM FlatMerge(bom1, bom2, strategy) overload; the CLI's merge command (and anyone merging more than two documents) needs the IEnumerable<Bom> form too. Also fixes a now- ambiguous overload resolution: the existing single-arg FlatMerge(boms) delegated via `FlatMerge(boms, null)`, which became ambiguous between the Component and MergeStrategy overloads once the latter existed -- disambiguated with an explicit (Component)null cast. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
… enum MergeStrategy.Default()'s initial choice for resolving a Required-vs- Optional Scope conflict picked the narrower Optional reading. On reflection that is the wrong default: the specification says an absent/ambiguous Scope SHOULD be treated as required, so silently downgrading a Required dependency to Optional risks under-reporting it in downstream tooling (e.g. vulnerability scanners that key off Scope). Flips MergeStrategy.Default() to Squash_UpgradeScope and renames the enum values to contrast directly: Squash -> Squash_DowngradeScope, SquashUpgradeScope -> Squash_UpgradeScope. Also reserves Squash_RenameByScope (replacing the placeholder RenameByScope) and wires TryMergeScope to refuse merging differently-scoped components under it, ahead of implementing the actual scope-partitioning pass in a follow-up commit. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
RenameRef previously rewrote every occurrence of oldRef to newRef unconditionally -- if newRef already identified (or was referenced by) some other entity in the document, the rename would silently make two different entities share one bom-ref, or repoint an existing back-reference at the wrong entity. Now does a read-only collection pass first (reusing BomRefWalker's traversal, so "what counts as a ref" can't drift between the check and the real rewrite) and throws InvalidOperationException if newRef is already in use, instead of performing the corrupting rename. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
Independent of the RenameRef collision fix (different code path: this is about the generic Components/Dependencies list merge, not the rename-entity walker) -- addresses the gap where two BOMs describing the same component with different direct-dependency lists (e.g. a Maven module built standalone vs. as part of a parent build) would each contribute their own <dependency ref="X"> entry, since Dependency previously only merged via IMergeable<T>'s exact-equality default. Equivalent() matches on Ref alone. MergeWith() unions the two Dependencies sub-lists (via the same MergeableListHelper.Merge used everywhere else, so it recurses correctly into nested dependency trees), gated by MergeStrategy.MergeSubsetDependencies: when that's false, a genuine difference between the two dependsOn sets is treated as a real conflict (refuse, keep both entries as before) rather than silently unioned. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
… bom-ref When two Equivalent components (same type/name/version-if-present) differ in Scope, they are kept as two distinct entries instead of squashed or refused, suffixed :scope=<value> (e.g. "lp:scope=Required" / "lp:scope=Excluded"), with every back-reference in the document rewritten to match (via BomRefWalker, per-source-bom, before the generic Components merge runs). No suffix is ever added unless a real conflict appears: if every source agrees on Scope for a given identity, the original bom-ref is kept as-is (ApplyRenameByScope only mutates on an actual scope mismatch). A retroactive rename handles the first-ever split (the already-accumulated entry, and any of its back-references already recorded from earlier merged BOMs, get suffixed too, not just the new arrival); a third+ incoming copy matching an existing scope partition squashes into it independently of the others. TryMergeScope now refuses to merge differing scopes at all under this resolution (partitioning happens via bom-ref splitting instead, not field-level squashing). RenameBomRefCollisions (the existing same- bomref-but-unequal check for RenameConflictingComponents) now skips pairs that are Equivalent to each other when this resolution is active, so it doesn't rename the pair with a blunt ":2" suffix before ApplyRenameByScope gets a chance to do it precisely. Fixes a stale-closure bug caught by a test during development: the rewrite lambda in ApplyRenameByScope compared against `incoming.BomRef` read live from the (mutating) property instead of a captured snapshot, so a component's own bom-ref rename would silently poison the comparison used for its own back-references later in the same walk pass. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
jimklimov
force-pushed
the
merge-cleanup-empty-lists-deep
branch
from
September 17, 2026 09:30
eb37fdc to
278342f
Compare
The comment above result.Dependencies still claimed reconciliation beyond exact-match was 'not yet implemented', left over from before this same PR's 'Dependency: real Equivalent/MergeWith for subset- dependency merging' commit gave Dependency real MergeWith logic. Update the comment to describe what actually happens now. Signed-off-by: Jim Klimov <jimklimov@gmail.com> Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rges CleanupMetadataComponent(bom, strategy): a document's Metadata.Component describes the subject of the BOM, not one of its own parts, but callers that auto-select a subject from an input BOM's own metadata can end up with that same component also present in the top-level Components list -- two entries sharing one bom-ref, which the specification does not allow. This detects that and removes the duplicate, first folding in (via MergeWith) whatever fields the duplicate carried that the kept entry didn't already have. CleanupEmptyLists(bom): replaces empty top-level list properties with null, so a merged document doesn't serialize e.g. an empty "components": [] for a section nothing ended up populating. Both are called from the end of the strategy-aware FlatMerge and HierarchicalMerge overloads. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
… not Contains() When merging many BOMs (e.g. one per module in a large multi-module build), a module's own rich self-description (its Metadata.Component) often needs to be reconciled with a thinner, dependency-only entry for the same real-world package already contributed by some other module's Components list. The exact-equality List<T>.Contains() check used to decide whether to add that self-description as a new Components entry can't recognize this -- thin and rich descriptions are never Equals() -- so it silently added a second, duplicate-bom-ref entry instead of folding into the existing one. At the scale of a real multi-hundred- module merge this produced hundreds of duplicate bom-refs, a spec violation. Reconciles via Equivalent()/MergeWith() instead, matching how MergeableListHelper.Merge already handles list-to-list entries. Also fixes a related edge case surfaced by the same code path: a leaf module with no Components of its own (so the merged list was still null at this point) previously had its own self-description silently dropped, since the guard required a non-null list before even attempting to add it. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
…ash anyway RenameBomRefCollisions renamed bom2's copy of any component sharing a bom-ref with a not-Equals() bom1 component, unconditionally -- even when the two were Equivalent() and the subsequent Components merge (MergeableListHelper.Merge, via Equivalent()/MergeWith()) was going to successfully fold them into one entry anyway. Since that merge is bom-ref-blind, the survivor keeps bom1's (unrenamed) bom-ref; the rename's own side effect -- rewriting every back-reference in bom2's own dependency graph to the new, suffixed ref -- had already happened and was never undone. At real-world scale (merging many per-module Maven SBOMs where the same shared dependency is described slightly differently by different modules) this produced dangling dependsOn entries pointing at a bom-ref no component actually carried. Predicts the same outcome Component.MergeWith would reach -- via the now-public Component.TryMergeScope, the one place that reconciliation can still refuse -- and only renames when the merge would genuinely refuse and leave both entries in place (KeepSeparate always refuses; otherwise only a real Scope conflict does). Signed-off-by: Jim Klimov <jimklimov@gmail.com>
FlatMerge unions every input's own Components and Dependencies, but never guarantees the union stays one connected graph -- that depends entirely on dependsOn edges the inputs already had. A flat merge of many independently-generated documents (e.g. one per module in a large multi-module build) can easily leave some of them unreachable from the document's own subject: structurally valid per the schema, but invisible to any consumer that walks the dependency graph from the root rather than scanning the flat Components list (Dependency- Track being a common example). Bom.AttachDanglingComponents finds every top-level component (in Metadata.Component and Components) that no dependsOn edge anywhere in Dependencies ever targets, buckets what it finds by Scope (Excluded, Optional, Required, unspecified), and gives each non-empty bucket its own synthetic "unreferenced-components:scope=X" Component wired into the graph as a child of a caller-specified attachment point (falling back to the document's own subject if not given or not found). Returns what it attached, keyed by the new attachment components' own bom-refs. Idempotent -- running it again after nothing changed finds nothing left to attach. Signed-off-by: Jim Klimov <jimklimov@gmail.com>
BomUtils.GetBomForSerialization only copies+downgrades a BOM when its SpecVersion differs from SpecificationVersionHelpers.CurrentVersion; for the current version it serializes the object graph as-is, so empty (non-null, zero-count) lists like a Component's "licenses": [] or a Dependency's "dependsOn": []/"provides": [] survive untouched. Every older target version already loses these for free, as an undocumented side effect of the Protobuf round-trip CopyBomAndDowngrade uses for its deep copy (proto3 can't distinguish an empty repeated field from an absent one). CleanupEmptyListsDeep makes that omission consistent and intentional across all spec versions, by reflecting over the entire object graph and nulling out any empty list property that's actually JSON-serialized (skipping [JsonIgnore] members -- the Protobuf-only *_Protobuf mirror properties must not be touched, since setting them has side effects on the property they mirror). Complements the existing top-level-only CleanupEmptyLists. Not required for schema validity in any spec version 1.4-1.7 -- none of these list properties are required or carry minItems -- purely cosmetic. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Jim Klimov <jimklimov@gmail.com>
jimklimov
force-pushed
the
merge-cleanup-empty-lists-deep
branch
from
September 17, 2026 10:25
278342f to
2e3a2c2
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Stacked on #449, #448, #447, #446 (the full merge-strategy stack).
Merge.cs: addsCleanupEmptyListsDeep(), a recursive empty-list pruner that walks a mergedBomand clears out empty collections left behind anywhere in the object graph (not just the top-level ones
CleanupEmptyLists()from #448 already handles), so a merged document doesn't carry aroundspec-technically-valid-but-noisy
[]properties.Why
New capability, no earlier PR — a natural finishing touch once the rest of the merge-strategy stack
existed.
cyclonedx-cliexposes this asconvert/merge --strip-empty-listsin a follow-up CLI PR.PR 5 of 5, last one in this stack.