<div dir="auto"><div>Hi Laurent,<div dir="auto"><br></div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 21 Jan 2021, 12:46 pm Laurent Pinchart, <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank" rel="noreferrer">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Naush,<br>
<br>
On Thu, Jan 21, 2021 at 09:06:18AM +0000, Naushir Patuck wrote:<br>
> Hi Umang,<br>
> <br>
> On Wed, 20 Jan 2021 at 16:52, Umang Jain <<a href="mailto:email@uajain.com" rel="noreferrer noreferrer" target="_blank">email@uajain.com</a>> wrote:<br>
> <br>
> > Hi Naushir<br>
> ><br>
> > Yes, I have hit this issue with pre-commit and your diagonsis is right.<br>
> <br>
> Thanks for confirming! I'm glad it's not only me :-) I'm positive this has<br>
> only recently started happening.<br>
> <br>
> > On 1/20/21 4:29 PM, Naushir Patuck wrote:<br>
> > On Wed, 20 Jan 2021 at 10:52, Naushir Patuck wrote:<br>
> ><br>
> >> Hi all,<br>
> >><br>
> >> I have started noticing a problem with the utils/checkstyle.py script<br>
> >> when run in the provided pre-commit hook (utils/hooks/pre-commit). Here is<br>
> >> an example output when I do a "git commit --amend --fixup=xxxx" command:<br>
> ><br>
> > Sorry, my git command has a typo above, it is "git commit -a --fixup=xxxx"<br>
> ><br>
> >> fatal: ambiguous argument '': unknown revision or path not in the working<br>
> >> tree.<br>
> >> Use '--' to separate paths from revisions, like this:<br>
> >> 'git <command> [<revision>...] -- [<file>...]'<br>
> >> Traceback (most recent call last):<br>
> >> File "./utils/checkstyle.py", line 875, in <module><br>
> >> sys.exit(main(sys.argv))<br>
> >> File "./utils/checkstyle.py", line 850, in main<br>
> >> commits.append(StagedChanges())<br>
> >> File "./utils/checkstyle.py", line 237, in __init__<br>
> >> Commit.__init__(self, '')<br>
> >> File "./utils/checkstyle.py", line 206, in __init__<br>
> >> self.__parse()<br>
> >> File "./utils/checkstyle.py", line 215, in __parse<br>
> >> self.__title = files[0]<br>
> >> IndexError: list index out of range<br>
> >><br>
> >> This used to certainly work in the past, so I can only assume either<br>
> >> something in the script, or my environment has changed. The reason behind<br>
> >> the git command failure seems to be related to the last argument in the<br>
> >> subprocess call:<br>
> >><br>
> >> ret = subprocess.run(['git', 'show', '--pretty=oneline', '--name-status',<br>
> >> self.commit],<br>
> >><br>
> >> stdout=subprocess.PIPE).stdout.decode('utf-8')<br>
> >><br>
> >> where self.commit is an empty string.<br>
> >><br>
> >> This may have been introduced by commit 097720840 ('utils: checkstyle.py:<br>
> >> Move commit handling to a separate section'). A fix that works for me is<br>
> >> as follows:<br>
<br>
I think the issue has been introduced in commit 4f5d17f3a4f5 ("utils:<br>
checkstyle.py: Make title and files properties of commit class"). I have<br>
sent a tentative fix to the mailing list and CC'ed you. Could you please<br>
test it ?<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">Thanks for the fix. Will test later and report back.</div><div dir="auto"><br></div><div dir="auto">Regards,</div><div dir="auto">Naush</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> >> diff --git a/utils/checkstyle.py b/utils/checkstyle.py<br>
> >> index 0e9659e98518..cb7b2d151412 100755<br>
> >> --- a/utils/checkstyle.py<br>
> >> +++ b/utils/checkstyle.py<br>
> >> @@ -207,9 +207,11 @@ class Commit:<br>
> >><br>
> >> def __parse(self):<br>
> >> # Get the commit title and list of files.<br>
> >> - ret = subprocess.run(['git', 'show', '--pretty=oneline',<br>
> >> '--name-status',<br>
> >> - self.commit],<br>
> >> -<br>
> >> stdout=subprocess.PIPE).stdout.decode('utf-8')<br>
> >> + args = ['git', 'show', '--pretty=oneline', '--name-status']<br>
> >> + if self.commit != '':<br>
> >> + args.append(self.commit)<br>
> >> +<br>
> >> + ret = subprocess.run(args,<br>
> >> stdout=subprocess.PIPE).stdout.decode('utf-8')<br>
> >> files = ret.splitlines()<br>
> >> self.__files = [CommitFile(f) for f in files[1:]]<br>
> >><br>
> >> I think this will parse the HEAD commit and not the diff/staged changes.<br>
> ><br>
> > I haven't looked at it closely but that's my understanding. I have seen<br>
> > mostly using the hook as a post-commit by other devs and that (in my<br>
> > testing) works fine.<br>
> <br>
> Perhaps - my fix above worked on the (seemingly correct) assumption that an<br>
> empty string appended to the end of the argument list would break the<br>
> command. If there is a commit hash in the self.commit string, it will<br>
> append it as before.<br>
> If the checkpatch script is not meant to be used as a pre-commit hook,<br>
> perhaps we should remove the utils/hooks/pre-commit script entirely so<br>
> others don't encounter this problem?<br>
> <br>
> >> I was just wondering if I have something strange in my environment that is<br>
> >> causing these issues, or are other folks seeing this as well?<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div></div>