From patchwork Fri Aug 10 18:05:57 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben Pfaff X-Patchwork-Id: 956384 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=ovn.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41nCjc5g0mz9rxx for ; Sat, 11 Aug 2018 04:06:11 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id D4EB9D3B; Fri, 10 Aug 2018 18:06:08 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 089F3CE1 for ; Fri, 10 Aug 2018 18:06:08 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [217.70.183.197]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 0CD6876D for ; Fri, 10 Aug 2018 18:06:04 +0000 (UTC) X-Originating-IP: 208.91.3.26 Received: from sigabrt.benpfaff.org (unknown [208.91.3.26]) (Authenticated sender: blp@ovn.org) by relay5-d.mail.gandi.net (Postfix) with ESMTPSA id B4FB01C000B; Fri, 10 Aug 2018 18:06:01 +0000 (UTC) From: Ben Pfaff To: dev@openvswitch.org Date: Fri, 10 Aug 2018 11:05:57 -0700 Message-Id: <20180810180557.5097-1-blp@ovn.org> X-Mailer: git-send-email 2.16.1 X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Cc: Ben Pfaff Subject: [ovs-dev] [PATCH] checkpatch: Improve accuracy and specificity of sign-off checking. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org This also makes a start at a testsuite for checkpatch. CC: Aaron Conole Signed-off-by: Ben Pfaff --- tests/automake.mk | 1 + tests/checkpatch.at | 128 ++++++++++++++++++++++++++++++++++++++++++++++++ tests/testsuite.at | 1 + utilities/checkpatch.py | 56 +++++++++++++++++---- 4 files changed, 177 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..ac39d1dfdee5 --- /dev/null +++ b/tests/checkpatch.at @@ -0,0 +1,128 @@ +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" + +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..3c3d2fc7c915 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,47 @@ 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 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 +851,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