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.

Sign up for GitHub

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

Jump to bottom

license: permit no license for non-core taps, add exempted license groups #8211

Merged
merged 3 commits into from Aug 5, 2020
Merged

license: permit no license for non-core taps, add exempted license groups #8211

merged 3 commits into from Aug 5, 2020

Conversation

jonchang
Copy link
Contributor

@jonchang jonchang commented Aug 4, 2020

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

With this pull request, it's OK for non-core taps to either not specify a license or to say that the license is NOASSERTION.

Furthermore, the GitHub license detection system is not perfect. Create groups of permitted licenses where the license will be considered matching if both the GitHub license and the formula license are members of the same groups. Currently this just applies to GPLv2 and GPLv3. cc @Rylan12 and @chenrui333 for feedback on if they've seen other bad license detection.

@jonchang jonchang changed the title license: permit NOASSERTION for non-core taps, exempted license groups license: permit no license for non-core taps, exempted license groups Aug 4, 2020
@jonchang jonchang changed the title license: permit no license for non-core taps, exempted license groups license: permit no license for non-core taps, add exempted license groups Aug 4, 2020
MikeMcQuaid
MikeMcQuaid reviewed Aug 4, 2020
View reviewed changes
Library/Homebrew/dev-cmd/audit.rb Outdated Show resolved Hide resolved
Library/Homebrew/dev-cmd/audit.rb Outdated Show resolved Hide resolved
Library/Homebrew/dev-cmd/audit.rb Show resolved Hide resolved
@Rylan12
Copy link
Member

Rylan12 commented Aug 4, 2020

As far as I know, the GitHub API only returns the identifiers listed here.

From those, it looks like the only issues we'll have are with:

  • AGPL-3.0 --> AGPL-3.0-only or AGPL-3.0-or-later
  • GPL-2.0 --> GPL-2.0-only or GPL-2.0-or-later
  • GPL-3.0 --> GPL-3.0-only or GPL-3.0-or-later
  • LGPL-2.1 --> LGPL-2.1-only or LGPL-2.1-or-later
  • LGPL-3.0 --> LGPL-3.0-only or LGPL-3.0-or-later

@MikeMcQuaid
Copy link
Member

From those, it looks like the only issues we'll have are with:

  • AGPL-3.0 --> AGPL-3.0-only or AGPL-3.0-or-later
  • GPL-2.0 --> GPL-2.0-only or GPL-2.0-or-later
  • GPL-3.0 --> GPL-3.0-only or GPL-3.0-or-later
  • LGPL-2.1 --> LGPL-2.1-only or LGPL-2.1-or-later
  • LGPL-3.0 --> LGPL-3.0-only or LGPL-3.0-or-later

Yeh, that's what I was thinking. If the "key" there is specified: require/allow one of the "values".

@jonchang
Copy link
Contributor Author

jonchang commented Aug 5, 2020

Ok, I've tweaked the license matching code against GitHub. This should be ready assuming full tests pass.

@jonchang jonchang mentioned this pull request Aug 5, 2020
Closed
5 tasks
@Rylan12 Rylan12 mentioned this pull request Aug 5, 2020
Merged
6 tasks
MikeMcQuaid
MikeMcQuaid approved these changes Aug 5, 2020
View reviewed changes
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

A few nits but good to merge after.

Library/Homebrew/.rubocop.yml Outdated Show resolved Hide resolved
Library/Homebrew/test/dev-cmd/audit_spec.rb Outdated Show resolved Hide resolved
Library/Homebrew/test/dev-cmd/audit_spec.rb Outdated Show resolved Hide resolved
@jonchang @MikeMcQuaid
license: style fixes
fa80b48 
Co-authored-by: Mike McQuaid <mike@mikemcquaid.com>
@jonchang jonchang merged commit 0e8962f into Homebrew:master Aug 5, 2020
@jonchang
Copy link
Contributor Author

jonchang commented Aug 5, 2020

Thanks for the reviews!

@jonchang jonchang deleted the license-groups branch August 6, 2020 03:38
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Dec 20, 2020
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Dec 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Rylan12 Rylan12 Rylan12 left review comments

@MikeMcQuaid MikeMcQuaid MikeMcQuaid approved these changes

Assignees
No one assigned
Labels
outdated PR was locked due to age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@jonchang @Rylan12 @MikeMcQuaid @BrewTestBot