Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ProtoBuf,XML): component data repeatable #530

Merged
merged 4 commits into from
Nov 7, 2024

Conversation

jkowalleck
Copy link
Member

fixes #518

@jkowalleck
Copy link
Member Author

@nscuro , could I ask you for a review?

would you consider the proposed changes as non-breaking?
I got the impression this would be a non-breaking based on this: #518 (comment)

Despite bufbuild/buf:1.30.1 thinks this would be a breaking change ...

@nscuro
Copy link
Member

nscuro commented Nov 3, 2024

@jkowalleck You are using the config category FILE for buf's breaking change detection:

version: v1
breaking: # https://buf.build/docs/configuration/v1/buf-yaml#breaking
use: # see https://buf.build/docs/breaking/overview#rules-and-categories
- FILE
- WIRE_JSON

The docs define FILE as:

Detects generated source code breakage on a per-file basis

So, while the proposed change is compatible WRT the wire format, source code generated from the schema will break. For example in Java, optional ComponentData data would be accessible as:

// Returns ComponentData
component.getData()

whereas repeated ComponentData data would be:

// Returns List<ComponentData>
component.getDataList()

Similarly, construction of Proto objects will be different, too.

It's safe to assume that almost all language bindings will mirror this behavior.

IMO this is fine. I think the spec should focus on wire compatibility.

@jkowalleck
Copy link
Member Author

jkowalleck commented Nov 3, 2024

thank you so much, @nscuro.

I am not very experienced with ProtoBuf, so I might have configured the checks unideal.

IMO this is fine. I think the spec should focus on wire compatibility.

This sounds plausible.
Would you say it is safe to change the breaking detection to use "wire" only, by dropping "file"?
If so, I will change the applied configs via #532

@stevespringett
Copy link
Member

LGTM

jkowalleck added a commit that referenced this pull request Nov 4, 2024
our spec describes how data models look in data transfers.
current protobuf breaking detection adheres this.

the protobuf breaking detection also does unnecessary detections, which
should not matter for our domain.
they are removed, here.

----


changes are based on
#530 (comment)

---------

Signed-off-by: Jan Kowalleck <[email protected]>
Co-authored-by: andreas-hilti <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
@jkowalleck jkowalleck merged commit 8858683 into CycloneDX:master Nov 7, 2024
6 of 8 checks passed
@jkowalleck jkowalleck deleted the fix/component-data-repeated branch November 7, 2024 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Defect]: Inconsistency of formats: can data in data components be repeated?
4 participants