[ovs-dev] [PATCHv2] checkpatch: Don't enforce char limit on tests.

[Aaron Conole]

Joe Stringer <joe at ovn.org> writes:

> Although tests ideally also stick to shorter line lengths, it is very
> common for fixed text blocks like flows or large packets to be specified
> within tests. Checkpatch shouldn't complain about cases like these.
>
> Signed-off-by: Joe Stringer <joe at ovn.org>
> ---
> v2: Broaden the set of blacklisted files to not check.
> ---
>  utilities/checkpatch.py | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
> index d9011f370816..83d14e89269e 100755
> --- a/utilities/checkpatch.py
> +++ b/utilities/checkpatch.py
> @@ -56,6 +56,12 @@ skip_trailing_whitespace_check = False
>  skip_block_whitespace_check = False
>  skip_signoff_check = False
>  
> +# Don't enforce character limit on files that include these characters in their
> +# name, as they may have legitimate reasons to have longer lines.
> +#
> +# Python isn't checked as flake8 performs these checks during build.
> +line_length_blacklist = ['.am', '.at', 'etc', '.in', '.mk', '.patch', '.py']
> +
>  
>  def is_added_line(line):
>      """Returns TRUE if the line in question is an added line.
> @@ -99,14 +105,23 @@ def ovs_checkpatch_parse(text):
>      co_authors = []
>      parse = 0
>      current_file = ''
> +    previous_file = ''
>      scissors = re.compile(r'^[\w]*---[\w]*')
>      hunks = re.compile('^(---|\+\+\+) (\S+)')
>      is_signature = re.compile(r'((\s*Signed-off-by: )(.*))$',
>                                re.I | re.M | re.S)
>      is_co_author = re.compile(r'(\s*(Co-authored-by: )(.*))$',
>                                re.I | re.M | re.S)
> +    skip_line_length_check = False
>  
>      for line in text.split('\n'):
> +        if current_file is not previous_file:
> +            previous_file = current_file
> +            for filetype in line_length_blacklist:
> +                if filetype in current_file:
> +                    skip_line_length_check = True
> +                    break

You need an else condition here; otherwise a patch which orders files

.c
[blacklisted]
.c

will skip the second .c length line checks.

Incremental patch could look something like (completely untested)
below. Thanks for doing this.

diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py
index 83d14e8..1579c1f 100755
--- a/utilities/checkpatch.py
+++ b/utilities/checkpatch.py
@@ -121,6 +121,9 @@ def ovs_checkpatch_parse(text):
                 if filetype in current_file:
                     skip_line_length_check = True
                     break
+                else:
+                    skip_line_length_check = False
 
         lineno = lineno + 1
         if len(line) <= 0:

> +
>          lineno = lineno + 1
>          if len(line) <= 0:
>              continue
> @@ -154,7 +169,7 @@ def ovs_checkpatch_parse(text):
>              if trailing_whitespace_or_crlf(line[1:]):
>                  print_line = True
>                  print_warning("Line has trailing whitespace", lineno)
> -            if len(line[1:]) > 79:
> +            if len(line[1:]) > 79 and not skip_line_length_check:
>                  print_line = True
>                  print_warning("Line is greater than 79-characters long",
>                                lineno)