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

add means to run dash-licenses #454

Closed

Conversation

marcdumais-work
Copy link
Contributor

@marcdumais-work marcdumais-work commented Aug 23, 2021

Discussing with @MatthewKhouzam about 3PP license checks, I mentioned an Eclipse Foundation tool, dash-licenses, that I am integrating in Theia's CI. Here's a quick integration for theia-trace-extension. Since only Ericsson employees have contributed to this, and both Eclipse Theia and this project here are done for Ericsson, I think I can change the license of the license check script to match this repo's license (and have done so).

The PR adds:

  • a GH workflow that runs the license check
  • a "baseline file" (license-check-baseline.json), that acts as a filter, for the check to temporarily ignore some dependencies that are already known to fail the license check
    • this permits running the check during a PR's CI, to fail only if any new dependency is found that does not pass the license check.
    • the dependencies in that file are assumed to be worked-on in the background (e.g. being investigated)
    • in some cases, dash-license can report false positives - they can also be added to that file.
    • each entry in the file has two fields: the dependency and an optional string, that can be used to document why an entry is present. e.g. "the license for X says $BADLICENSE but we believe that's wrong so let's ignore it"
  • a license-check script, check_3pp_licenses.js and a script entry in package.json to run it

To run locally, do:
yarn license:check

To get the full results, first temporarily remove file license-check-baseline.json:

$ mv license-check-baseline.json license-check-baseline.json.bak
$ yarn license:check
$ mv license-check-baseline.json.bak license-check-baseline.json

A summary, with all dependencies listed, is created: license-check-summary.txt

@bhufmann
Copy link
Collaborator

@marcdumais-work Thanks for this contribution. Yes, changing the license to MIT as you suggested would help.

The license check execution failed because some licenses could not be verified (I think). What would be the steps to fix this? Please let me know.

@marcdumais-work
Copy link
Contributor Author

@marcdumais-work Thanks for this contribution. Yes, changing the license to MIT as you suggested would help.

+1, will do. Can you propose a license header for me to use? Looking at a few source files in this repo I could not find an example.

The license check execution failed because some licenses could not be verified (I think).

Correct. Or could not be verified with enough certainty (clearlydefined score) or the license(s) are not approved by the Foundation (exceptions can sometimes be sought - e.g. we obtained permission for Theia to use jschardet (LGPL).

What would be the steps to fix this? Please let me know.

We're still working on the Theia repo version of this PR. The idea we have it to:
(1) do a baseline run of dash-licenses and handle the backlog of 3PPs to validate, license-wise, separately (e.g. open a CQ for them). Once approved, these 3PP should no longer be reported by dash-licenses.
(2) add an exception list mechanism that contains those baseline dependencies, so they are ignored when we run the CI job.

  • If no new dependency is found to be suspicious, the CI check will be considered passed (green)
  • if one or more new dependency is found, the CI check will fail. The project team will have to open a CQ for the new dependencies that failed. Once we get the permission to check-in the code (parallel IP) we add the dependencies to the exclude list, which will make the CI pass again.

You can follow the progress of the upstream PR, linked in the description above.

@marcdumais-work
Copy link
Contributor Author

FYI, the upstream PR has changed quite a bit since I opened this draft PR, with Paul helping improving it. I can uplift it, if you are interested to eventually merge.

Signed-off-by: Marc Dumais <[email protected]>
The upstream version is under the upstream project's
license. Ericsson is the only contributor to that file,
so we are able to license it otherwise here, as needed.

Signed-off-by: Marc Dumais <[email protected]>
This file acts as a filter, that contains dependencies that
fail the liocense check but are "known". This permits to ignore
these, and be warned when any new dependency that fails the check
is added, e.g. in a PR.

For now I have kept upstream dependencies that are relevant here.

Signed-off-by: Marc Dumais <[email protected]>
@marcdumais-work
Copy link
Contributor Author

I have updated the PR and description, to match what we merged upstream. I kept multiple commits so the changes I made are clearer - I can squash before an eventual merge.

@marcdumais-work
Copy link
Contributor Author

FYI, Paul had the idea to contribute an npm packaging, directly in the dash-licenses repo, which would make it even easier to integrate to various repos. No ETA, so in the meantime this PR is the best way forward that we have.
eclipse-dash/dash-licenses#96

@marcdumais-work
Copy link
Contributor Author

This PR has become obsolete. We are working to create an easily integrated wrapper for dash-licenses, that will be published as a npm package. Once that's done, adding license check here in this repo and others too, will become a lot easier and more maintainable.

@marcdumais-work marcdumais-work deleted the dash-licenses branch October 31, 2023 13:32
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