This also makes a start at a testsuite for checkpatch. CC: Aaron Conole <acon...@redhat.com> Signed-off-by: Ben Pfaff <b...@ovn.org> --- v1->v2: Fix case where there's no committer and add test for it.
tests/automake.mk | 1 + tests/checkpatch.at | 139 ++++++++++++++++++++++++++++++++++++++++++++++++ tests/testsuite.at | 1 + utilities/checkpatch.py | 58 ++++++++++++++++---- 4 files changed, 190 insertions(+), 9 deletions(-) create mode 100644 tests/checkpatch.at diff --git a/tests/automake.mk b/tests/automake.mk index 8224e5a4a22d..01f5077cd6ef 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -24,6 +24,7 @@ COMMON_MACROS_AT = \ TESTSUITE_AT = \ tests/testsuite.at \ tests/completion.at \ + tests/checkpatch.at \ tests/library.at \ tests/heap.at \ tests/bundle.at \ diff --git a/tests/checkpatch.at b/tests/checkpatch.at new file mode 100644 index 000000000000..81e0fc0a7e50 --- /dev/null +++ b/tests/checkpatch.at @@ -0,0 +1,139 @@ +AT_BANNER([checkpatch]) + +OVS_START_SHELL_HELPERS +# try_checkpatch PATCH [ERRORS] +# +# Runs checkpatch under Python 2 and Python 3, if installed, on the given +# PATCH, expecting the specified set of ERRORS (and warnings). +try_checkpatch() { + AT_SKIP_IF([test $HAVE_PYTHON2 = no && test $HAVE_PYTHON3 = no]) + # Take the patch to test from $1. Remove an initial four-space indent + # from it and, if it is just headers with no body, add a null body. + echo "$1" | sed 's/^ //' > test.patch + if grep '---' expout >/dev/null 2>&1; then : + else + printf '\n---\n' >> test.patch + fi + + # Take expected output from $2. + if test -n "$2"; then + echo "$2" | sed 's/^ //' > expout + else + : > expout + fi + + try_checkpatch__ "$HAVE_PYTHON2" "$PYTHON2" + try_checkpatch__ "$HAVE_PYTHON3" "$PYTHON3" +} +try_checkpatch__() { + if test $1 = no; then + : + elif test -s expout; then + AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch], + [255], [stdout]) + AT_CHECK([sed '/^Lines checked:/,$d' stdout], [0], [expout]) + else + AT_CHECK([$2 $top_srcdir/utilities/checkpatch.py -q test.patch]) + fi +} +OVS_END_SHELL_HELPERS + +AT_SETUP([checkpatch - sign-offs]) + +# Sign-off for single author who is also the committer. +try_checkpatch \ + "Author: A + Committer: A + + Signed-off-by: A" +try_checkpatch \ + "Author: A + Committer: A" \ + "ERROR: Author A needs to sign off." + +# Sign-off for single author and different committer. +try_checkpatch \ + "Author: A + Committer: B + + Signed-off-by: A + Signed-off-by: B" +try_checkpatch \ + "Author: A + Committer: B" \ + "ERROR: Author A needs to sign off. + ERROR: Committer B needs to sign off." + +# Sign-off for multiple authors with one author also the committer. +try_checkpatch \ + "Author: A + Committer: A + + Signed-off-by: A + Co-authored-by: B + Signed-off-by: B" +try_checkpatch \ + "Author: A + Committer: A + + Co-authored-by: B + Signed-off-by: B" \ + "ERROR: Author A needs to sign off." +try_checkpatch \ + "Author: A + Committer: A + + Signed-off-by: A + Co-authored-by: B" \ + "ERROR: Co-author B needs to sign off." +try_checkpatch \ + "Author: A + Committer: A + + Co-authored-by: B" \ + "ERROR: Author A needs to sign off. + ERROR: Co-author B needs to sign off." + +# Sign-off for multiple authors and separate committer. +try_checkpatch \ + "Author: A + Committer: C + + Signed-off-by: A + Co-authored-by: B + Signed-off-by: B + Signed-off-by: C" +try_checkpatch \ + "Author: A + Committer: C + + Signed-off-by: A + Co-authored-by: B + Signed-off-by: B" \ + "ERROR: Committer C needs to sign off." + +# Extra sign-offs. +try_checkpatch \ + "Author: A + Committer: C + + Signed-off-by: A + Co-authored-by: B + Signed-off-by: B + Signed-off-by: C + Signed-off-by: D + Signed-off-by: E" \ + "WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: D, E" + +# Missing committer is OK, missing author is an error. +try_checkpatch \ + "Author: A + + Signed-off-by: A" +try_checkpatch \ + "Committer: A + + Signed-off-by: A" \ + "ERROR: Patch lacks author." + +AT_CLEANUP diff --git a/tests/testsuite.at b/tests/testsuite.at index 15c385e2cddb..690904e30881 100644 --- a/tests/testsuite.at +++ b/tests/testsuite.at @@ -21,6 +21,7 @@ m4_include([tests/ovsdb-macros.at]) m4_include([tests/ofproto-macros.at]) m4_include([tests/completion.at]) +m4_include([tests/checkpatch.at]) m4_include([tests/bfd.at]) m4_include([tests/cfm.at]) m4_include([tests/lacp.at]) diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index b2f27f5b7dd1..ee4e8855988b 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -668,7 +668,7 @@ def run_file_checks(text): check['check'](text) -def ovs_checkpatch_parse(text, filename): +def ovs_checkpatch_parse(text, filename, author=None, committer=None): global print_file_name, total_line, checking_file, \ empty_return_check_state @@ -686,6 +686,8 @@ def ovs_checkpatch_parse(text, filename): hunks = re.compile('^(---|\+\+\+) (\S+)') hunk_differences = re.compile( r'^@@ ([0-9-+]+),([0-9-+]+) ([0-9-+]+),([0-9-+]+) @@') + is_author = re.compile(r'^(Author|From): (.*)$', re.I | re.M | re.S) + is_committer = re.compile(r'^(Committer: )(.*)$', re.I | re.M | re.S) is_signature = re.compile(r'^(Signed-off-by: )(.*)$', re.I | re.M | re.S) is_co_author = re.compile(r'^(Co-authored-by: )(.*)$', @@ -718,13 +720,49 @@ def ovs_checkpatch_parse(text, filename): if seppatch.match(line): parse = PARSE_STATE_DIFF_HEADER if not skip_signoff_check: - if len(signatures) == 0: - print_error("No signatures found.") - elif len(signatures) != 1 + len(co_authors): - print_error("Too many signoffs; " - "are you missing Co-authored-by lines?") - if not set(co_authors) <= set(signatures): - print_error("Co-authored-by/Signed-off-by corruption") + + # Check that the patch has an author, that the + # author is not among the co-authors, and that the + # co-authors are unique. + if not author: + print_error("Patch lacks author.") + continue + if author in co_authors: + print_error("Author should not be also be co-author.") + continue + if len(set(co_authors)) != len(co_authors): + print_error("Duplicate co-author.") + + # Check that the author, all co-authors, and the + # committer (if any) signed off. + if author not in signatures: + print_error("Author %s needs to sign off." % author) + for ca in co_authors: + if ca not in signatures: + print_error("Co-author %s needs to sign off." % ca) + break + if (committer + and author != committer + and committer not in signatures): + print_error("Committer %s needs to sign off." + % committer) + + # Check for signatures that we do not expect. + # This is only a warning because there can be, + # rarely, a signature chain. + extra_sigs = [x for x in signatures + if x not in co_authors + and x != author + and x != committer] + if extra_sigs: + print_warning("Unexpected sign-offs from developers " + "who are not authors or co-authors or " + "committers: %s" + % ", ".join(extra_sigs)) + elif is_committer.match(line): + committer = is_committer.match(line).group(2) + elif is_author.match(line): + author = is_author.match(line).group(2) elif is_signature.match(line): m = is_signature.match(line) signatures.append(m.group(2)) @@ -815,7 +853,9 @@ def ovs_checkpatch_file(filename): for part in mail.walk(): if part.get_content_maintype() == 'multipart': continue - result = ovs_checkpatch_parse(part.get_payload(decode=False), filename) + result = ovs_checkpatch_parse(part.get_payload(decode=False), filename, + mail.get('Author', mail['From']), + mail['Committer']) ovs_checkpatch_print_result(result) return result -- 2.16.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev