[issue-246] parse SPDXID for file correctly #247
Conversation
Signed-off-by: Meret Behrens <[email protected]>
spdx/parsers/tagvaluebuilders.py
Outdated
| @@ -1056,6 +1056,7 @@ def set_file_spdx_id(self, doc, spdx_id): | |||
| if self.has_package(doc) and self.has_file(doc): | |||
| if not self.file_spdx_id_set: | |||
| self.file_spdx_id_set = True | |||
| spdx_id = spdx_id.split("#")[-1] | |||
There was a problem hiding this comment.
If you plan to disregard the part before #, you have to at least check that this is exactly the same as the current enclosing identifier.
There was a problem hiding this comment.
Thanks for your review. I don't think that the validation should be part of this PR. In the current implementation this isn't checked for any other ID, e.g. for the package SPDX ID. I would suggest to open another issue for this validation so that the validation is consistent for all SPDXIDs parsed from a rdf-file. Are you okay with that?
Nevertheless I see the point to transfer this rdf-specific logic to the rdf-builder. I will change this.
There was a problem hiding this comment.
Moving it to RDF processing is correct, thanks for that.
I still maintain that the namespace should not be blindly discarded.
There was a problem hiding this comment.
I opened an issue concerning the validation here #251. It think it makes sense to introduce the valdiation for all values in one PR. Are you okay with that and would you approve this PR with that?
There was a problem hiding this comment.
I'm fine with handling the namespace elsewhere, especially if it happens for all cases.
But as I wrote in #251, it's not a validation. It's a check of whether it can be discarded. In some cases (when it is different than the enclosing namespace), it will have to be retained.
There was a problem hiding this comment.
Yes, I see. I didn't know that there is a difference. But this does also apply to the package SPDX ID, right, e.g. here? I will update the open issue.
With that I think it is best to close this PR without merging and I will change the implementation in #254 so that it is independant from this problem.
There was a problem hiding this comment.
Yes, it might happen in any id you encounter, e.g., document id, snippet id, license id, ...
Signed-off-by: Meret Behrens <[email protected]>
…spec 2.2 leads to failing test, which should be solved once PR spdx#247 is merged Signed-off-by: Meret Behrens <[email protected]>
…spec 2.2 leads to failing test, which should be solved once PR spdx#247 is merged Signed-off-by: Meret Behrens <[email protected]>
fixes #246
Signed-off-by: Meret Behrens [email protected]