Date: Wed, 13 Dec 2000 06:00:32 +0900 From: "Akinori MUSHA" <knu@iDaemons.org> To: Peter Pentchev <roam@orbitel.bg> Cc: mharo@FreeBSD.org, knu@FreeBSD.org, ports@FreeBSD.org Subject: Re: portlint - minor patches Message-ID: <868zplwe1b.wl@archon.local.idaemons.org> In-Reply-To: <20001212174943.C36405@ringworld.oblivion.bg> References: <20001212174943.C36405@ringworld.oblivion.bg>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi,
At Tue, 12 Dec 2000 17:49:43 +0200,
Peter Pentchev wrote:
> Firstly, for quite some time portlint -abcNv has not worked because
> the getopt parameter string incorrectly specified a ':' after 'N'
> instead of after 'B', thus also disabling the -B# functionality.
Good catch! And that confirms me in my longstanding vague suspicion
that the -N option hasn't been working. :(
When you do this:
$ portlint -abcN
-N isn't properly recognized because it has no following argument that
getopt() expects.
> Then, there's the other thing - when checking for contiguous blank
> lines, portlint makes an additional effort to not check patchfiles -
> it is legitimate for a patch to end with one or more blank lines of
> context. However, the check is slightly broken - it assumes that
> filenames are taken relative to the port's directory, while now filenames
> are absolute. Thus, the /^files\/patch-/ match fails, and patchfiles
> are also checked for blank lines at the end.
I drastically changed that part to check more effectively and strictly
in an extensible way using File::Find module. I added some extra
checks for the existence of symlinks, dotfiles, core files, and CVS
directories (only when -N is given), and more possible backup files
like *~, #*, and *.bak.
While I was at it, I also did a trivial cosmetic change against "OK:
checking <n>th section of $file ..." messages to make them be uniform.
Michael, would you review the attached patch when you've got the time?
I've fully tested it, but possibly I am missing something. Feel free
to point out anything you notice.
Regards,
--
/
/__ __ Akinori.org / MUSHA.org
/ ) ) ) ) / FreeBSD.org / Ruby-lang.org
Akinori MUSHA aka / (_ / ( (__( @ iDaemons.org / and.or.jp
"We're only at home when we're on the run, on the wing, on the fly"
Index: portlint.pl
===================================================================
RCS file: /home/ncvs/ports/devel/portlint/src/portlint.pl,v
retrieving revision 1.24
diff -u -u -r1.24 portlint.pl
--- portlint.pl 2000/12/12 14:24:12 1.24
+++ portlint.pl 2000/12/12 20:34:06
@@ -22,6 +22,7 @@
use vars qw/ $opt_a $opt_b $opt_c $opt_h $opt_t $opt_v $opt_M $opt_N $opt_B $opt_V /;
use Getopt::Std;
+use File::Find;
use IPC::Open2;
#use strict;
@@ -97,7 +98,7 @@
}
-getopts('abchtvBM:N:V');
+getopts('abchtvB:M:NV');
&usage if $opt_h;
&version if $opt_V;
@@ -274,26 +275,49 @@
} else {
my $proc = $checker{$i};
&$proc($i) || &perror("Cannot open the file $i\n");
- if ($i !~ /^files\/patch-/) {
+ if ($proc ne 'checkpatch') {
&checklastline($i)
|| &perror("Cannot open the file $i\n");
}
}
}
if ($committer) {
- if (scalar(@_ = <work/*>) || -d "work") {
- &perror("FATAL: be sure to cleanup $portdir/work ".
- "before committing the port.");
- }
- if (scalar(@_ = <*/*~>) || scalar(@_ = <*~>)) {
- &perror("FATAL: for safety, be sure to cleanup ".
- "editor backup files before committing the port.");
- }
- if (scalar(@_ = <*/*.orig>) || scalar(@_ = </*.orig>)
- || scalar(@_ = <*/*.rej>) || scalar(@_ = <*.rej>)) {
- &perror("FATAL: for safety, be sure to cleanup ".
- "patch backup files before committing the port.");
- }
+ sub find_proc {
+ return if /^\.\.?$/;
+
+ (my $fullname = $File::Find::name) =~ s#^\./##;
+
+ print "OK: checking the file name of $fullname.\n" if ($verbose);
+
+ if ($fullname eq 'work') {
+ &perror("FATAL: $fullname: be sure to cleanup the working directory ".
+ "before committing the port.");
+
+ $File::Find::prune = 1;
+ } elsif (-l) {
+ &perror("Warning: $fullname: this is a symlink. ".
+ "CVS will ignore it.");
+ } elsif (/^\./) {
+ &perror("Warning: $fullname: dotfiles are not preferred. ".
+ "If this file is a sample dotfile to be installed, ".
+ "consider importing it as \"dot$_\".");
+ } elsif (/\.(orig|rej|bak)$/ || /~$/ || /^\#/) {
+ &perror("FATAL: $fullname: for safety, be sure to cleanup ".
+ "backup files before committing the port.");
+ } elsif (/(^|\.)core$/) {
+ &perror("FATAL: $fullname: for safety, be sure to cleanup ".
+ "core files before committing the port.");
+ } elsif ($_ eq 'CVS' && -d) {
+ if ($newport) {
+ &perror("FATAL: $fullname: for safety, be sure to cleanup ".
+ "CVS directories before importing the new port.");
+ }
+
+ $File::Find::prune = 1;
+ }
+ }
+
+ find(\&find_proc, '.');
}
if ($err || $warn) {
print "$err fatal errors and $warn warnings found.\n"
@@ -898,7 +922,7 @@
#
# section 2: PORTNAME/PORTVERSION/...
#
- print "OK: checking first section of $file. (PORTNAME/...)\n"
+ print "OK: checking first section of $file (PORTNAME/...).\n"
if ($verbose);
$tmp = $sections[$idx++];
@@ -1122,7 +1146,7 @@
#
# section 3: PATCH_SITES/PATCHFILES(optional)
#
- print "OK: checking second section of $file, (PATCH*: optinal).\n"
+ print "OK: checking second section of $file (PATCH*: optinal).\n"
if ($verbose);
$tmp = $sections[$idx];
@@ -1178,7 +1202,7 @@
#
# section 5: *_DEPENDS (may not be there)
#
- print "OK: checking fourth section of $file(*_DEPENDS).\n"
+ print "OK: checking fourth section of $file (*_DEPENDS).\n"
if ($verbose);
$tmp = $sections[$idx];
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-ports" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?868zplwe1b.wl>