Repository navigation
Fix up a few filesAnalyzed issues - #195
Conversation
|
Looks like circleci stuff on mac is broken. I'm not quite sure how to fix it. |
pombredanne
left a comment
pombredanne
left a comment
There was a problem hiding this comment.
Excellent catch!
Thank you ++
Could I interest you in adding a unit test too to avoid regression?
|
And sorry if I did update your branch by mistake with the latest. Let's pretend it was because of fat fingers. |
|
Yes, of course. I'll add some tests. |
|
@lhh Thank you! you rock. |
|
I am chasing possibly a bug in the RDF writer not handling multiple packages properly (particularly when one is filesAnalyzed = false), which is why that and the tag/value writer tests are not present yet. My apologies for the delay. I'm out for a few days. |
|
Should |
@dholth this code is a source of troubles more often than not. We should make it optional in this library IMHO. |
|
@lhh, do you still plan on updating this PR? |
|
Ah, yes - my apologies. I'll address the above issues shortly. |
d4a1415 to
03788d4
Compare
|
@pombredanne you still have a blocker on this PR. As I don't want to dismiss your review, would you care to have a short look at it again? |
nicoweidner
left a comment
nicoweidner
left a comment
There was a problem hiding this comment.
I left two minor questions. Thanks for the contribution!
When filesAnalyzed is False, several sections should be omitted when writing rather than being presented as empty sets or "None". SPDX 2.2, sec: 3.8.3 - files analyzed specification 3.9.3 - verification code 3.14.3 - licenses from files Without doing this, invalid SPDX documents are written and cannot be parse by the parser. Signed-off-by: Lon Hohberger <lhh@redhat.com>
xmltodict is used to generate a Python dictionary from XML tags. Unfortunately, it does not transmogrify CDATA "true" or "false" to their Python bool representation, so we must do it ourselves. Add this to the XML example for testing purposes. Signed-off-by: Lon Hohberger <lhh@redhat.com>
Reading in SPDX BOMs with multiple packages produces a list of dictionaries, which cannot be sorted (throws a TypeError) Signed-off-by: Lon Hohberger <lhh@redhat.com>
When multiple packages are present, some may have filesAnalyzed set to True, others set to False. Make sure we test both cases. Signed-off-by: Lon Hohberger <lhh@redhat.com>
The SPDX specification holds that the package verification code is mandatory whenever files_analyzed is True or none (omitted), however, for the purposes of this library, we make it optional here. Signed-off-by: Lon Hohberger <lhh@redhat.com>
nicoweidner
left a comment
nicoweidner
left a comment
There was a problem hiding this comment.
Thanks for addressing my questions! I didn't look at the remaining code in detail, but two other reviewes already approved, so this should be good to merge
No description provided.