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

Add SPDX JSON format object #584

Merged
merged 10 commits into from
Oct 29, 2021
Merged

Add SPDX JSON format object #584

merged 10 commits into from
Oct 29, 2021

Conversation

wagoodman
Copy link
Contributor

This is a follow up from #578

Continues on with the format pattern introduced in #550 with the spdx22json format. This is the same implementation that existed with the presenters, but does not support any additional capabilities (decoding or validation).

Additional work also done:

  • migrated SPDX helpers under the formats package and split up the implementation into separate files

@github-actions
Copy link

github-actions bot commented Oct 22, 2021

Benchmark Test Results

Benchmark results from the latest changes vs base branch
name                                                   old time/op    new time/op    delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2          1.52ms ± 7%    1.20ms ± 2%  -20.75%  (p=0.008 n=5+5)
ImagePackageCatalogers/python-package-cataloger-2        2.99ms ± 2%    2.43ms ± 2%  -18.67%  (p=0.008 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2     894µs ± 2%     740µs ± 5%  -17.22%  (p=0.008 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                 913µs ± 1%     755µs ± 1%  -17.33%  (p=0.016 n=5+4)
ImagePackageCatalogers/rpmdb-cataloger-2                  925µs ± 2%     840µs ±25%     ~     (p=0.151 n=5+5)
ImagePackageCatalogers/java-cataloger-2                  12.7ms ± 2%    11.1ms ± 6%  -13.18%  (p=0.008 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                 1.22ms ± 1%    1.07ms ±14%     ~     (p=0.095 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2       752ns ± 3%     627ns ± 5%  -16.66%  (p=0.008 n=5+5)

name                                                   old alloc/op   new alloc/op   delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2           227kB ± 0%     228kB ± 0%   +0.38%  (p=0.016 n=5+5)
ImagePackageCatalogers/python-package-cataloger-2         980kB ± 0%     980kB ± 0%   -0.04%  (p=0.016 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2     192kB ± 0%     192kB ± 0%     ~     (p=0.222 n=5+5)
ImagePackageCatalogers/dpkgdb-cataloger-2                 211kB ± 0%     211kB ± 0%   -0.16%  (p=0.008 n=5+5)
ImagePackageCatalogers/rpmdb-cataloger-2                  217kB ± 0%     217kB ± 0%   -0.01%  (p=0.016 n=5+4)
ImagePackageCatalogers/java-cataloger-2                  3.12MB ± 0%    3.12MB ± 0%     ~     (p=0.056 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                 1.26MB ± 0%    1.26MB ± 0%     ~     (p=0.095 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2        336B ± 0%      336B ± 0%     ~     (all equal)

name                                                   old allocs/op  new allocs/op  delta
ImagePackageCatalogers/ruby-gemspec-cataloger-2           5.46k ± 0%     5.46k ± 0%     ~     (all equal)
ImagePackageCatalogers/python-package-cataloger-2         18.1k ± 0%     18.1k ± 0%     ~     (p=0.540 n=5+5)
ImagePackageCatalogers/javascript-package-cataloger-2     4.80k ± 0%     4.80k ± 0%     ~     (all equal)
ImagePackageCatalogers/dpkgdb-cataloger-2                 5.53k ± 0%     5.53k ± 0%     ~     (all equal)
ImagePackageCatalogers/rpmdb-cataloger-2                  6.22k ± 0%     6.22k ± 0%     ~     (all equal)
ImagePackageCatalogers/java-cataloger-2                   51.5k ± 0%     51.5k ± 0%     ~     (p=0.413 n=5+5)
ImagePackageCatalogers/apkdb-cataloger-2                  5.49k ± 0%     5.49k ± 0%     ~     (p=0.881 n=5+5)
ImagePackageCatalogers/go-module-binary-cataloger-2        9.00 ± 0%      9.00 ± 0%     ~     (all equal)

@wagoodman wagoodman requested a review from a team October 22, 2021 20:16
@wagoodman wagoodman self-assigned this Oct 22, 2021
@wagoodman wagoodman marked this pull request as ready for review October 22, 2021 20:16
Copy link
Contributor

@luhring luhring left a comment

Choose a reason for hiding this comment

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

Looks good! Treat all comments as "nit"!

(Another general "nit" for consideration would be to consider more use of vertical space to visually separate if/switch blocks, variable decls, and return statements.)

internal/formats/common/spdxhelpers/description.go Outdated Show resolved Hide resolved
@wagoodman wagoodman force-pushed the add-spdxjson-format-lossy branch from 6ad50c2 to 2433d60 Compare October 29, 2021 14:33
Signed-off-by: Alex Goodman <[email protected]>
Signed-off-by: Alex Goodman <[email protected]>
@wagoodman wagoodman enabled auto-merge (squash) October 29, 2021 14:53
@wagoodman wagoodman merged commit 9aca23f into main Oct 29, 2021
@wagoodman wagoodman deleted the add-spdxjson-format-lossy branch October 29, 2021 14:55
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
* remove existing spdxjson presenter + helpers

Signed-off-by: Alex Goodman <[email protected]>

* add new spdx22json format

Signed-off-by: Alex Goodman <[email protected]>

* add common sdpxhelpers (migrated)

Signed-off-by: Alex Goodman <[email protected]>

* use new common spdx helpers

Signed-off-by: Alex Goodman <[email protected]>

* wire up new spdx22json format object

Signed-off-by: Alex Goodman <[email protected]>

* remove lossless syft-specific property bags

Signed-off-by: Alex Goodman <[email protected]>

* remove spdxjson decoder and validator

Signed-off-by: Alex Goodman <[email protected]>

* add nil checks in spdx test helpers

Signed-off-by: Alex Goodman <[email protected]>

* remove empty default case

Signed-off-by: Alex Goodman <[email protected]>

* use explicit golden snapshot

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

Successfully merging this pull request may close these issues.

2 participants