Subject: Re: Framework for identifying the job of patches is missing
To: Christian Hattemer <c.hattemer@arcor.de>
From: Roland Illig <roland.illig@gmx.de>
List: tech-pkg
Date: 03/08/2005 23:46:20
This is a multi-part message in MIME format.
--------------010205020500090205060508
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Christian Hattemer wrote:
> Hi,
> 
> in many packages the patches directory contains a bunch of patches doing
> various things. Some patches are necessary for building the package, some
> add nice litte features, some fix security issues etc.
> 
> However one doesn't know about that without deeper investigation. From
> looking at the code it often isn't easy to see what the patch changes and
> why it was changed. The cvs log messages have more details about that, but
> it is always an extra effort to look them up.
> 
> It would be nice to have something like "make describe-patches" which lists
> all patches and what they are supposed to do.
> 
> With this you could immediately check e. g. if the fix for a recent
> vulnerability is already part of the package. It also makes it easier to
> decide which patches are no longer needed when updating a package.
> 
> Comments?
> I think this would make pkgsrc a great deal more userfriendly.

I just added a check to pkglint that checks for a comment in every patch 
file. Currently it emits 17500 new warnings for uncommented patches.

Should I commit this change (after the freeze)?

Roland

--------------010205020500090205060508
Content-Type: text/plain;
 name="pkglint.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="pkglint.patch"

Index: files/pkglint.pl
===================================================================
RCS file: /cvsroot/pkgsrc/pkgtools/pkglint/files/pkglint.pl,v
retrieving revision 1.135
diff -u -p -r1.135 pkglint.pl
--- files/pkglint.pl	24 Feb 2005 22:50:44 -0000	1.135
+++ files/pkglint.pl	8 Mar 2005 22:42:05 -0000
@@ -1030,12 +1030,20 @@ sub checkfile_patches_patch($) {
 	}
 
 	# The first line should contain the RCS Id string
-	if (scalar(@$lines) == 0) {
-		log_error($fname, NO_LINE_NUMBER, "Empty patch file.");
+	if (scalar(@$lines) < 3) {
+		log_error($fname, NO_LINE_NUMBER, "File too short.");
 		return true;
-	} elsif ($lines->[0]->text !~ /^$regex_rcsidstr$/) {
+	}
+
+	if ($lines->[0]->text !~ /^$regex_rcsidstr$/) {
 		log_error($lines->[0]->file, $lines->[0]->lineno, "Expected RCS tag \"\$$conf_rcsidstr\$\" (and nothing more) here.");
 	}
+	if ($lines->[1]->text ne "") {
+		log_warning($lines->[1]->file, $lines->[1]->lineno, "Expected an empty line.");
+	}
+	if ($lines->[2]->text !~ qr"^[\w]") {
+		log_warning($lines->[2]->file, $lines->[2]->lineno, "Expected a comment describing the intent of the patch.");
+	}
 
 	foreach my $line (@$lines[1..scalar(@$lines)-1]) {
 		if ($opt_committer && $line->text =~ /$regex_known_rcs_tag/) {

--------------010205020500090205060508--