tools/licenses: Fix pattern files and add checks so that it does not happen again. #11330
+122
−56
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The file without license is to test that the bug is fixed and these files are detected. The other ones are to check that we do not break the other licenses after converting them to a single line.
The "ls" command is meant for human consumption only. It is bad practice to parse that command. That is what "find" and globs are for. For more information, see: - http://mywiki.wooledge.org/ParsingLs - https://unix.stackexchange.com/questions/128985/why-not-parse-ls-and-what-do-to-instead
This patch checks that all license patterns (or templates) have exactly one line. A non-compliant pattern was recently causing all files without a license to be (erroneously) assigned to that pattern's license. 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. All of this means that license templates must be written in a single line.
Pattern files must be written in a single line.
To fix these patterns, the following function was used:
fix_pattern () {
mv "$1" "$1.bkp" && (
cat "$1.bkp" | sed -e 's/[\/\*'"${TAB_CHAR}"']/ /g' -e 's/$/ /' | tr -d '\r\n' | sed -e 's/ */ /g;s/
\+$//g' && echo
) > "$1"
}
The code was taken from dist/tools/licenses/check.sh. This was followed by some
manual retouchuing of stops and parentheses.
jcarrano
added
Type: bug
|
I would prefer if the files in |
|
@cladmi I don't have time to do it now, and in the meanwhile the thing is still broken. Can we go with this solution and then when time allows make it pretty. Until then, developers will have to turn on line wrapping in the editor. |
|
BTW, the PR is smaller than +122 (the first commit is a throwaway and has a lot of lines). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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" against 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 with "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_machine1: observe the issue
Checkout the first commit (the "REMOVE ME!!!" one). Run
./dist/tools/licenses/check.sh(orBASE_BRANCH=your-upstream-master ./dist/tools/licenses/check.sh)Look how the file without license
examples/hello-world/random_file_without_license.cis 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.cis rejected. 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 .