Skip to content

CI: Add check-dependencies workflow #120

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

Merged
merged 14 commits into from
Sep 6, 2022
Merged

CI: Add check-dependencies workflow #120

merged 14 commits into from
Sep 6, 2022

Conversation

polldo
Copy link
Contributor

@polldo polldo commented Sep 2, 2022

This PR rebases #41 .

Motivation

We need a flow to check if our dependencies are compatible with our license.

Change description

Additional Notes

Thanks @per1234 you're our hero ❤️

@polldo
Copy link
Contributor Author

polldo commented Sep 2, 2022

From #41 (comment) @per1234

Right now, it cannot be merged because many dependencies have a custom license that is not recognized by the workflow.

Hi @polldo. The license file detection system used by the Licensed tool is very strict, which is actually a good thing because even a small difference in wording or even punctuation might completely change the legal meaning of a license, and the tools are not smart enough to reliably interpret whether such a change is significant. So it will be inevitable that human review will be needed for some dependencies. This can take some time to do when the system is first set up because of the large number of existing dependencies that must be analyzed, but I think it won't be so much ongoing work after that initial set up because it won't be too often that new dependencies are introduced, and the tool does automatically detect the licenses of many dependencies.

The human review process for dependency licenses that could not be detected:

  1. Generate dependency license metadata cache by running this command:

    task general:cache-dep-licenses
    

    the cache is also generated by the CI workflow and uploaded as a workflow asset, so if it is more convenient you can download from the workflow run summary page (currently here) and then extract the downloaded file to the .licenses subfolder of the repository.

  2. You can now search the files in the .licenses subfolder of the repository for "license: other". These are the dependencies that the tool could not automatically determine the license types for.

  3. Now manually review each of these dependencies to determine the license type. If a license text was discovered by the tool, it will be in that .dep.yml file.
    I find the licensee to be helpful for this process. You can run the following command on the dependency:

    licensee detect --diff
    

    This will show a list of the standard licenses that most closely matched the dependency license, as well as a diff between the dependency license and the standard license with the closest similarity. Fortunately, the cause of the mismatch is often just a copyright notice in the text.

  4. Once you have determined the license type of the dependency, update its metadata, using the SPDX license identifier of the dependency's license, converted to all lowercase letters.
    Example here: per1234/arduino-lint@162b6ea

  5. Now commit the .licensed folder to the repository.

A dependency might use a license type that can not be allowed globally via the allowed key of the .licensed.yml configuration file, but that is determined to be acceptable for that specific dependency.

In this case, the dependency's identifier must be added to the reviewed.<source ID>[*] key in the .licensed.yml configuration file.

@polldo polldo requested a review from per1234 September 2, 2022 14:24
@polldo polldo marked this pull request as ready for review September 2, 2022 14:24
@polldo polldo requested a review from rsora September 2, 2022 14:28
Copy link

@rsora rsora left a comment

Choose a reason for hiding this comment

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

We did it basically in pair programming, so LGTM!

PS: @per1234 🚚 ❤️

@polldo polldo force-pushed the polldo/check-dependencies branch from 96ac38b to 32f723f Compare September 5, 2022 07:08
@polldo polldo requested a review from per1234 September 5, 2022 07:08
@polldo polldo merged commit fc1b5c3 into main Sep 6, 2022
@polldo polldo deleted the polldo/check-dependencies branch September 6, 2022 07:46
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.

3 participants