| Message ID | CAEmqJPrFWxpdjqP18m3x8Hj+HAWEFOfeU4giZX-daALa9Jon0A@mail.gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
On Wed, 20 Jan 2021 at 10:52, Naushir Patuck <naush@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 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 > >
Hi Naushir Yes, I have hit this issue with pre-commit and your diagonsis is right. On 1/20/21 4:29 PM, Naushir Patuck wrote: > > > On Wed, 20 Jan 2021 at 10:52, Naushir Patuck <naush@raspberrypi.com > <mailto:naush@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. > > 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 list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Umang, On Wed, 20 Jan 2021 at 16:52, Umang Jain <email@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@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@lists.libcamera.orghttps://lists.libcamera.org/listinfo/libcamera-devel > > >
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@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?
Hi Laurent, On Thu, 21 Jan 2021, 12:46 pm Laurent Pinchart, < laurent.pinchart@ideasonboard.com> wrote: > 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@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 ? > Thanks for the fix. Will test later and report back. Regards, Naush > > > >> 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 >
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()
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: 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: self.__files = [CommitFile(f) for f in files[1:]] 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