From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-11.5 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7AA1AC43461 for ; Sun, 6 Sep 2020 10:00:02 +0000 (UTC) Received: from fraxinus.osuosl.org (smtp4.osuosl.org [140.211.166.137]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5943C20C09 for ; Sun, 6 Sep 2020 10:00:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=disroot.org header.i=@disroot.org header.b="OkqcoDWt" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5943C20C09 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=disroot.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linux-kernel-mentees-bounces@lists.linuxfoundation.org Received: from localhost (localhost [127.0.0.1]) by fraxinus.osuosl.org (Postfix) with ESMTP id 0314A85F08; Sun, 6 Sep 2020 10:00:01 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from fraxinus.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id VAzkKX9DaRYb; Sun, 6 Sep 2020 09:59:59 +0000 (UTC) Received: from lists.linuxfoundation.org (lf-lists.osuosl.org [140.211.9.56]) by fraxinus.osuosl.org (Postfix) with ESMTP id B873C85EF3; Sun, 6 Sep 2020 09:59:59 +0000 (UTC) Received: from lf-lists.osuosl.org (localhost [127.0.0.1]) by lists.linuxfoundation.org (Postfix) with ESMTP id 9C9D0C0859; Sun, 6 Sep 2020 09:59:59 +0000 (UTC) Received: from silver.osuosl.org (smtp3.osuosl.org [140.211.166.136]) by lists.linuxfoundation.org (Postfix) with ESMTP id C459FC0051 for ; Sun, 6 Sep 2020 09:59:57 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by silver.osuosl.org (Postfix) with ESMTP id 945442038B for ; Sun, 6 Sep 2020 09:59:57 +0000 (UTC) X-Virus-Scanned: amavisd-new at osuosl.org Received: from silver.osuosl.org ([127.0.0.1]) by localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id TqjXsT7wy0yo for ; Sun, 6 Sep 2020 09:59:55 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from knopi.disroot.org (knopi.disroot.org [178.21.23.139]) by silver.osuosl.org (Postfix) with ESMTPS id EEC2B2037A for ; Sun, 6 Sep 2020 09:59:54 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by disroot.org (Postfix) with ESMTP id B4171529B5; Sun, 6 Sep 2020 11:59:52 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at disroot.org Received: from knopi.disroot.org ([127.0.0.1]) by localhost (disroot.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 2fza_7aRBkGP; Sun, 6 Sep 2020 11:59:51 +0200 (CEST) Mime-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=disroot.org; s=mail; t=1599386390; bh=9szsjSVjl0qTdDS1/uS6NIyJTD1/Y9aeXx6aN1njXgo=; h=Date:From:Subject:To:Cc:In-Reply-To:References; b=OkqcoDWtXmx/IV5FBxzbvJ8g76t9NAkFrUy2ceWg8fRkrrsChlbTMMn1/PK1DtRJw fLS6KTysKsueE1EDPc/FO26wq3ZmW1d8J6jYRDYmr2XJw1vwn4ETgkyP5DcNvXlOVf xE2PMj2iZU2ZfO8NiqAHQfCxQsslBiAXVTLOyi4buL3YT6/cwHXgHcXxeNTSKkNMkc QOJtg3NtcVsvsq8loZBVC2SSY/sG+tZ7bUhUqbAH0ZcqkjiHQsfy5yNHXQ15yfynKz T4V5UOfpZfQHwxCHJg27VsnFR3Ll6ciBJmD7kB1jaN3zkjwq3GTKp8+qIw8ctwwBMx jvHviKCxjKZ8A== Date: Sun, 06 Sep 2020 09:59:48 +0000 From: "Ayush" Message-ID: To: "Lukas Bulwahn" In-Reply-To: References: <20200810125354.xeijyh3v5upatrez@salamander> <4f5612540c184970157a98d103d7cd04@disroot.org> <32595de077ee7d674a628df11c58152f@disroot.org> <97cef44dfa6c692c20ef7a0d981e4863@disroot.org> Cc: linux-kernel-mentees@lists.linuxfoundation.org Subject: Re: [Linux-kernel-mentees] Regarding "Linux Kernel: Evaluate and Improve checkpatch.pl" X-BeenThere: linux-kernel-mentees@lists.linuxfoundation.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: linux-kernel-mentees-bounces@lists.linuxfoundation.org Sender: "Linux-kernel-mentees" August 31, 2020 10:44 AM, "Lukas Bulwahn" wrote: > On Sun, 30 Aug 2020, Ayush wrote: > >> August 27, 2020 10:59 AM, "Lukas Bulwahn" wrote: >> >> On Mon, 24 Aug 2020, Ayush wrote: >> >> August 22, 2020 1:36 PM, "Lukas Bulwahn" wrote: >> >> On Fri, 21 Aug 2020, Ayush wrote: >> >> Hints to the first task: >> >> Can you create a list of all non-merge commits that were added in the >> version v5.8 of the kernel, i.e., all non-merge commits that are in v5.8 >> and not already in v5.7? >> >> Can you share the script/command you executed and the resulting list on >> github? >> >> Can you run your script on all commits of this list above and record >> all checkpatch.pl reports, and store them in your github repository? >> >> Can you suggest ideas how to aggregate the findings and create a >> statistics? For example: Which type of error is reported most? >> Can you implement that idea? >> >> I also suggest to have a look at >> the options ./scripts/checkpatch.pl --list-types and >> ./scripts/checkpatch.pl --show-types. The option --show-types changes >> the output of checkpatch.pl to list type identifiers, so it is easier >> to parse and aggregate the output. >> >> Please also share the script you create for that purpose on your >> github repository. >> >> The second task is to pick one warning that appears often and improve >> checkpatch.pl to handle that better and get it accepted by the kernel >> community. >> >> Hints to the second task follow when the first task is solved. >> >> If you fail on any of those tasks, you are out of the selection process. >> >> Lukas >> >> Sir, >> >> I have attempted the task 1 and pushed the same to GitHub. >> >> Please have a look and suggest improvements. >> >> https://github.com/eldraco19/evalute_improve_checkpatch_pl >> >> Please let me know if there are any issues with this. >> >> So far, so good. >> >> Here are the questions we want to answer: >> >> - So what are the 20 categories that occur most? >> >> You are getting close to that answer, but you are not there yet. >> >> Then look at the findings. For those 20 categories, are there specific >> findings that are multiple times false positives? >> >> So, the script complains about something, but it does not get that the >> patch author wrote something completely unrelated to the error message. >> >> Lukas >> >> Sir, >> >> I tried the given tasks and it can be found here, >> >> https://github.com/eldraco19/evalute_improve_checkpatch_pl/blob/master/STATS.md >> >> The solution is implemented a bit complicated, but well, at least, it >> works if I believe your report. (I only read the code, but did not run >> it.) >> >> The goal now is to find a class of false positives and improve >> checkpatch.pl accordingly. >> >> I suggest that you look at the specific DIFF_IN_COMMIT_MSG reported >> errors? >> >> Provide a short assessment for each DIFF_IN_COMMIT_MSG error in the >> 10 commits. >> >> It should tell: >> - what lines in the commit message did checkpatch.pl complain about? >> - what is the pattern in the commit message? >> - does patch(1) really stumble over that pattern? >> - how would this pattern need to be provided to patch(1) so that it >> would stumble over it? >> - if no, why not? >> - can we change checkpatch.pl to not raise an error for such a >> situation? So, only raise an error when the pattern would really make >> patch stumble on it? >> >> Depending on the evaluation, we might continue to improve checkpatch.pl >> for reporting this error type, or we decide to look at GIT_COMMIT_ID >> errors, where I can quickly spot some false positives. >> >> Best regards, >> >> Lukas >> >> Sir, >> >> I analysed the given error type and my analysis can be found here: >> >> https://github.com/eldraco19/evalute_improve_checkpatch_pl/blob/master/DIFF_IN_COMMIT_MSG.md > > Evaluation looks sound. Although, I cannot really see the analysis of all > 10 commits. You say the 10 commits fall into two classes, but how can > anyone else judge this from your report? > > I also do not fully understand your conclusion; to me, it seems to > contradict itself. Fortunately, I think your analysis suggests that there > is not a clear improvement to checkpatch.pl, as far as I see. > > So, I do not think that this is a good starting point for a change of > checkpatch.pl. > > I suggest that you look at the error type GIT_COMMIT_ID. I have found some > cases that seem to be suitable for improvement of the checkpatch.pl > script. > > Lukas Sir, I have been looking for more improvements in checkpatch.pl, especially with GIT_COMMIT_ID. I found that commits which mentioned "revert commits" in their description will get error even if they follow the proper syntax. for example: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20200903&id=e8a170ff9a3576730e43c0dbdd27b7cd3dc56848 In this example, commit description has, commit 193392ed9f69 ("Revert "drm/amd/display: add -msse2 to prevent Clang from emitting libcalls to undefined SW FP routines"") which is correct as per the syntax, but checkpatch still gives an error. So, I tried to fix this by: --- diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 149518d2a6a7..01df2b9b2236 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2874,6 +2874,9 @@ sub process { $rawlines[$linenr] =~ /^\s*([^"]+)"\)/; $orig_desc .= " " . $1; $hasparens = 1; + } elsif ($line =~ /\bcommit\s+[0-9a-f]{5,}\s+\("(Revert "[^"]+[^"]")"\)/i) { + $orig_desc = $1; + $hasparens = 1; } ($id, $description) = git_commit_info($orig_commit, --- (on linux next-20200903) This patch fixes the issues with commits of the similar type given in the above example but some cases like - commit 1234567890ab ("Revert "foo bar"") - commit 1234567890ab ("Revert "foo bar"") - commit 1234567890ab ("Revert "foo bar"") basically the cases where next-line comes are not handled. But there can a lot of different patterns where next-line is coming, so do we add a separate if condition for all the patterns? or we just continue giving an error in case of next-line? Please look into this. Thanks, Ayush _______________________________________________ Linux-kernel-mentees mailing list Linux-kernel-mentees@lists.linuxfoundation.org https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees