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

Accept all single SPDX license identifiers #52

Merged
merged 5 commits into from
Oct 25, 2021

Conversation

jdblischak
Copy link
Collaborator

This PR expands the acceptable valid licenses to include all the SPDX license identifiers. I downloaded the list of identifiers using the same script used by the conda-forge linter. I also changed the error to a warning when an invalid license is detected. It's better to have users submit incomplete recipes for feedback than to fail locally.

This is meant as a start. There is plenty of room for improvement, e.g.:

  • Support compound licenses (AND, OR, WITH). Maybe the linter in conda-smithy has code for combining the single licenses that we could borrow.
  • CRAN packages don't use SPDX license identifiers. Currently only GPL-2 is automatically converted. My guess is that it will be easier for us to add more support to the helper script than to upstream SPDX support to the official skeleton (conda-build Issues about SPDX support have been open for years)

Please let me know what you think about this PR in particular as well as plans for the next steps. Thanks!

xref: #49, #51
cc: @brendanf @nick-youngblut @cbrueffer @bgruening

Current limitations:
- Doesn't accept compound licenses (AND, OR)
- Doesn't accept WITH (e.g. WITH LLVM-exception)

Scraped spdx.org on 2021-10-20 with
https://github.com/conda-forge/conda-smithy/blob/adb09bbed8136976db3caa139ebcd6fef19f2e8e/extract_licenses.bash
@brendanf
Copy link
Contributor

Thanks for taking care of this @jdblischak. I think this solves my immediate issues (#51).

For the future I think it would be useful to add replacement text for the most common licenses in CRAN, similar to what #49 did for "GPL-2" -> "GPL-2.0-or-later". However I am a little uncertain about that particular conversion; I would typically interpret CRAN's "GPL-2" to mean "GPL-2.0-only" if I was making a package, and reserve "GPL-2.0-or-later" for packages where CRAN says "GPL (>=2)".

@jdblischak
Copy link
Collaborator Author

For the future I think it would be useful to add replacement text for the most common licenses in CRAN

@brendanf Agreed. But this is a more complicated task than is typically performed by this helper script (typically simple regex's to find/replace). I'm thinking it may finally be time to create a more robust solution. I recently shared my thoughts in #7 (comment)

Copy link
Owner

@bgruening bgruening 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 @jdblischak ... just 2 optional comments.

Thanks!

run.py Outdated
'MIT AND BSD-2-Clause', 'PSF-2.0'}
with open('spdx-licenses.txt') as f:
SPDX_licenses = f.readlines()
SPDX_licenses = [x.strip() for x in SPDX_licenses]
Copy link
Owner

Choose a reason for hiding this comment

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

SPDX_licenses = [x.strip() for x in open(f)]

Should do the trick, imho.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! I simplified the code. Though I still kept the with open(... construct, as recommended from this SO thread.

run.py Outdated
msg = '"{}" license not valid. See {}'
raise ValueError(msg.format(license, SPDX_url))
msg = 'Warning: "{}" license not valid. See {}\n'
sys.stderr.write(msg.format(license, SPDX_url))
Copy link
Owner

Choose a reason for hiding this comment

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

You could use f-strings here.

sys.stderr.write( f'Warning: "{license}" license not valid. See {SPDX_url}\n' )

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! I updated the entire script to use f-strings

@jdblischak jdblischak merged commit 18a2684 into bgruening:master Oct 25, 2021
@jdblischak jdblischak deleted the spdx branch October 25, 2021 14:45
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

3 participants