| Message ID | 20200316132121.23330-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | d15f979ead99fd0b2acc7cbe5bd1375d7c6df706 |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Mon, Mar 16, 2020 at 01:21:21PM +0000, Kieran Bingham wrote: > Hook the utility 'shellcheck' into our checkstyle helper to > automatically verify shell script additions. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > utils/checkstyle.py | 38 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > > diff --git a/utils/checkstyle.py b/utils/checkstyle.py > index 5ac14508bad5..0827a1e6ba0f 100755 > --- a/utils/checkstyle.py > +++ b/utils/checkstyle.py > @@ -341,6 +341,44 @@ class Pep8Checker(StyleChecker): > return issues > > > +class ShellChecker(StyleChecker): > + patterns = ('*.sh',) > + results_line_regex = re.compile('In - line ([0-9]+):') > + > + def __init__(self, content): > + super().__init__() > + self.__content = content > + > + def check(self, line_numbers): > + issues = [] > + data = ''.join(self.__content).encode('utf-8') > + > + try: > + ret = subprocess.run(['shellcheck', '-Cnever', '-'], > + input=data, stdout=subprocess.PIPE) > + except FileNotFoundError: > + issues.append(StyleIssue(0, None, "Please install shellcheck to validate shell script additions")) > + return issues > + > + results = ret.stdout.decode('utf-8').splitlines() > + for nr, item in enumerate(results): > + search = re.search(ShellChecker.results_line_regex, item) > + if search is None: > + continue > + > + line_number = int(search.group(1)) > + line = results[nr + 1] > + msg = results[nr + 2] > + > + # Determined, but not yet used > + position = msg.find('^') + 1 > + > + if line_number in line_numbers: > + issues.append(StyleIssue(line_number, line, msg)) > + > + return issues > + > + > # ------------------------------------------------------------------------------ > # Formatters > #
Hi Laurent On 16/03/2020 13:27, Laurent Pinchart wrote: > Hi Kieran, > > Thank you for the patch. > > On Mon, Mar 16, 2020 at 01:21:21PM +0000, Kieran Bingham wrote: >> Hook the utility 'shellcheck' into our checkstyle helper to >> automatically verify shell script additions. >> >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks, I meant to add here as a note: This produces the following output: libcamera/utils$ ./checkstyle.py HEAD^ ----------------------------------------------------------------- 652973f905fdb2b642e7893d7d39c000432c0e03 DNI: Bad shell bad shell ----------------------------------------------------------------- --- utils/gen-version.sh +++ utils/gen-version.sh #25: ^-- SC2039: In POSIX sh, [[ ]] is undefined. +if [[ -z "$build_dir" ]] || (echo "$build_dir" | grep -q "$src_dir") --- 1 potential style issue detected, please review The only 'undesirable' issue is that in shellcheck, the message is printed below the 'line', and thus the ' ^--' is aligned to the point of the issue, which it isn't here. That's not currently possible in our checkstyle, and I haven't even filtered out the '^--', but those can be future improvements, and are purely cosmetic really. -- Kieran > >> --- >> utils/checkstyle.py | 38 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 38 insertions(+) >> >> diff --git a/utils/checkstyle.py b/utils/checkstyle.py >> index 5ac14508bad5..0827a1e6ba0f 100755 >> --- a/utils/checkstyle.py >> +++ b/utils/checkstyle.py >> @@ -341,6 +341,44 @@ class Pep8Checker(StyleChecker): >> return issues >> >> >> +class ShellChecker(StyleChecker): >> + patterns = ('*.sh',) >> + results_line_regex = re.compile('In - line ([0-9]+):') >> + >> + def __init__(self, content): >> + super().__init__() >> + self.__content = content >> + >> + def check(self, line_numbers): >> + issues = [] >> + data = ''.join(self.__content).encode('utf-8') >> + >> + try: >> + ret = subprocess.run(['shellcheck', '-Cnever', '-'], >> + input=data, stdout=subprocess.PIPE) >> + except FileNotFoundError: >> + issues.append(StyleIssue(0, None, "Please install shellcheck to validate shell script additions")) >> + return issues >> + >> + results = ret.stdout.decode('utf-8').splitlines() >> + for nr, item in enumerate(results): >> + search = re.search(ShellChecker.results_line_regex, item) >> + if search is None: >> + continue >> + >> + line_number = int(search.group(1)) >> + line = results[nr + 1] >> + msg = results[nr + 2] >> + >> + # Determined, but not yet used >> + position = msg.find('^') + 1 >> + >> + if line_number in line_numbers: >> + issues.append(StyleIssue(line_number, line, msg)) >> + >> + return issues >> + >> + >> # ------------------------------------------------------------------------------ >> # Formatters >> # >
Hi Kieran, On Mon, Mar 16, 2020 at 01:43:51PM +0000, Kieran Bingham wrote: > On 16/03/2020 13:27, Laurent Pinchart wrote: > > On Mon, Mar 16, 2020 at 01:21:21PM +0000, Kieran Bingham wrote: > >> Hook the utility 'shellcheck' into our checkstyle helper to > >> automatically verify shell script additions. > >> > >> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Thanks, I meant to add here as a note: > > This produces the following output: > > libcamera/utils$ ./checkstyle.py HEAD^ > ----------------------------------------------------------------- > 652973f905fdb2b642e7893d7d39c000432c0e03 DNI: Bad shell bad shell > ----------------------------------------------------------------- > --- utils/gen-version.sh > +++ utils/gen-version.sh > #25: ^-- SC2039: In POSIX sh, [[ ]] is undefined. > +if [[ -z "$build_dir" ]] || (echo "$build_dir" | grep -q "$src_dir") > --- > 1 potential style issue detected, please review > > The only 'undesirable' issue is that in shellcheck, the message is > printed below the 'line', and thus the ' ^--' is aligned to the point > of the issue, which it isn't here. > > That's not currently possible in our checkstyle, and I haven't even > filtered out the '^--', but those can be future improvements, and are > purely cosmetic really. I was about to recommend dropping the ^-- marker, but it serves as a reminder that we need to fix this, and will hopefully nerd-snipe someone :-) > >> --- > >> utils/checkstyle.py | 38 ++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 38 insertions(+) > >> > >> diff --git a/utils/checkstyle.py b/utils/checkstyle.py > >> index 5ac14508bad5..0827a1e6ba0f 100755 > >> --- a/utils/checkstyle.py > >> +++ b/utils/checkstyle.py > >> @@ -341,6 +341,44 @@ class Pep8Checker(StyleChecker): > >> return issues > >> > >> > >> +class ShellChecker(StyleChecker): > >> + patterns = ('*.sh',) > >> + results_line_regex = re.compile('In - line ([0-9]+):') > >> + > >> + def __init__(self, content): > >> + super().__init__() > >> + self.__content = content > >> + > >> + def check(self, line_numbers): > >> + issues = [] > >> + data = ''.join(self.__content).encode('utf-8') > >> + > >> + try: > >> + ret = subprocess.run(['shellcheck', '-Cnever', '-'], > >> + input=data, stdout=subprocess.PIPE) > >> + except FileNotFoundError: > >> + issues.append(StyleIssue(0, None, "Please install shellcheck to validate shell script additions")) > >> + return issues > >> + > >> + results = ret.stdout.decode('utf-8').splitlines() > >> + for nr, item in enumerate(results): > >> + search = re.search(ShellChecker.results_line_regex, item) > >> + if search is None: > >> + continue > >> + > >> + line_number = int(search.group(1)) > >> + line = results[nr + 1] > >> + msg = results[nr + 2] > >> + > >> + # Determined, but not yet used > >> + position = msg.find('^') + 1 > >> + > >> + if line_number in line_numbers: > >> + issues.append(StyleIssue(line_number, line, msg)) > >> + > >> + return issues > >> + > >> + > >> # ------------------------------------------------------------------------------ > >> # Formatters > >> #
diff --git a/utils/checkstyle.py b/utils/checkstyle.py index 5ac14508bad5..0827a1e6ba0f 100755 --- a/utils/checkstyle.py +++ b/utils/checkstyle.py @@ -341,6 +341,44 @@ class Pep8Checker(StyleChecker): return issues +class ShellChecker(StyleChecker): + patterns = ('*.sh',) + results_line_regex = re.compile('In - line ([0-9]+):') + + def __init__(self, content): + super().__init__() + self.__content = content + + def check(self, line_numbers): + issues = [] + data = ''.join(self.__content).encode('utf-8') + + try: + ret = subprocess.run(['shellcheck', '-Cnever', '-'], + input=data, stdout=subprocess.PIPE) + except FileNotFoundError: + issues.append(StyleIssue(0, None, "Please install shellcheck to validate shell script additions")) + return issues + + results = ret.stdout.decode('utf-8').splitlines() + for nr, item in enumerate(results): + search = re.search(ShellChecker.results_line_regex, item) + if search is None: + continue + + line_number = int(search.group(1)) + line = results[nr + 1] + msg = results[nr + 2] + + # Determined, but not yet used + position = msg.find('^') + 1 + + if line_number in line_numbers: + issues.append(StyleIssue(line_number, line, msg)) + + return issues + + # ------------------------------------------------------------------------------ # Formatters #
Hook the utility 'shellcheck' into our checkstyle helper to automatically verify shell script additions. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- utils/checkstyle.py | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)