tests: Limit Python linting to files in the repo #16124

1. 
   practicalswift commented at 6:42 AM on May 30, 2019: contributor

   Limit Python linting to files in the repo.

   Before:

   $ test/lint/lint-python.sh
   not_under_version_control.py:195:9: F841 local variable 'e' is assigned to but never used
   $
   

   After:

   $ test/lint/lint-python.sh
   $
   

2. DrahtBot added the label Tests on May 30, 2019
3. LarryRuane approved
4. 
   LarryRuane commented at 5:16 PM on May 30, 2019: contributor

   tACK https://github.com/bitcoin/bitcoin/pull/16124/commits/7c1fa584f1a30b621525bf33a1434a7fb81a8830

5. in test/lint/lint-python.sh:97 in 7c1fa584f1 outdated

     89 | @@ -90,4 +90,8 @@ elif PYTHONWARNINGS="ignore" flake8 --version | grep -q "Python 2"; then
     90 |      exit 0
     91 |  fi
     92 |  
     93 | -PYTHONWARNINGS="ignore" flake8 --ignore=B,C,E,F,I,N,W --select=$(IFS=","; echo "${enabled[*]}") "${@:-.}"
     94 | +if [[ $# == 0 ]]; then
     95 | +    git ls-files "*.py"
     96 | +else
     97 | +    echo "$@"
     98 | +fi | PYTHONWARNINGS="ignore" xargs flake8 --ignore=B,C,E,F,I,N,W --select=$(IFS=","; echo "${enabled[*]}")
   

   ---

   ---

   

   MarcoFalke commented at 5:36 PM on May 30, 2019:

   Couldn't you do this without xargs?

   I.e.

   FILES=$(
   if [[$# == 0 ...
   ...
   fi
   )
   flake8 ... $FILES
   

   ---

   

   LarryRuane commented at 7:09 PM on May 30, 2019:

   I'm not sure if what I'm about to say is relevant here in a practical sense, but what I've always liked about xargs is that it avoids a limit you can reach with a very long command line argument list. This creates a very long command line, but is okay (on my Ubuntu system running bash):

   /bin/echo $(seq 100000) | wc
         1  100000  588895
   $ 
   

   But now let's go to the next order of magnitude (one million arguments):

   $ /bin/echo $(seq 1000000) | wc
   bash: /bin/echo: Argument list too long
         0       0       0
   

   Doing this with xargs works:

   $ seq 1000000 | xargs echo | wc
        53 1000000 6888896    
   $ 
   

   Notice that xargs generated 53 individual echo commands (the number of lines as reported by wc). The xargs program is aware of the various shells' command line limits, or at least it's conservative enough that it works with any shell.

   So even though it won't matter here (we're unlikely to have such a huge number of python files), I like to use xargs on principle whenever the command line can be arbitrarily long. So I'd vote for keeping this PR as is.

   ---

   

   LarryRuane commented at 7:22 PM on May 30, 2019:

   Just a fun side comment: While writing the previous comment, I discovered that the bash built-in echo command has a much higher command-line length limit; even 10 million worked:

   echo $(seq 10000000) | wc
         1 10000000 78888897
   $ 
   

   Then I tried 100m, and the bash's memory usage reached 10gb, and minutes of CPU time, and control-C wouldn't kill the command! (It did finally die.) So I recommend against trying that.

   Anyway, so this is why the previous comment says /bin/echo instead of echo. Of course, flake8 is like /bin/echo, an external executable.

   ---

   

   practicalswift commented at 8:38 PM on May 30, 2019:

   @MarcoFalke Sure! The update version does not use xargs. Please re-review :-)

6. Limit Python linting to files in the repo 3c5254a820
7. practicalswift force-pushed on May 30, 2019
8. 
   fanquake commented at 9:51 PM on May 30, 2019: member

   tACK https://github.com/bitcoin/bitcoin/pull/16124/commits/3c5254a820c892b448dfb42991f6109a032a3730

   This also means that we're no longer including any .py files from the depends/ dir, which was generating hundreds of warnings, i.e: https://gist.github.com/fanquake/443b5fecaf59ed228310110157b49912.

9. MarcoFalke added this to the milestone 0.19.0 on May 30, 2019
10. 
    Empact commented at 1:56 AM on May 31, 2019: member

    utACK https://github.com/bitcoin/bitcoin/commit/3c5254a820c892b448dfb42991f6109a032a3730

    I also tested ${@:-$(git ls-files "*.py")} as an alternative and it works locally.

11. 
    laanwj commented at 9:22 PM on June 3, 2019: member

    utACK 3c5254a820c892b448dfb42991f6109a032a3730

12. laanwj merged this on Jun 3, 2019
13. laanwj closed this on Jun 3, 2019

    ---

14. laanwj referenced this in commit d3a1c2502b on Jun 3, 2019
15. practicalswift deleted the branch on Apr 10, 2021
16. Munkybooty referenced this in commit 44c4449c7b on Oct 17, 2021
17. Munkybooty referenced this in commit ac0c489340 on Oct 25, 2021
18. Munkybooty referenced this in commit e225aeaec4 on Oct 26, 2021
19. Munkybooty referenced this in commit c67aac1ac4 on Oct 26, 2021
20. Munkybooty referenced this in commit a92ddf2784 on Nov 9, 2021
21. pravblockc referenced this in commit 99333ad279 on Nov 18, 2021
22. DrahtBot locked this on Aug 16, 2022

Contributors

practicalswiftLarryRuaneMarcoFalkefanquakeEmpactlaanwj

Milestone
0.19.0