netdev - [PATCH] scripts: checkpatch: steer people away from using file paths

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230725155926.2775416-1-kuba@kernel.org>
Date: Tue, 25 Jul 2023 08:59:26 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: krzk@...nel.org
Cc: Jakub Kicinski <kuba@...nel.org>,
	joe@...ches.com,
	geert@...ux-m68k.org,
	netdev@...r.kernel.org,
	workflows@...r.kernel.org,
	mario.limonciello@....com
Subject: [PATCH] scripts: checkpatch: steer people away from using file paths

We repeatedly see noobs misuse get_maintainer by running it on
the file paths rather than the patchfile. This leads to authors
of changes (quoted commits and commits under Fixes) not getting
CCed. These are usually the best reviewers!

The file option should really not be used by noobs, unless
they are just trying to find a maintainer to manually contact.

Print a warning when someone tries to use -f and remove
the "auto-guessing" of file paths.

This script may break people's "scripts on top of get_maintainer"
if they are using -f... but that's the point.

Signed-off-by: Jakub Kicinski <kuba@...nel.org>
---
This is what I had in mind.

CC: joe@...ches.com
Cc: geert@...ux-m68k.org
Cc: krzk@...nel.org
Cc: netdev@...r.kernel.org
Cc: workflows@...r.kernel.org
Cc: mario.limonciello@....com
---
 scripts/get_maintainer.pl | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/scripts/get_maintainer.pl b/scripts/get_maintainer.pl
index ab123b498fd9..3635b3d40ebe 100755
--- a/scripts/get_maintainer.pl
+++ b/scripts/get_maintainer.pl
@@ -51,6 +51,7 @@ my $output_roles = 0;
 my $output_rolestats = 1;
 my $output_section_maxlen = 50;
 my $scm = 0;
+my $silence_file_warning = 0;
 my $tree = 1;
 my $web = 0;
 my $subsystem = 0;
@@ -267,6 +268,7 @@ if (!GetOptions(
 		'subsystem!' => \$subsystem,
 		'status!' => \$status,
 		'scm!' => \$scm,
+		'silence-file-warning!' => \$silence_file_warning,
 		'tree!' => \$tree,
 		'web!' => \$web,
 		'letters=s' => \$letters,
@@ -544,7 +546,13 @@ foreach my $file (@ARGV) {
     if ($from_filename && (vcs_exists() && !vcs_file_exists($file))) {
 	warn "$P: file '$file' not found in version control $!\n";
     }
-    if ($from_filename || ($file ne "&STDIN" && vcs_file_exists($file))) {
+    if ($from_filename) {
+	if (!$silence_file_warning) {
+	    warn "$P: WARNING: Prefer running the script on patches as "
+	        . "generated by git format-patch. Selecting paths is known "
+		. "to miss recipients!\n";
+	}
+
 	$file =~ s/^\Q${cur_path}\E//;	#strip any absolute path
 	$file =~ s/^\Q${lk_path}\E//;	#or the path to the lk tree
 	push(@files, $file);
@@ -1081,6 +1089,7 @@ version: $V
   --mailmap => use .mailmap file (default: $email_use_mailmap)
   --no-tree => run without a kernel tree
   --self-test => show potential issues with MAINTAINERS file content
+  --silence-file-warning => silence the warning about -f being used (see Notes)
   --version => show version
   --help => show this help information
 
@@ -1089,6 +1098,10 @@ version: $V
    --pattern-depth=0 --remove-duplicates --rolestats]
 
 Notes:
+  Using "-f file" is generally discouraged, running the script on a filepatch
+      (as generated by git format-patch) is usually the right thing to do.
+      Commit message is an integral part of the change and $P
+      will extract additional information from it (keywords, Fixes tags etc.)
   Using "-f directory" may give unexpected results:
       Used with "--git", git signators for _all_ files in and below
           directory are examined as git recurses directories.
-- 
2.41.0