[PATCH 02/10] Run the pre-commit hook on content staged for commit

David Shea dshea at redhat.com
Wed Sep 25 19:02:15 UTC 2013


On 09/25/2013 02:42 PM, Vratislav Podzimek wrote:
> On Wed, 2013-09-25 at 14:21 -0400, David Shea wrote:
>> Changed the pylint Module output so that it's more clear what file is
>> being checked. Added an option to runpylint.sh to manually set the
>> filename to display.
>> ---
>>   scripts/githooks/pre-commit | 47 ++++++++++++++++++++++++++++++++-------------
>>   tests/pylint/runpylint.sh   | 16 ++++++++++++++-
>>   2 files changed, 49 insertions(+), 14 deletions(-)
>>
>> diff --git a/scripts/githooks/pre-commit b/scripts/githooks/pre-commit
>> index 1528793..a3307b9 100755
>> --- a/scripts/githooks/pre-commit
>> +++ b/scripts/githooks/pre-commit
>> @@ -7,6 +7,7 @@
>>   """
>>   import os
>>   import sys
>> +import tempfile
>>   from subprocess import check_output, CalledProcessError
>>   
>>   def is_python(filename):
>> @@ -14,11 +15,11 @@ def is_python(filename):
>>           return True
>>   
>>       try:
>> -        with open(filename) as testfile:
>> -            if testfile.readline().startswith("#!/usr/bin/python"):
>> -                return True
>> -    except IOError:
>> -        pass
>> +        s = check_output(["git", "show", (":%s" % filename)]).splitlines()
>> +        if s and s[0].startswith("#!/usr/bin/python"):
>> +            return True
>> +    except CalledProcessError:
>> +        sys.exit(1)
>>   
>>       return False
>>   
>> @@ -37,24 +38,44 @@ except CalledProcessError:
>>   pylint_files = []
>>   # Lines look like: :100755 100755 2cf57974e13a2aae778e28f942a4d44bf6567409 6fe1b6caf32d565b2cdb6d1aee250aaddc6d3a04 M      tests/pylint/runpylint.sh
>>   for gf in git_files.splitlines():
>> -    path = gf.split()[-1]
>> -    if is_python(path):
>> -        pylint_files.append(path)
>> +    fields = gf.split()
>> +
>> +    # If the file is being removed, ignore it
>> +    if fields[4] == 'D':
>> +        continue
>> +    elif is_python(fields[-1]):
>> +        pylint_files.append(fields[-1])
>>   
>>   if not pylint_files:
>>       sys.exit(0)
>> -pylint_files = " ".join(pylint_files)
>>   
>>   # Make sure pykickstart and blivet can be found
>>   # Note that if the checked out versions are too far off pylint may fail
>>   env = os.environ.copy()
>>   env["PYTHONPATH"] = OTHER_MODULES_PATH
>>   
>> +# Make a list of tuples of (filename, tempfile) where the tempfile
>> +# contains the file as staged for commit.
>> +pylint_commits = []
>> +for pylint_file in pylint_files:
>> +    commit = tempfile.NamedTemporaryFile()
> Those files are gonna end up under /tmp, right? Won't it cause some
> troubles with PYTHONPATH and stuff like that?
>
Erk, yeah. This'll cause trouble with modules that use relative imports. 
The use case I was aiming for was you get ready to commit a module, it 
fails pylint, you fix the problems, and then you forget to git-add 
before running the commit again. I figured it's probably good enough to 
check the staged module using imports that are on the filesystem and not 
necessarily staged. For the problem of relative imports, I'm tempted to 
un-disable W0403 ("Relative import %r...") and just make everything 
absolute, which will let PYTHONPATH work.


More information about the anaconda-patches mailing list