Search code examples
bashgitflake8

flake8 behaves differently when run from within a bash script

I suppose the answer here might be trivial, but I it might require some intricate bash knowledge. I have been browsing bash docs for a few hours now and can't seem to find the answer.

I'm working on a python repository, and came up with a simple script to lint only the files that differ between the current branch and master. Here's the minimal working example, extracted from said script (lint.sh):

#!/bin/bash    
paths=$(git diff --name-only -r origin/master...HEAD | grep \.py$)    
flake8 $paths

For testing purposes, let's say I only committed one file, bad.py, with the following contents:

hello
there

The expected output of bash lint.sh is:

bad.py:1:1: F821 undefined name 'hello'
bad.py:2:1: F821 undefined name 'there'

However, the output is empty. When run in debug mode, bash shows the following commands:

++ git diff --name-only -r origin/master...HEAD
++ grep '.py$'
+ paths='bad.py'
+ flake8 'bad.py'

Which is what I expect. Also, when I simply run flake8 bad.py, the output is as expected.

I expect this might have something to do with parameter passing which varies between different bash versions. The output of bash --version: GNU bash, version 4.4.23(1)-release (x86_64-apple-darwin17.5.0)

I will appreciate all insights

Solution

  • Very sorry this isn't exactly an answer, but it surely didn't fit in a comment!

    The hint here to me is the following:

    + paths='bad.py'
+ flake8 'bad.py'

    In my execution of the same script, I get the following:

    $ bash -x lint.sh 
++ git diff --name-only -r origin/master...HEAD
++ grep '.py$'
+ paths=bar.py
+ flake8 bar.py
bar.py:1:1: F821 undefined name 'hello'
bar.py:2:1: F821 undefined name 'world'

    Notice here how my output does not contain quotes around the filename or the assignment. bash won't usually add quotes unless they are necessary. What this tells me is there's probably some sort of control character in that string (my best guess is either colors or \b + some other characters (this might be one of the few cases where a screenshot is actually helpful!)).

    Here's one way that I was able to reproduce your findings:

    mkdir -p bin

cat > bin/grep << EOF
#!/usr/bin/env bash
exec /bin/grep --color=always "\$@"
EOF

chmod +x bin/grep

# simulate having this `grep` on your path
PATH=$PWD/bin:$PATH bash -x lint.sh

    (and while this seems like an odd thing to do, in the past I've put my own grep in ~/bin so I could add --line-buffered --color=auto now that GREP_OPTIONS is deprecated -- one might erroneously add --color=always and have it work... for the most part). Today I use an alias instead since I ran into sharp edges even with that.

    The output in that case matches yours above:

    $ PATH=$PWD/bin:$PATH bash -x lint.sh
++ git diff --name-only -r origin/master...HEAD
++ grep '.py$'
+ paths='bar.py'
+ flake8 'bar.py'

    But the tricky hint is in the highlighting

    screenshot of terminal output

    addendum

    While unrelated to your problem, here's probably a better way to accomplish what you want:

    # if you have GNU xargs
git diff -z --name-only origin/master...HEAD -- '*.py' | xargs --null --no-run-if-empty flake8

# if you need to be more portable (I see you're probably on macos)
git diff -z --name-only origin/master...HEAD -- '*.py' | xargs -0 flake8 /dev/null

    Explanation of the different parts:

    • git diff -z: output filenames with null bytes delimiting. This prevents splicing if filenames contain spaces or other special characters
    • xargs --null: split the input by null bytes when splatting arguments
    • xargs --no-run-if-empty: don't run the executable at all if there's no arguments (this is a GNU extension)
    • xargs -0: same as xargs --null, however if you're stuck with non-GNU xargs you won't have access to the long options
    • flake8 /dev/null: this is a sneaky trick, since there's no "no run if empty" option to bsd xargs, it's always going to invoke flake8. If flake8 gets invoked with zero arguments, it defaults to recursing your current working directory (and linting all your files). By putting /dev/null at the beginning this prevents this behaviour and instead lints an empty file!

    Addendum 2, you probably might want to consider using a git hooks framework to handle all of this for you, I maintain pre-commit which aims to smooth out a lot of the rough edges around git (such as this one!).