[libcamera-devel] Problem with utils/checkstyle.py in a pre-commit hook?

[Naushir Patuck]

Hi Umang,


On Wed, 20 Jan 2021 at 16:52, Umang Jain <email at uajain.com> wrote:

> Hi Naushir
>
> Yes, I have hit this issue with pre-commit and your diagonsis is right.
>

Thanks for confirming!  I'm glad it's not only me :-) I'm positive this has
only recently started happening.


>
> On 1/20/21 4:29 PM, Naushir Patuck wrote:
>
>
>
> On Wed, 20 Jan 2021 at 10:52, Naushir Patuck <naush at raspberrypi.com>
> wrote:
>
>> Hi all,
>>
>> I have started noticing a problem with the utils/checkstyle.py script
>> when run in the provided pre-commit hook (utils/hooks/pre-commit).  Here is
>> an example output when I do a "git commit --amend --fixup=xxxx" command:
>>
>
> Sorry, my git command has a typo above, it is  "git commit -a --fixup=xxxx"
>
>
>>
>> fatal: ambiguous argument '': unknown revision or path not in the working
>> tree.
>> Use '--' to separate paths from revisions, like this:
>> 'git <command> [<revision>...] -- [<file>...]'
>> Traceback (most recent call last):
>>   File "./utils/checkstyle.py", line 875, in <module>
>>     sys.exit(main(sys.argv))
>>   File "./utils/checkstyle.py", line 850, in main
>>     commits.append(StagedChanges())
>>   File "./utils/checkstyle.py", line 237, in __init__
>>     Commit.__init__(self, '')
>>   File "./utils/checkstyle.py", line 206, in __init__
>>     self.__parse()
>>   File "./utils/checkstyle.py", line 215, in __parse
>>     self.__title = files[0]
>> IndexError: list index out of range
>>
>> This used to certainly work in the past, so I can only assume either
>> something in the script, or my environment has changed.  The reason behind
>> the git command failure seems to be related to the last argument in the
>> subprocess call:
>>
>> ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-status',
>>                                   self.commit],
>>
>> stdout=subprocess.PIPE).stdout.decode('utf-8')
>>
>> where self.commit is an empty string.
>>
>> This may have been introduced by commit 097720840 ('utils: checkstyle.py:
>> Move commit handling to a separate section').  A fix that works for me is
>> as follows:
>>
>> diff --git a/utils/checkstyle.py b/utils/checkstyle.py
>> index 0e9659e98518..cb7b2d151412 100755
>> --- a/utils/checkstyle.py
>> +++ b/utils/checkstyle.py
>> @@ -207,9 +207,11 @@ class Commit:
>>
>>      def __parse(self):
>>          # Get the commit title and list of files.
>> -        ret = subprocess.run(['git', 'show', '--pretty=oneline',
>> '--name-status',
>> -                              self.commit],
>> -
>> stdout=subprocess.PIPE).stdout.decode('utf-8')
>> +        args = ['git', 'show', '--pretty=oneline', '--name-status']
>> +        if self.commit != '':
>> +            args.append(self.commit)
>> +
>> +        ret = subprocess.run(args,
>> stdout=subprocess.PIPE).stdout.decode('utf-8')
>>          files = ret.splitlines()
>>          self.__files = [CommitFile(f) for f in files[1:]]
>>
>> I think this will parse the HEAD commit and not the diff/staged changes.
> I haven't looked at it closely but that's my understanding. I have seen
> mostly using the hook as a post-commit by other devs and that (in my
> testing) works fine.
>

Perhaps - my fix above worked on the (seemingly correct) assumption that an
empty string appended to the end of the argument list would break the
command.  If there is a commit hash in the self.commit string, it will
append it as before.
If the checkpatch script is not meant to be used as a pre-commit hook,
perhaps we should remove the utils/hooks/pre-commit script entirely so
others don't encounter this problem?

Regards,
Naush



> I was just wondering if I have something strange in my environment that is
>> causing these issues, or are other folks seeing this as well?
>>
>> Regards,
>> Naush
>>
>>
> _______________________________________________
> libcamera-devel mailing listlibcamera-devel at lists.libcamera.orghttps://lists.libcamera.org/listinfo/libcamera-devel
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210121/62bc94dd/attachment.htm>