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 special IDs for right-side 2.2 Relationships #63

Merged
merged 1 commit into from
May 2, 2021

Conversation

swinslow
Copy link
Member

In SPDX 2.2, the right-hand side of Relationships are not limited
to SPDX IDs; they can also include the special values NONE and
NOASSERTION.

To handle these, since Golang doesn't (to my knowledge) have a
concept of union types, and since I don't want to use interface{},
this commit instead adds a new SpecialID field to DocElementID.
When SpecialID is non-empty, it should be treated as being a
"special" ID value, and DocumentRefID / ElementRefID should be
ignored.

(Unfortunately, we can't just use ElementRefID == "NONE", etc.
for this purpose, because in theory an SPDX document could define
the identifier SPDXRef-NONE to mean something. Even though they
really, really shouldn't do that.)

This commit updates tvloader and tvsaver to appropriately handle
the possibility of NONE and NOASSERTION for this field.

Fixes #59

Signed-off-by: Steve Winslow steve@swinslow.net

In SPDX 2.2, the right-hand side of Relationships are not limited
to SPDX IDs; they can also include the special values NONE and
NOASSERTION.

To handle these, since Golang doesn't (to my knowledge) have a
concept of union types, and since I don't want to use interface{},
this commit instead adds a new SpecialID field to DocElementID.
When SpecialID is non-empty, it should be treated as being a
"special" ID value, and DocumentRefID / ElementRefID should be
ignored.

(Unfortunately, we can't just use ElementRefID == "NONE", etc.
for this purpose, because in theory an SPDX document could define
the identifier SPDXRef-NONE to mean something. Even though they
really, really shouldn't do that.)

This commit updates tvloader and tvsaver to appropriately handle
the possibility of NONE and NOASSERTION for this field.

Signed-off-by: Steve Winslow <steve@swinslow.net>
@swinslow
Copy link
Member Author

Hi @matthewkmayer, this should address the issue you raised in #59. Grateful if you can take a look and let me know your thoughts. When I try it on my local machine, it can now successfully parse the file from the demo you shared in https://github.com/matthewkmayer/spdx-go-repro.

This PR is to merge into a new v0.1.1 branch (since I tagged and released v0.1.0 earlier today). The main branch has some other more significant API changes that are likely to get merged shortly, but I wanted to get this bug fix in for the existing API since you flagged it.

@matthewkmayer
Copy link

Thanks! This change fixes #59 in my test project.

The addition of tests here make it easier for me to follow along with the changes as well. 👍

@swinslow
Copy link
Member Author

swinslow commented May 2, 2021

Ugh -- so sorry for the delay in merging this :( April was a long month.

@matthewkmayer thank you for responding so quickly on this. I'm merging it now into the 0.1.1 branch.

@swinslow swinslow merged commit 434b693 into spdx:0.1.1 May 2, 2021
@swinslow swinslow added this to the 0.2.0 milestone Jul 4, 2021
@swinslow swinslow self-assigned this Jul 4, 2021
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.

None yet

2 participants