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

travis: add PR license check for missing/not-valid license files #12437

Merged
merged 6 commits into from
Feb 24, 2020

Conversation

0xc0170
Copy link
Contributor

@0xc0170 0xc0170 commented Feb 14, 2020

Summary of changes

The goal: check license offenders in pull requests

This is similar to what astyle does in Travis. We get list of files being changed. Because scancode does not support list of files being scanned but rather a file or directory, we copy files to SCANCODE folder. Execute scancode license check in this folder and check for offenders.

The rules there are: code files must have a license and SDPX identifier. If they don't, we print these files with their licenses and request review (Travis fails).

There is currently no ignore list as we have for astyle. I believe all code files in our codebase should have valid license (even if they are 3rd party).

Impact of changes

Migration actions required

Documentation


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers


@mergify mergify bot added the do not merge label Feb 14, 2020
@0xc0170 0xc0170 force-pushed the dev_travis_scancode branch 2 times, most recently from b88211f to aaa21ca Compare February 14, 2020 13:16
@mergify mergify bot added the needs: work label Feb 14, 2020
@0xc0170 0xc0170 force-pushed the dev_travis_scancode branch from aaa21ca to 02bc192 Compare February 14, 2020 13:19
@ciarmcom ciarmcom requested review from a team February 14, 2020 14:00
@ciarmcom
Copy link
Member

@0xc0170, thank you for your changes.
@ARMmbed/mbed-os-tools @ARMmbed/mbed-os-hal @ARMmbed/mbed-os-maintainers please review.

@0xc0170 0xc0170 removed the request for review from a team February 14, 2020 14:11
@0xc0170 0xc0170 removed the request for review from a team February 14, 2020 14:11
@0xc0170 0xc0170 force-pushed the dev_travis_scancode branch from df3182c to 88b27f4 Compare February 14, 2020 14:21
@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 14, 2020

scancode is functional now ! We got 2 offenders in this branch and it found them. One is without license and one with non-permissive license 🎉 More testing ongoing

@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 14, 2020

What it reports if there is no license or non permissive:

Found files with missing license details, please review and fix
File:  /hal/analogin_api.h : licenses found:  []
File:  /hal/buffer.h : licenses found:  [{'key': 'gpl-3.0', 'score': 95.74, 'name': 'GNU General Public License 3.0', 'short_name': 'GPL 3.0', 'category': 'Copyleft', 'is_exception': False, 'owner': 'Free Software Foundation (FSF)', 'homepage_url': 'http://www.gnu.org/licenses/gpl-3.0.html', 'text_url': 'http://www.gnu.org/licenses/gpl-3.0-standalone.html', 'reference_url': 'https://enterprise.dejacode.com/urn/urn:dje:license:gpl-3.0', 'spdx_license_key': 'GPL-3.0-only', 'spdx_url': 'https://spdx.org/licenses/GPL-3.0-only', 'start_line': 6, 'end_line': 10, 'matched_rule': {'identifier': 'gpl-3.0_96.RULE', 'license_expression': 'gpl-3.0', 'licenses': ['gpl-3.0'], 'is_license_text': False, 'is_license_notice': True, 'is_license_reference': False, 'is_license_tag': False, 'matcher': '3-seq', 'rule_length': 94, 'matched_length': 90, 'match_coverage': 95.74, 'rule_relevance': 100}}]

@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 14, 2020

I cleaned up the branch (removed testing files), this is ready for review.

@SeppoTakalo this one replaces gnu license check we had, we can now find out more non permissive licences in the codebase.

@0xc0170 0xc0170 changed the title WIP: travis: add scancode job travis: add PR license check - find missing/not-valid license files Feb 14, 2020
@0xc0170 0xc0170 changed the title travis: add PR license check - find missing/not-valid license files travis: add PR license check for missing/not-valid license files Feb 14, 2020
@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 14, 2020

I'll do one more test for changing file without SPDX identifier

@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 14, 2020

I add a reason for offenders, see:

Found files with missing license details, please review and fix
File:  /hal/analogin_api.h : reason:  Missing SPDX identifier

@0xc0170 0xc0170 force-pushed the dev_travis_scancode branch from 5a2a6a8 to 5cc4a08 Compare February 14, 2020 16:37
@0xc0170 0xc0170 force-pushed the dev_travis_scancode branch 2 times, most recently from 7c0c52f to dbd9b7a Compare February 21, 2020 15:38
@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 21, 2020

OK, ready for review and I can squash the long history of nothing-but-fixes. It reports now the number of offenders:

"Needs review, 2 license issues found". We will be monitoring this and reporting meanwhile the codebase gets cleaned up.

Please review again (just the diff, not each commits, will be squashed into one)

@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 21, 2020

  • I will remove 2 offenders for testing (you can review what it reports in license check now)

@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 24, 2020

@adbridge Please review, I've just fixed printing the offenders (was skipped). I can squash this once approved and 2 files for testing removed.

drivers/CAN.h Outdated
* See the License for the specific language governing permissions and
* limitations under the License.
*/

Copy link
Contributor

Choose a reason for hiding this comment

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

Where has the license header gone here ?

@@ -1,6 +1,5 @@
/*
* Copyright (c) 2018-2019, Arm Limited and affiliates.
* SPDX-License-Identifier: Apache-2.0
*
Copy link
Contributor

Choose a reason for hiding this comment

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

Why has SPDX line gone ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

testing purpose (to see what travis reportS) as stated earlier. I'l lrevert this with rebase

Master branch contains lot of missing SPDX identifiers, we will clean them up but this will take some time. In the meantime, we should not increase the license missing files. Each PR will report if there is no license issue or positive number reported as Github status. Travis won't fail if there are issues. This will highlight the issues that anyone can fix.

As soon as master is clean, we can fix set_status and revert part of this commit.
@0xc0170 0xc0170 force-pushed the dev_travis_scancode branch from 56646c3 to a0248c1 Compare February 24, 2020 12:04
@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 24, 2020

Rebased, the test files removed, history cleaned up.

@mergify mergify bot dismissed adbridge’s stale review February 24, 2020 12:04

Pull request has been modified.

@0xc0170
Copy link
Contributor Author

0xc0170 commented Feb 24, 2020

CI started

@mbed-ci
Copy link

mbed-ci commented Feb 24, 2020

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 6
Build artifacts

@0xc0170 0xc0170 merged commit bb48fa4 into ARMmbed:master Feb 24, 2020
@mergify mergify bot removed the ready for merge label Feb 24, 2020
@0xc0170 0xc0170 deleted the dev_travis_scancode branch February 24, 2020 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants