[libcamera-devel] Problem with utils/checkstyle.py in a pre-commit hook?
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Jan 21 13:46:27 CET 2021
Hi Naush,
On Thu, Jan 21, 2021 at 09:06:18AM +0000, Naushir Patuck wrote:
> 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 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:
I think the issue has been introduced in commit 4f5d17f3a4f5 ("utils:
checkstyle.py: Make title and files properties of commit class"). I have
sent a tentative fix to the mailing list and CC'ed you. Could you please
test it ?
> >> 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?
>
> >> 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,
Laurent Pinchart
More information about the libcamera-devel
mailing list