From e2cc546ce51007b667a7927a6e536a9ed68f3d1c Mon Sep 17 00:00:00 2001 From: hubertf Date: Mon, 28 Jun 2004 14:33:47 +0000 Subject: Update to 3.82, committing yesterday's changes from Roland Illig. Changes: - Handle a rarely used patch format correctly - Do not output superflouus linebreaks anymore - Check the contents of the COMMENT field only if it exists - Handle invalid dependency specifications correctly - Correctly handle Makefile variables ordering - Output complete filename of absolute filenames - Added a new scheme for loading text files into memory so that the location of an error or warning can be given exactly (file + line) to the user. - Refactored the checkfile_patches_patch subprogram to use the new text file processing and to make clearer what is checked. --- pkgtools/pkglint/Makefile | 4 +- pkgtools/pkglint/files/pkglint.pl | 209 ++++++++++++++++++++++---------------- 2 files changed, 124 insertions(+), 89 deletions(-) (limited to 'pkgtools') diff --git a/pkgtools/pkglint/Makefile b/pkgtools/pkglint/Makefile index 82ab4902b89..692c61c1d63 100644 --- a/pkgtools/pkglint/Makefile +++ b/pkgtools/pkglint/Makefile @@ -1,7 +1,7 @@ -# $NetBSD: Makefile,v 1.194 2004/06/28 09:55:44 abs Exp $ +# $NetBSD: Makefile,v 1.195 2004/06/28 14:33:47 hubertf Exp $ # -DISTNAME= pkglint-3.81 +DISTNAME= pkglint-3.82 CATEGORIES= pkgtools devel MASTER_SITES= # empty DISTFILES= # empty diff --git a/pkgtools/pkglint/files/pkglint.pl b/pkgtools/pkglint/files/pkglint.pl index 932ad4f2022..8836f5df2a8 100644 --- a/pkgtools/pkglint/files/pkglint.pl +++ b/pkgtools/pkglint/files/pkglint.pl @@ -11,7 +11,7 @@ # Freely redistributable. Absolutely no warranty. # # From Id: portlint.pl,v 1.64 1998/02/28 02:34:05 itojun Exp -# $NetBSD: pkglint.pl,v 1.108 2004/06/26 18:53:17 hubertf Exp $ +# $NetBSD: pkglint.pl,v 1.109 2004/06/28 14:33:47 hubertf Exp $ # # This version contains lots of changes necessary for NetBSD packages # done by Hubert Feyrer , @@ -42,6 +42,10 @@ my $opt_dumpmakefile = 0; # my $opt_contblank = 1; # number of allowed contigoous blank lines my $opt_packagedir = "."; # +# Constants +my $regex_rcsidstr = qr"\$($conf_rcsidstr)(?::[^\$]*|)\$"; +my $regex_known_rcs_tag = qr"\$(Author|Date|Header|Id|Locker|Log|Name|RCSfile|Revision|Source|State|$conf_rcsidstr)(?::[^\$]*?|)\$"; + # Global variables that should be eliminated by the next refactoring. my %definesfound = (); my $errors = 0; # number of errors @@ -294,6 +298,30 @@ if ($opt_committer) { } print_summary_and_exit(); +# +# Subroutines common to all checking routines +# + +# Loads a text file completely into memory. Returns undef on error +# or a reference to an array of lines. A line itself is an array of +# three components: the originating file, the line number and the +# contents of the line. +sub load_file($) { + my ($fname) = @_; + my ($result, $line, $lineno); + + $result = []; + open(F, "< $fname") or return undef; + $lineno = 0; + while (defined($line = )) { + $lineno++; + $line =~ s/\r*\n*\z//; + push(@$result, [$fname, $lineno, $line]); + } + close(F) or return undef; + return $result; +} + sub checkfile_DESCR($) { local($file) = @_; local(%maxchars) = ('DESCR', 80); @@ -608,75 +636,81 @@ sub checklastline($) { return 1; } +# $lines => an array of lines as returned by load_file(). +sub check_for_multiple_patches($) { + my ($lines) = @_; + my ($files_in_patch, $patch_state); + + $files_in_patch = 0; + $patch_state = ""; + foreach my $line (@$lines) { + if (index($line->[2], "--- ") == 0 && $line->[2] !~ qr"^--- \d+(?:,\d+|) ----$") { + $line_type = "-"; + } elsif (index($line->[2], "*** ") == 0 && $line->[2] !~ qr"^\*\*\* \d+(?:,\d+|) \*\*\*\*$") { + $line_type = "*"; + } elsif (index($line->[2], "+++ ") == 0) { + $line_type = "+"; + } else { + $line_type = ""; + } + + if ($patch_state eq "*") { + if ($line_type eq "-") { + $files_in_patch++; + $patch_state = ""; + } else { + log_warning($line->[0], $line->[1], "unknown patch format (might be an internal error)"); + } + } elsif ($patch_state eq "-") { + if ($line_type eq "+") { + $files_in_patch++; + $patch_state = ""; + } else { + log_warning($line->[0], $line->[1], "unknown patch format (might be an internal error)"); + } + } elsif ($patch_state eq "") { + $patch_state = $line_type; + } + } + + if ($files_in_patch > 1) { + log_warning($lines->[0]->[0], NO_LINE_NUMBER, "contains patches for $files_in_patch files, should be only one"); + } elsif ($files_in_patch == 0) { + log_warning($lines->[0]->[0], NO_LINE_NUMBER, "contains no patch"); + } + return 1; +} + sub checkfile_patches_patch($) { my ($file) = @_; - my ($rcsidseen, $whole, @lines, $line); - $rcsidseen = 0; + my ($fname) = "$opt_packagedir/$file"; + my ($lines); if ($file =~ /.*~$/) { - log_warning(NO_FILE, NO_LINE_NUMBER, "is $file a backup file? If so, please remove it \n" - ." and rerun '$conf_make makepatchsum'"); + log_warning($fname, NO_LINE_NUMBER, "In case this is a backup file: please remove it and rerun '$conf_make makepatchsum'"); } - &checkperms("$opt_packagedir/$file"); - - open(IN, "< $opt_packagedir/$file") || return 0; - $whole = ''; - while (defined($line = )) { - $rcsidseen++ if ($line =~ /\$$conf_rcsidstr[:\$]/); - $whole .= $line; - push(@lines, $line); - } - if ($opt_committer && $whole =~ /.\$(Author|Date|Header|Id|Locker|Log|Name|RCSfile|Revision|Source|State|NetBSD)(:.*\$|\$)/) { # XXX - # RCS ID in very first line is ok, to identify version - # of patch (-> only warn if there's something before the - # actual $RCS_ID$, not on BOF - '.' won't match there) - log_warning(NO_FILE, NO_LINE_NUMBER, "$file includes possible RCS tag \"\$$1\$\". ". - "use binary mode (-ko) on commit/import."); + checkperms($fname); + if (!defined($lines = load_file($fname))) { + log_error($fname, NO_LINE_NUMBER, "Could not load file."); + return 0; } - if (!$rcsidseen) { - log_error(NO_FILE, NO_LINE_NUMBER, "RCS tag \"\$$conf_rcsidstr\$\" must be present ". - "in patch $file.") + + # The first line should contain the RCS Id string + if (scalar(@$lines) == 0) { + log_error($fname, NO_LINE_NUMBER, "Empty patch file."); + return 0; + } elsif ($lines->[0]->[2] !~ /^$regex_rcsidstr$/) { + log_error($lines->[0]->[0], $lines->[0]->[1], "Expected RCS tag \"\$$conf_rcsidstr\$\" (and nothing more) here."); } - close(IN); - $files_in_patch = 0; - $patch_state = ""; - foreach my $patch_line (@lines) { - chomp($patch_line); - if (index($patch_line, "--- ") == 0 && $patch_line !~ qr"^--- \d+,\d+ ----$") { - $line_type = "-"; - } elsif (index($patch_line, "*** ") == 0 && $patch_line !~ qr"^\*\*\* \d+,\d+ \*\*\*\*$") { - $line_type = "*"; - } elsif (index($patch_line, "+++ ") == 0) { - $line_type = "+"; - } else { - $line_type = ""; - } - if ($patch_state eq "*") { - if ($line_type eq "-") { - $files_in_patch++; - $patch_state = ""; - } else { - log_warning(NO_FILE, NO_LINE_NUMBER, "$i:$.: unknown patch format (might be an internal error)"); + foreach my $line (@$lines[1..scalar(@$lines)-1]) { + if ($opt_committer && $line->[2] =~ /$regex_known_rcs_tag/) { + log_warning($line->[0], $line->[1], "Possible RCS tag \"\$$1\$\". Use binary mode (-ko) on commit/import."); } - } elsif ($patch_state eq "-") { - if ($line_type eq "+") { - $files_in_patch++; - $patch_state = ""; - } else { - log_warning(NO_FILE, NO_LINE_NUMBER, "$i:$.: unknown patch format (might be an internal error)"); - } - } elsif ($patch_state eq "") { - $patch_state = $line_type; - } - #printf("%s:%d: state=(%s), line=(%s)\n", $i, $., $patch_state, $line_type); - } - if ($files_in_patch > 1) { - log_warning(NO_FILE, NO_LINE_NUMBER, "patch `$i' contains patches for more than one file, namely $files_in_patch"); - } elsif ($files_in_patch == 0) { - log_warning(NO_FILE, NO_LINE_NUMBER, "patch `$i' contains no patch"); } + + check_for_multiple_patches($lines); return 1; } @@ -706,7 +740,7 @@ sub readmakefile($) { if ($line =~ /^\.\s*if\s+!defined\s*\((\w+)\)/) { if ($definesfound{$1}) { $level = 1; - log_info($file, $., "omitting contents of !defined($1)\n"); + log_info($file, $., "omitting contents of !defined($1)"); $contents .= "# omitted inclusion for !defined($1) here\n"; while (defined($line = <$handle>)) { if ($line =~ /^\.\s*if\s+/) { @@ -767,7 +801,7 @@ sub readmakefile($) { sub checkfile_Makefile($) { local($file) = @_; local($rawwhole, $whole, $idx, @sections); - local($tmp, $tmp2); + local($tmp); local($i, $j, $k, $l); local(@varnames) = (); local($distfiles, $pkgname, $svrpkgname, $distname, @@ -1136,7 +1170,7 @@ EOF log_info(NO_FILE, NO_LINE_NUMBER, "URL \"$i\" ok."); } } else { - log_info(NO_FILE, NO_LINE_NUMBER, "non-URL \"$i\" ok.\n"); + log_info(NO_FILE, NO_LINE_NUMBER, "non-URL \"$i\" ok."); } if ($tmp =~ /\nDYNAMIC_MASTER_SITES[+?]?=/) { log_warning(NO_FILE, NO_LINE_NUMBER, "MASTER_SITES and DYNAMIC_MASTER_SITES ". @@ -1340,20 +1374,21 @@ EOF # warnings for missing COMMENT if ($tmp !~ /\nCOMMENT=\s*(.*)$/) { log_error(NO_FILE, NO_LINE_NUMBER, "please add a short COMMENT describing the package."); - } - # and its properties: - $tmp2 = $1; - if ($tmp2 =~ /\.$/i) { - log_warning(NO_FILE, NO_LINE_NUMBER, "COMMENT should not end with a '.' (period)."); - } - if ($tmp2 =~ /^(a|an) /i) { - log_warning(NO_FILE, NO_LINE_NUMBER, "COMMENT should not begin with '$1 '."); - } - if ($tmp2 =~ /^[a-z]/) { - log_warning(NO_FILE, NO_LINE_NUMBER, "COMMENT should start with a capital letter."); - } - if (length($tmp2) > 70) { - log_warning(NO_FILE, NO_LINE_NUMBER, "COMMENT should not be longer than 70 characters."); + } else { + # and its properties: + my $tmp2 = $1; + if ($tmp2 =~ /\.$/i) { + log_warning(NO_FILE, NO_LINE_NUMBER, "COMMENT should not end with a '.' (period)."); + } + if ($tmp2 =~ /^(a|an) /i) { + log_warning(NO_FILE, NO_LINE_NUMBER, "COMMENT should not begin with '$1 '."); + } + if ($tmp2 =~ /^[a-z]/) { + log_warning(NO_FILE, NO_LINE_NUMBER, "COMMENT should start with a capital letter."); + } + if (length($tmp2) > 70) { + log_warning(NO_FILE, NO_LINE_NUMBER, "COMMENT should not be longer than 70 characters."); + } } &checkearlier($tmp, @varnames); @@ -1426,14 +1461,16 @@ EOF } # check pkg dir existence - $k = (split(':', $k))[1]; - $k =~ s/\${PKGSRCDIR}/$ENV{'PKGSRCDIR'}/; - if (! -d "$opt_packagedir/$k") { - log_warning(NO_FILE, NO_LINE_NUMBER, "no package directory $k ". - "found, even though it is ". - "listed in $j."); + my @m = split(/:/, $k); + if ($#m >= 1) { + $m[1] =~ s/\${PKGSRCDIR}/$ENV{'PKGSRCDIR'}/; + if (! -d "$opt_packagedir/$m[1]") { + log_warning(NO_FILE, NO_LINE_NUMBER, "no package directory $m[1] found, even though it is listed in $j."); + } else { + log_info(NO_FILE, NO_LINE_NUMBER, "package directory $m[1] found."); + } } else { - log_info(NO_FILE, NO_LINE_NUMBER, "package directory $k found."); + log_error(NO_FILE, NO_LINE_NUMBER, "invalid package dependency specification \"$k\"."); } } } @@ -1577,7 +1614,7 @@ sub checkorder($$@) { while ($k < scalar(@order) && $order[$k] ne $i) { $k++; } - if ($order[$k] eq $i) { + if ($k <= $#order) { if ($k < $j) { log_error(NO_FILE, NO_LINE_NUMBER, "$i appears out-of-order."); $invalidorder++; @@ -1633,9 +1670,7 @@ sub abspathname($$) { if ($i ne '') { $i =~ s/\s.*$//; $i =~ s/['"].*$//; - $i = substr($i, 0, 20) . '...' if (20 < length($i)); - log_warning(NO_FILE, NO_LINE_NUMBER, "possible use of absolute pathname ". - "\"$i\", in $file."); + log_warning($file, NO_LINE_NUMBER, "possible use of absolute pathname \"$i\"."); } } -- cgit v1.2.3