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

Rework audit-licenses check [ci] #123119

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from
Draft

Conversation

cdce8p
Copy link
Member

@cdce8p cdce8p commented Aug 3, 2024

Proposed change

Followup to #120683 (review)

This PR reworks the audit-licenses check with several improvements.

  • The check is run for all tested python-versions. I.e. all for which we build environments for testing. Requirements might slightly differ based on the Python version and the check is fast enough to run for all.
  • Add an argument parser and a logger to allow custom file paths and debug logging.
  • Add --from=all argument for pip-licenses. This will write both, the metadata license string and the license classifier, to the json file.
    https://github.com/raimon49/pip-licenses/tree/v-4.4.0?tab=readme-ov-file#option-from
  • Prefer the metadata license string over the classifier when checking for OSI approved licenses.
  • Add the license-expression package to validate the metadata license string and require a valid SPDX license expression.
    https://github.com/nexB/license-expression
  • Add support for AND and OR license expressions. E.g. Apache-2.0 OR BSD-3-Clause or MPL-2.0 AND MIT.
  • Fall back and improve support for license classifier. If multiple are specified, they are outputted as one string with ; . If multiple classifier are given, they are now interpreted as AND (instead of OR). That is the safer option considering cases like: ['Apache Software License', 'Other/Proprietary License'].

Consequences

  • Invalid metadata license strings and full license texts are no longer excepted. These packages were added to the TODO list.

Note

Call to action
As it looks ATM, PEP-639 is close to being finalized. With that SPDX license expression will be standardized for the license metadata. We should recommend / nudge developers to start using them today - fix the license metadata of their project if it's on the TODO list. Especially poetry users will benefit here, since poetry automatically adds the Other/Proprietary License classifier if it can't detect a valid SPDX license identifier.

Future work
At some point it might make sense to only consider the license metadata going forward and deprecate / remove the license classifier check.

--
https://spdx.org/licenses/

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Deprecation (breaking change to happen in the future)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -m script.gen_requirements_all.
  • For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

@cdce8p cdce8p requested a review from a team as a code owner August 3, 2024 20:28
@home-assistant home-assistant bot added cla-signed small-pr PRs with less than 30 lines. labels Aug 3, 2024
@cdce8p cdce8p requested a review from joostlek August 3, 2024 20:28
@@ -189,61 +205,250 @@ def from_dict(cls, data: dict[str, str]) -> PackageDefinition:
"uvcclient": AwesomeVersion(
"0.11.0"
), # No License https://github.com/kk7ds/uvcclient/issues/7
# -- Full license text in metadata
Copy link
Member

Choose a reason for hiding this comment

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

Should these be in TODO or in exceptions?

Copy link
Member Author

Choose a reason for hiding this comment

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

All of them are OSI licenses, so I wouldn't consider them to be "exceptions" per se. More things we should take care of which would be "todo".

I can change it though if you think it makes more sense there.

@joostlek
Copy link
Member

ftfy is the first libray to use this one right?

@cdce8p
Copy link
Member Author

cdce8p commented Oct 11, 2024

ftfy is the first libray to use this one right?

The first to use Library-Expression although I'd expect more now that `hatchling uses it by default.
The PR doesn't actually deal with that though. I had considered the preparation for the eventual support.

Let's wait for "official" support in packaging before moving forward here. The PR is almost done.

We'll also need License-Expression parsing in pip-licenses. Will hopefully be able to look at it this weekend.

@cdce8p cdce8p marked this pull request as draft October 11, 2024 17:42
@cdce8p
Copy link
Member Author

cdce8p commented Oct 11, 2024

We'll also need License-Expression parsing in pip-licenses. Will hopefully be able to look at it this weekend.

Opened a PR for it upstream.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed code-quality small-pr PRs with less than 30 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants