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

Check that no GPL licence inside code #6848

Merged
merged 1 commit into from Jun 4, 2018

Conversation

Projects
None yet
5 participants
@OPpuolitaival
Contributor

OPpuolitaival commented May 9, 2018

Description

Check that there is not GPL licence text in pull request

Pull request type

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change
@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented May 9, 2018

You will most probably need to use case insensitive search.

@OPpuolitaival OPpuolitaival force-pushed the OPpuolitaival:licence_check branch from 178105c to 2195f8d May 9, 2018

@OPpuolitaival

This comment has been minimized.

Contributor

OPpuolitaival commented May 9, 2018

@SeppoTakalo good point! Now fixed.

@cmonr

This test case should be failing since GPL code is currently in the repo.

@cmonr

This comment has been minimized.

Contributor

cmonr commented May 9, 2018

@OPpuolitaival I like how quickly this PR came in from when the issue was discovered.

However, as this is right now, the test case will either never fail or always fail. git grep (and grep) don't change their error codes if they don't find matches. If you're looking to use either of these commands, you need to check if the command generated any output, and force and error code as appropriate.

This is a quick example of what I mean: cmonr@54e11e2
And this is the result it produces: https://travis-ci.org/cmonr/mbed-os/jobs/376894645

The second issue is that because this test will fail until the GPL code is removed, it will need to be rebased once a PR is introduced and merged in to remove the offending code. This second issue is more of an FYI.

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented May 11, 2018

@cmonr Actually, GREP return code reflects to findings.

From man page:

EXIT STATUS
     The grep utility exits with one of the following values:

     0     One or more lines were selected.
     1     No lines were selected.
     >1    An error occurred.

Git grep --help

-q
--quiet
Do not output matched lines; instead, exit with status 0 when there is a match and with non-zero status when there isn’t.

So we could use either git grep -q or grep >/dev/null and ignore the lines found.

@OPpuolitaival OPpuolitaival force-pushed the OPpuolitaival:licence_check branch from 2195f8d to 7729491 May 11, 2018

@OPpuolitaival

This comment has been minimized.

Contributor

OPpuolitaival commented May 11, 2018

I tested and return code is same without -q but now changed to use -q option

@cmonr

This comment has been minimized.

Contributor

cmonr commented May 30, 2018

Re-opening because this is still an issue.

CC: @ARMmbed/mbed-os-maintainers @ChiefBureaucraticOfficer

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented May 30, 2018

Current master should not anymore contain GPL code so this test should pass.
Please rebase and try out. If it works, merge in, so mistakes are not going to happen anymore.

@OPpuolitaival OPpuolitaival force-pushed the OPpuolitaival:licence_check branch from 7729491 to a4f8c07 May 30, 2018

@OPpuolitaival

This comment has been minimized.

Contributor

OPpuolitaival commented May 30, 2018

rebased

@cmonr

cmonr approved these changes May 30, 2018

LGTM! Good to know about the -q flag.

@cmonr

This comment has been minimized.

Contributor

cmonr commented May 31, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Jun 1, 2018

Build : SUCCESS

Build number : 2213
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6848/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Jun 4, 2018

@cmonr cmonr merged commit 0e7561f into ARMmbed:master Jun 4, 2018

14 checks passed

AWS-CI uVisor Build & Test Success
Details
ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
travis-ci/astyle Passed, 918 files
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 10449 cycles
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 9964B
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details

@cmonr cmonr removed the ready for merge label Jun 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment