-
Notifications
You must be signed in to change notification settings - Fork 697
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
Ignore case when matching SPDX identifiers #5432
Conversation
(as per the SPDX License List Matching Guidelines, v2.0) https://spdx.org/spdx-license-list/matching-guidelines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
/cc @phadej
This may actually also apply to |
Isn't this going to change the semantics of 2.2-spec files between Cabal 2.2 and Cabal 2.4? Then again, the worst that can happen is that 2.4 accepts a file that 2.2 doesn't; I guess we could add a |
@quasicomputational hmm, I guess you have a point. I'm looking into the check. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be cabal-version
feature guarded.
I.e. I cannot write cabal-version: 2.2
name: pkg
version: 0
license: bsd-3-clause That will cause missparse. At the very least it should be |
@phadej I think @quasicomputational already pointed that out, but thanks for the noise. |
In fact, I'm not sure we are reading right guidelines. The https://spdx.org/spdx-specification-21-web-version#h.jxpfx0ykyb60 spec appendix doesn't say anythign about case-sensitivity or insensitivity. Matching guidelines are about matching the license texts to the ones in SPDX license list. I'm very unsure it applies to the identifiers. I'll ask SPDX technical team for clarification |
@phadej hmm, the matching guidelines explicitly mention "SPDX License List short identifiers":
I'm not sure about license expressions in general (specifically |
Related issue for Composer: composer/composer#7039 |
I found the issue: spdx/spdx-spec#63 It's unsolved, and to me it's not clear which way it will turn out. So I'd rather keep Last comment says:
|
After I read the whole discussion at spdx/spdx-spec#63 I'm less inclined to move this forward. Assuming spdx/spdx-spec#37 gets merged in its current form, this is what we may end up with:
|
@sol Feel free to close this PR if you've changed your mind. |
FWIW, this is Python so likely not your cup of tea.... but the right way when parsing or detecting SPDX expressions is to ignore case alright (and use the canonical case when producing them). |
I'm going to close this for now. |
(as per the SPDX License List Matching Guidelines, v2.0)
https://spdx.org/spdx-license-list/matching-guidelines
Please include the following checklist in your PR:
[ci skip]
is used to avoid triggering the build bots.Please also shortly describe how you tested your change. Bonus points for added tests!