core icon indicating copy to clipboard operation
core copied to clipboard
Published 2 months ago • verified publisher icon home-assistant

Rework audit-licenses check [ci]

Open cdce8p opened this issue 1 year ago • 3 comments

Proposed change

Followup to https://github.com/home-assistant/core/pull/120683#pullrequestreview-2151030803

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)
  • [x] 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 avatar Aug 03 '24 20:08 cdce8p

ftfy is the first libray to use this one right?

joostlek avatar Oct 11 '24 17:10 joostlek

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.

  • https://github.com/pypa/packaging/pull/828/

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

cdce8p avatar Oct 11 '24 17:10 cdce8p

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.

  • https://github.com/raimon49/pip-licenses/pull/213/

cdce8p avatar Oct 11 '24 21:10 cdce8p