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

feat: Add support for new Copyrights and schema updates #3156

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dor-hayun
Copy link
Contributor

@dor-hayun dor-hayun commented Aug 22, 2024

JSON Schema

  • Added: New JSON schema version 16.0.16 with support for the new Copyrights.
  • Modified: Updated the JSONSchemaVersion parameter to use the new schema.

Package Structs

  • Added: New Copyrights field to the Package and PackageBasicData structs, similar to the existing Licenses field.
  • Added: New Copyright struct.
  • Implemented: Sorting methods for the Copyright struct.

toPackages Function

  • Changed: Updated the PackageCopyrightText to use helpers.GetCopyrights(p.Copyrights), which formats the copyright text and returns a string. Example output: "Copyright 2014-2014 Matt Zabriskie & Collaborators".

toSyftPackage Function

  • Added: Copyrights assignment to the toSyftPackage function.

also added support in CycloneDX format

@github-actions github-actions bot added the json-schema Changes the json schema label Aug 22, 2024
@kzantow
Copy link
Contributor

kzantow commented Aug 23, 2024

Generally, I think this looks pretty good @dor-hayun. A couple comments:

If we're going to add this to the Syft data model and SPDX, we should also add this to CycloneDX, since it appears to support copyright of components also.

That said, I don't see where this is ever populated -- are there some catalogers we could add this to with the initial implementation?

@dor-hayun
Copy link
Contributor Author

@kzantow I'll add this to CycloneDX as well. The goal is to enable the addition of copyright text either as part of the existing catalogers or by injecting this data directly from other sources and formatting it. For instance, I can iterate over the sbom.Artifacts to manually add the copyright, and then use the formatter to display it in SPDX format too.

@dor-hayun
Copy link
Contributor Author

@kzantow added also in CycloneDX

@dor-hayun dor-hayun force-pushed the support-copyrights branch 19 times, most recently from 43aeb7a to 1e121e8 Compare August 28, 2024 12:20
@github-actions github-actions bot removed the json-schema Changes the json schema label Aug 28, 2024
@dor-hayun dor-hayun force-pushed the support-copyrights branch 2 times, most recently from b113b35 to 052f526 Compare August 28, 2024 12:57
@github-actions github-actions bot added the json-schema Changes the json schema label Aug 28, 2024
@dor-hayun dor-hayun force-pushed the support-copyrights branch 2 times, most recently from d27e313 to abf35cc Compare August 28, 2024 13:41
go.mod Outdated Show resolved Hide resolved
Copy link
Contributor

@kzantow kzantow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a quick look and left a bit of feedback here that would need to be addressed.

A couple other things:

  • SPDX and CycloneDX should both encode and decode copyright information -- Syft should be able to read its own files
  • I do not see any catalogers which are surfacing copyrights, I don't think we would want to add to the data model without using it at all

@@ -61,11 +66,31 @@ func CommonOptions(licenseCmp LicenseComparer, locationCmp LocationComparer) []c
return true
},
),
cmp.Comparer(
func(x, y pkg.CopyrightsSet) bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be split out to a separate function, following the same pattern as the other comparers

@@ -360,12 +362,33 @@ func AssertPackagesEqual(t *testing.T, a, b pkg.Package) {
return true
},
),
cmp.Comparer(
func(x, y pkg.CopyrightsSet) bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should follow the existing patterns with a separate function

@@ -1,13 +1,13 @@
//nolint:dupl
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this considered duplicated code? we should probably reuse existing functionality if needed

syft/pkg/license_set.go Outdated Show resolved Hide resolved
- **Added**: New JSON schema version `16.0.16` with support for the new `Copyrights`.
- **Modified**: Updated the `JSONSchemaVersion` parameter to use the new schema.

- **Added**: New `Copyrights` field to the `Package` and `PackageBasicData` structs, similar to the existing `Licenses` field.
- **Added**: New `Copyright` struct.
- **Implemented**: Sorting methods for the `Copyright` struct.

- **Changed**: Updated the `PackageCopyrightText` to use `helpers.GetCopyrights(p.Copyrights)`, which formats the copyright text and returns a string. Example output: "Copyright 2014-2014 Matt Zabriskie & Collaborators".

- **Added**: `Copyrights` assignment to the `toSyftPackage` function.

Signed-off-by: dor-hayun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
json-schema Changes the json schema
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants