From patchwork Tue Jan 16 13:08:48 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dumitru Ceara X-Patchwork-Id: 1887072 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=MqpUlAAy; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=openvswitch.org (client-ip=140.211.166.133; helo=smtp2.osuosl.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=patchwork.ozlabs.org) Received: from smtp2.osuosl.org (smtp2.osuosl.org [140.211.166.133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4TDq8r2khbz1yPJ for ; Wed, 17 Jan 2024 00:09:14 +1100 (AEDT) Received: from localhost (localhost [127.0.0.1]) by smtp2.osuosl.org (Postfix) with ESMTP id 30F5342467; Tue, 16 Jan 2024 13:09:12 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org 30F5342467 Authentication-Results: smtp2.osuosl.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=MqpUlAAy X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp2.osuosl.org ([127.0.0.1]) by localhost (smtp2.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id CUebd2cwXgHV; Tue, 16 Jan 2024 13:09:10 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [IPv6:2605:bc80:3010:104::8cd3:938]) by smtp2.osuosl.org (Postfix) with ESMTPS id A3FAE41D78; Tue, 16 Jan 2024 13:09:09 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp2.osuosl.org A3FAE41D78 Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 79FC9C0072; Tue, 16 Jan 2024 13:09:09 +0000 (UTC) X-Original-To: ovs-dev@openvswitch.org Delivered-To: ovs-dev@lists.linuxfoundation.org Received: from smtp4.osuosl.org (smtp4.osuosl.org [IPv6:2605:bc80:3010::137]) by lists.linuxfoundation.org (Postfix) with ESMTP id C14DAC0037 for ; Tue, 16 Jan 2024 13:09:08 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by smtp4.osuosl.org (Postfix) with ESMTP id 9BFD541570 for ; Tue, 16 Jan 2024 13:09:08 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 9BFD541570 Authentication-Results: smtp4.osuosl.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=MqpUlAAy X-Virus-Scanned: amavisd-new at osuosl.org Received: from smtp4.osuosl.org ([127.0.0.1]) by localhost (smtp4.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ziMaXRUXnHNc for ; Tue, 16 Jan 2024 13:09:07 +0000 (UTC) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by smtp4.osuosl.org (Postfix) with ESMTPS id 94354409C4 for ; Tue, 16 Jan 2024 13:09:07 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 smtp4.osuosl.org 94354409C4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1705410546; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=OantYdwu6aQM6QWm4PYD98xCip2ntptcaprMTAteG/Q=; b=MqpUlAAy+S6snShCmXmcu3yGO7mKT/B2rEkvnxfVMdnKyRXQnE+acEC9PvyKb+7/p2CAiU JdhCzpO1ewndSeLdVfsSeCBal+16rcCTXhN7C3K7WsWJlNdvAhYR+j19PdQ4eS69Mfa7wo ZnNnsgB0pgfNU3799t8Ipt0LMwUfLI8= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-14-i7koXl5ONTOLOtVxI6Aohw-1; Tue, 16 Jan 2024 08:09:03 -0500 X-MC-Unique: i7koXl5ONTOLOtVxI6Aohw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id DB5BD38135E7; Tue, 16 Jan 2024 13:09:00 +0000 (UTC) Received: from dceara.remote.csb (unknown [10.39.195.20]) by smtp.corp.redhat.com (Postfix) with ESMTP id 08E221121306; Tue, 16 Jan 2024 13:08:59 +0000 (UTC) From: Dumitru Ceara To: ovs-dev@openvswitch.org Date: Tue, 16 Jan 2024 14:08:48 +0100 Message-Id: <20240116130848.1341709-1-dceara@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Subject: [ovs-dev] [PATCH ovn] checkpatch.py: Port checkpatch related changes from the OVS repo. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: ovs-dev-bounces@openvswitch.org Sender: "dev" This picks up the following OVS changes: 00d3d4a7d375 ("checkpatch: Avoid catastrophic backtracking.") d25c6bd8df37 ("checkpatch: Reorganize flagged words using a list.") 9a50170a805a ("checkpatch: Add suggestions to the spell checker.") 799f697e51ec ("checkpatch: Print subject field if misspelled or missing.") 1b8fa4a66aa4 ("checkpatch: Add checks for the subject line.") bf843fd439b2 ("checkpatch: Don't spell check Fixes tag.") 74bfe3701407 ("checkpatch: Add argument to skip committer signoff check.") 21c61243fb75 ("checkpatch: Fix personal word list storage.") 915b97971d58 ("checkpatch.py: Load codespell dictionary.") Signed-off-by: Dumitru Ceara Acked-by: Numan Siddique --- tests/checkpatch.at | 45 +++++++++++++++++ utilities/checkpatch.py | 104 ++++++++++++++++++++++++++++++++++++---- 2 files changed, 140 insertions(+), 9 deletions(-) diff --git a/tests/checkpatch.at b/tests/checkpatch.at index 26f3197053..e7322fff48 100755 --- a/tests/checkpatch.at +++ b/tests/checkpatch.at @@ -8,7 +8,14 @@ OVS_START_SHELL_HELPERS try_checkpatch() { # 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. + # If it does not have a 'Subject', add a valid one. echo "$1" | sed 's/^ //' > test.patch + if grep 'Subject\:' test.patch >/dev/null 2>&1; then : + else + sed -i'' -e '1i\ +Subject: Patch this is. +' test.patch + fi if grep '---' expout >/dev/null 2>&1; then : else printf '\n---\n' >> test.patch @@ -236,6 +243,22 @@ done AT_CLEANUP +AT_SETUP([checkpatch - catastrophic backtracking]) +dnl Special case this rather than using the above construct because sometimes a +dnl warning needs to be generated for line lengths (f.e. when the 'while' +dnl keyword is used). +try_checkpatch \ + "COMMON_PATCH_HEADER + + if (!b_ctx_in->chassis_rec || !b_ctx_in->br_int || !b_ctx_in->ovs_idl_txn) + " \ + "ERROR: Inappropriate bracing around statement + #8 FILE: A.c:1: + if (!b_ctx_in->chassis_rec || !b_ctx_in->br_int || !b_ctx_in->ovs_idl_txn) +" + +AT_CLEANUP + + AT_SETUP([checkpatch - parenthesized constructs - for]) try_checkpatch \ "COMMON_PATCH_HEADER @@ -560,3 +583,25 @@ try_checkpatch \ " AT_CLEANUP + +AT_SETUP([checkpatch - subject]) +try_checkpatch \ + "Author: A + Commit: A + Subject: netdev: invalid case and dot ending + + Signed-off-by: A" \ + "WARNING: The subject summary should start with a capital. + WARNING: The subject summary should end with a dot. + Subject: netdev: invalid case and dot ending" + +try_checkpatch \ + "Author: A + Commit: A + Subject: netdev: This is a way to long commit summary and therefor it should report a WARNING! + + Signed-off-by: A" \ + "WARNING: The subject, ': ', is over 70 characters, i.e., 85. + Subject: netdev: This is a way to long commit summary and therefor it should report a WARNING!" + +AT_CLEANUP diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index 5467d604d1..52d3fa8455 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -40,6 +40,15 @@ MAX_LINE_LEN = 79 def open_spell_check_dict(): import enchant + try: + import codespell_lib + codespell_dir = os.path.dirname(codespell_lib.__file__) + codespell_file = os.path.join(codespell_dir, 'data', 'dictionary.txt') + if not os.path.exists(codespell_file): + codespell_file = '' + except: + codespell_file = '' + try: extra_keywords = ['ovs', 'vswitch', 'vswitchd', 'ovs-vswitchd', 'netdev', 'selinux', 'ovs-ctl', 'dpctl', 'ofctl', @@ -92,9 +101,18 @@ def open_spell_check_dict(): 'syscall', 'lacp', 'ipf', 'skb', 'valgrind'] global spell_check_dict + spell_check_dict = enchant.Dict("en_US") + + if codespell_file: + with open(codespell_file) as f: + for line in f.readlines(): + words = line.strip().split('>')[1].strip(', ').split(',') + for word in words: + spell_check_dict.add_to_session(word.strip()) + for kw in extra_keywords: - spell_check_dict.add(kw) + spell_check_dict.add_to_session(kw) return True except: @@ -190,6 +208,7 @@ skip_trailing_whitespace_check = False skip_gerrit_change_id_check = False skip_block_whitespace_check = False skip_signoff_check = False +skip_committer_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. @@ -282,9 +301,13 @@ def if_and_for_end_with_bracket_check(line): if len(line) == MAX_LINE_LEN - 1 and line[-1] == ')': return True - if __regex_ends_with_bracket.search(line) is None and \ - __regex_if_macros.match(line) is None: - return False + if __regex_ends_with_bracket.search(line) is None: + if line.endswith("\\") and \ + __regex_if_macros.match(line) is not None: + return True + else: + return False + if __regex_conditional_else_bracing.match(line) is not None: return False if __regex_conditional_else_bracing2.match(line) is not None: @@ -410,9 +433,15 @@ def check_spelling(line, comment): if not spell_check_dict or not spellcheck: return False + if line.startswith('Fixes: '): + return False + words = filter_comments(line, True) if comment else line words = words.replace(':', ' ').split(' ') + flagged_words = [] + num_suggestions = 3 + for word in words: skip = False strword = re.subn(r'\W+', '', word)[0].replace(',', '') @@ -437,9 +466,15 @@ def check_spelling(line, comment): skip = True if not skip: - print_warning("Check for spelling mistakes (e.g. \"%s\")" - % strword) - return True + flagged_words.append(strword) + + if len(flagged_words) > 0: + for mistake in flagged_words: + print_warning("Possible misspelled word: \"%s\"" % mistake) + print("Did you mean: ", + spell_check_dict.suggest(mistake)[:num_suggestions]) + + return True return False @@ -780,6 +815,36 @@ def run_file_checks(text): check['check'](text) +def run_subject_checks(subject, spellcheck=False): + warnings = False + + if spellcheck and check_spelling(subject, False): + warnings = True + + summary = subject[subject.rindex(': ') + 2:] + area_summary = subject[subject.index(': ') + 2:] + area_summary_len = len(area_summary) + if area_summary_len > 70: + print_warning("The subject, ': ', is over 70 " + "characters, i.e., %u." % area_summary_len) + warnings = True + + if summary[0].isalpha() and summary[0].islower(): + print_warning( + "The subject summary should start with a capital.") + warnings = True + + if subject[-1] not in [".", "?", "!"]: + print_warning( + "The subject summary should end with a dot.") + warnings = True + + if warnings: + print(subject) + + return warnings + + def ovs_checkpatch_parse(text, filename, author=None, committer=None): global print_file_name, total_line, checking_file, \ empty_return_check_state @@ -800,6 +865,7 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): 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'^(Commit: )(.*)$', re.I | re.M | re.S) + is_subject = re.compile(r'^(Subject: )(.*)$', 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: )(.*)$', @@ -874,7 +940,8 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): break if (committer and author != committer - and committer not in signatures): + and committer not in signatures + and not skip_committer_signoff_check): print_error("Committer %s needs to sign off." % committer) @@ -899,6 +966,8 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): committer = is_committer.match(line).group(2) elif is_author.match(line): author = is_author.match(line).group(2) + elif is_subject.match(line): + run_subject_checks(line, spellcheck) elif is_signature.match(line): m = is_signature.match(line) signatures.append(m.group(2)) @@ -990,7 +1059,8 @@ Check options: -S|--spellcheck Check C comments and commit-message for possible spelling mistakes -t|--skip-trailing-whitespace Skips the trailing whitespace test - --skip-gerrit-change-id Skips the gerrit change id test""" + --skip-gerrit-change-id Skips the gerrit change id test + --skip-committer-signoff Skips the committer sign-off test""" % sys.argv[0]) @@ -1017,6 +1087,19 @@ def ovs_checkpatch_file(filename): result = ovs_checkpatch_parse(part.get_payload(decode=False), filename, mail.get('Author', mail['From']), mail['Commit']) + + if not mail['Subject'] or not mail['Subject'].strip(): + if mail['Subject']: + mail.replace_header('Subject', sys.argv[-1]) + else: + mail.add_header('Subject', sys.argv[-1]) + + print("Subject missing! Your provisional subject is", + mail['Subject']) + + if run_subject_checks('Subject: ' + mail['Subject'], spellcheck): + result = True + ovs_checkpatch_print_result() return result @@ -1048,6 +1131,7 @@ if __name__ == '__main__': "skip-signoff-lines", "skip-trailing-whitespace", "skip-gerrit-change-id", + "skip-committer-signoff", "spellcheck", "quiet"]) except: @@ -1068,6 +1152,8 @@ if __name__ == '__main__': skip_trailing_whitespace_check = True elif o in ("--skip-gerrit-change-id"): skip_gerrit_change_id_check = True + elif o in ("--skip-committer-signoff"): + skip_committer_signoff_check = True elif o in ("-f", "--check-file"): checking_file = True elif o in ("-S", "--spellcheck"):