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

Use SPDX license identifier #1029

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cdce8p
Copy link
Contributor

@cdce8p cdce8p commented Aug 3, 2024

Use the SPDX license identifier for Apache-2.0 instead of the whole license file inside the project metadata. This will make it easier to correctly identify the project license. See also PEP-639 which is close to being finalized.

https://spdx.org/licenses/

Apache License
Version 2.0, January 2004
http://www.apache.org/licenses/

@MartinHjelmare
Copy link
Contributor

Use the SPDX license identifier for GPL-2.0-or-later

We use Apache 2.0.

@cdce8p
Copy link
Contributor Author

cdce8p commented Aug 3, 2024

Use the SPDX license identifier for GPL-2.0-or-later

We use Apache 2.0.

Sorry, copy-paste error in the PR description. The file change is correct though.

@MartinHjelmare
Copy link
Contributor

MartinHjelmare commented Aug 3, 2024

How compatible is this with our current license validation in Home Assistant core? I know there was some work done already to replace the text variable with classifiers in 3rd party libraries.

@cdce8p
Copy link
Contributor Author

cdce8p commented Aug 3, 2024

How compatible is this with our current license validation in Home Assistant core?

It's fully compatible. The current implementation just looks for "Apache License" and similar (which is obviously part of the full license text) but it also checks the SPDX identifier "Apache-2.0".

I found this as part of an effort to improve the license check in Core. See home-assistant/core#123119 for details.

I know there was some work done already to replace the text variable with classifiers in 3rd party libraries.

That was a mistake IMO. License classifier have several shortcomings compared to full license expressions so I believe the latter will be the future, especially with PEP 639 finally being finalized after such a long time.

Anyway, I don't yet plan on updating opening PRs in other projects, besides this one. I'd appreciate your feedback on home-assistant/core#123119. Maybe we can discuss it there further if there are open questions?

@MartinHjelmare
Copy link
Contributor

License SPDX identifiers sounds good. But until the PEP is accepted I don't think we should try to change libraries in one way or the other. I think we should try to be as forgiving as possible in our core license validation. If we can detect a valid open source license that's enough and we shouldn't try to nudge libraries until there's a clear path forward as stipulated by the PEP.

@cdce8p
Copy link
Contributor Author

cdce8p commented Aug 4, 2024

License SPDX identifiers sounds good. But until the PEP is accepted I don't think we should try to change libraries in one way or the other.

The current status https://discuss.python.org/t/pep-639-round-3-improving-license-clarity-with-better-package-metadata/53020/80 from the PEP-Delegate. They are only discussing one last implementation detail around the supported glob syntax for license-files. Everything else looks good.

we shouldn't try to nudge libraries until there's a clear path forward as stipulated by the PEP.

I'd slightly disagree here. For example, as mentioned in home-assistant/core#123119, poetry will auto-add the Other/Proprietary License license classifier if no valid SPDX license is detected. Just from looking at the classifiers alone it isn't clear what the intent here would be ['Apache Software License', 'Other/Proprietary License'].

As mentioned, I won't go around opening PRs for it. However, I still think this particular one is still fine to merge as is 😄

@cdce8p
Copy link
Contributor Author

cdce8p commented Sep 24, 2024

PEP 639 has been provisionally accepted. It'll still take some time before the build backends fully support it though. However, changing the license text to the SPDX identifier should be fine now. Saw you also merged a similar PR in another project: MartinHjelmare/aiovlc#268

@MartinHjelmare
Copy link
Contributor

That project is using poetry which documents that the SPDX identifiers should be used for the license field. I think we should wait until the PEP is fully accepted until we move forward here.

@cdce8p
Copy link
Contributor Author

cdce8p commented Sep 24, 2024

That project is using poetry which documents that the SPDX identifiers should be used for the license field.

BTW: Both (poetry and setuptools) output to the same project metadata field: License.

@MartinHjelmare
Copy link
Contributor

The setuptools documentation doesn't clarify what way to identify the license is preferred and the Python Packaging User Guide prefers the classifiers. Until this is spelled out better (accepting the PEP will do that), we should wait.

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.

2 participants