[riot-notifications] [RIOT-OS/RIOT] tools/licenses: Fix pattern files and add checks so that it does not happen again. (#11330)
Juan I Carrano
notifications at github.com
Tue Apr 2 15:23:17 CEST 2019
### Contribution description
Recently a couple of files without license slipped through in #10290 . Some investigation revealed that they were being incorrectly classified as licensed under "mit-espressif".
License templates are matched with "pcregrep" agains a pre processed version of source files. This pre processing normalizes whitespace, which includes flattening the file to a single line.
Additionally, "pcregrep" pattern files have one pattern per line.
This means that a pattern written in multiple lines will most likely _not_ do what the author thinks it does, and that was the case of "mit-espressif", which was matching all files.
I've fixed pattern files, and added a check so that it does not happen again in the future. I also included a small unrelated fix in check.sh.
### Testing procedure
Note that depending on your setup, you may have to prefix the check command with `BASE_BRANCH=whatever_the_upstream_master_is_called_on_your_machine`
#### 1: observe the issue
Checkout the first commit (the "REMOVE ME!!!" one). Run `./dist/tools/licenses/check.sh` (or `BASE_BRANCH=your-upstream-master ./dist/tools/licenses/check.sh`)
Look how the file without license `examples/hello-world/random_file_without_license.c` is accepted.
#### 2: Verify that the 1-line sanity check works.
Checkout "tools/licenses: Check that license patterns...". Run the same command. See how you get an error message.
#### 3: Verify that files without license are correctly reported, and that I did not break anything.
Checkout the a last commit. Run the check command again. `examples/hello-world/random_file_without_license.c` is accepted. In the REMOVEME commit I placed one example file for each of the licenses that I "fixed". This is to verify that I did not break the pattern.
The commit message explains how I did the pattern conversion.
### Issues/PRs references
Found in #10290.
Check out #11329 .
You can view, comment on, or merge this pull request online at:
-- Commit Summary --
* REMOVE ME!! Files with different licenses for testing.
* tools/licenses: Do not parse the output of "ls".
* tools/licenses: Check that license patterns are well-formed.
* tools/licenses: Fix pattern files (flatten into single line).
-- File Changes --
M dist/tools/licenses/check.sh (22)
M dist/tools/licenses/patterns/3c-BSD-atmel4 (22)
M dist/tools/licenses/patterns/Apache2.0 (12)
M dist/tools/licenses/patterns/gplv2-short-alt (4)
M dist/tools/licenses/patterns/mcd-st-liberty (2)
M dist/tools/licenses/patterns/mit-espressif (17)
A examples/hello-world/random_file_with_apache.c (17)
A examples/hello-world/random_file_with_atmel-bsd.c (27)
A examples/hello-world/random_file_with_espressif.c (28)
A examples/hello-world/random_file_with_lgpl_short.c (9)
A examples/hello-world/random_file_with_mcd-st.c (14)
A examples/hello-world/random_file_without_license.c (4)
-- Patch Links --
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the notifications