Test: Harden lint-filenames.sh #13496

pull ghost wants to merge 1 commits into bitcoin:master from changing 1 files +4 −1

ghost commented at 3:21 PM on June 18, 2018: none
- This fixes that only files with lower case file name suffix where found before, which contradicted the Regex to find uppercase characters in file names (including suffixes I guess).
- --full-name switch was added to git ls-files, to define that the found file always includes it's full path in the git project.
- since we know now that the file name includes the full path, we can harden the Regex to exclude the secp256k1 and univalue sub folders.
- use backslash line break to make code easier to read and avoid too long line.
MarcoFalke requested review from practicalswift on Jun 18, 2018
MarcoFalke added the label Tests on Jun 18, 2018

in test/lint/lint-filenames.sh:16 in 8fd05107ee outdated

   9 | @@ -10,7 +10,11 @@
  10 |  export LC_ALL=C
  11 |  
  12 |  EXIT_CODE=0
  13 | -OUTPUT=$(git ls-files -- "*.cpp" "*.h" "*.py" "*.sh" | grep -vE '^[a-z0-9_./-]+$' | grep -vE 'src/(secp256k1|univalue)/')
  14 | +OUTPUT=$(git ls-files --full-name -- "*.[cC][pP][pP]" "*.[hH]" "*.[pP][yY]" "*.[sS][hH]" \
  15 | +| grep -vE '^[a-z0-9_./-]+$' \
  16 | +| grep -vE '^src/(secp256k1|univalue)/' \
  17 | +)

practicalswift commented at 3:51 PM on June 18, 2018:

Nit: Please format it like this to make it consistent with the rest of the lint shell scripts:

OUTPUT=$(git ls-files --full-name -- "*.[cC][pP][pP]" "*.[hH]" "*.[pP][yY]" "*.[sS][hH]" | \
    grep -vE '^[a-z0-9_./-]+$' | \
    grep -vE '^src/(secp256k1|univalue)/')

practicalswift commented at 3:56 PM on June 18, 2018: contributor
ACK 8fd05107ee5c188c9ab59b0c1b3f5698f8075c8d modulo nits

Luckily we've never seen this kind of ugly file suffix in the wild:
```
$ git log -u | grep -E '^\+\+\+ b/.*\.(CPP|H|PY|SH)$' | wc -l
0
```
Test: Harden lint-filenames.sh 927e1150bc
ghost commented at 4:09 PM on June 18, 2018: none

Changed coding style, allthough I think to have the closing bracket on it's own line would be clearer to read and less error prone if a line should be attached, also better diff reading. But that's no discussion for a single PR, so changed according to the existing scripting style as requested. Rebased and squashed.
practicalswift commented at 4:12 PM on June 18, 2018: contributor

ACK 927e1150bc207181f7d5b0948e87d04b737a1c27
ghost commented at 4:17 PM on June 18, 2018: none
Yes, only lowercase suffixes, here the Regex that also would find mixed case (even uglier :) suffixes:
```
git log -u | grep -E '^\+\+\+ b/' | grep -iE '\.(cpp|h|py|sh)$' | grep -cvE '\.(cpp|h|py|sh)$'
0
```
ken2812221 approved
ken2812221 commented at 4:21 PM on June 18, 2018: contributor

utACK 927e115
cdecker commented at 3:22 PM on June 24, 2018: contributor

ACK 927e115
laanwj merged this on Jun 24, 2018
laanwj closed this on Jun 24, 2018
laanwj referenced this in commit 9ab4c2a246 on Jun 24, 2018
unknown deleted the branch on Jun 24, 2018
PastaPastaPasta referenced this in commit 688040d9b3 on Jul 7, 2020
PastaPastaPasta referenced this in commit d909dd878f on Jul 7, 2020
PastaPastaPasta referenced this in commit 361d254524 on Jul 8, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels