sbolint: major update, see changelog comments

1 files changed, 135 insertions, 20 deletions

diff --git a/sbolint b/sbolint
index a4c8428..6c8f5b7 100755
--- a/sbolint
+++ b/sbolint
@@ -1,11 +1,28 @@
 #!/usr/bin/perl -w
 
-# sbolint, 20141114 bkw
-
-$VERSION="0.1";
+# ChangeLog:
+
+# 0.2 20200103 bkw:
+# - Use "git rev-parse" to decide if we're in a git repo, because
+#   "git status" traverses the whole repo looking for untracked files.
+#   It does this even if you use -uno (it won't *print* the untracked
+#   files, but it still searches for them). Thanks to alienBOB for cluing
+#   me in to using rev-parse for this.
+# - Skip the junkfiles check when we're in a git repo. It's more
+#   annoying than it is useful.
+# - Allow possible -e/-u arguments in the shebang.
+# - Avoid false positives when the script does a "cd $PKG" and then
+#   uses relative paths for install/*.
+# - Require VERSION= to appear within the first 10 non-comment/non-blank
+#   lines, and don't check it anywhere else in the script.
+# - Allow scripts to skip lint checks via ###sbolint on/off comments.
+
+# 0.1 20141114 bkw, Initial release.
+
+$VERSION="0.2";
 
 # generate man page with:
-# pod2man --stderr -r0.1 -s1 -c"SBo Maintainer Tools" sbolint > sbolint.1
+# pod2man --stderr -r0.2 -s1 -c"SBo Maintainer Tools" sbolint > sbolint.1
 
 # This script is meant to be fairly self-contained, prefer not to
 # require a huge pile of perl module dependencies. In some cases this
@@ -72,7 +89,10 @@ Quiet. Suppresses 'xxx checks out OK' and the total errors/warnings summary.
 
 URL check. Uses B<curl> to make HTTP HEAD requests for the B<HOMEPAGE>,
 B<DOWNLOAD>, and B<DOWNLOAD_x86_64> links. This won't guarantee that
-the links are good, but failure means they're definitely bad.
+the links are good, but some kinds of failure (e.g. site down, 404)
+means they're definitely bad. Unfortunately a lot of sites have stopped
+responding to HEAD requests in the name of "security", so your mileage
+man vary.
 
 =item B<-n>
 
@@ -188,6 +208,13 @@ must begin with B<ftp://>, B<http://>, or B<https://>).
 Optionally, .info file URLs can be checked for existence with an HTTP HEAD
 request (see the B<-u> option).
 
+=back
+
+The following tests are only done when sbolint's starting directory
+was NOT in a git repo:
+
+=over 4
+
 =item -
 
 Any files other than the .SlackBuild, .info, slack-desc, and README are
@@ -205,6 +232,10 @@ tool creates these.
 
 =back
 
+The rationale for skipping the above tests when in a git repo is that
+maintainers will be using git to track files and push changes, so we
+don't need to check them here.
+
 =head1 EXIT STATUS
 
 Exit status from sbolint will normally be 0 (success) if there were no
@@ -299,6 +330,10 @@ if($stdin) {
 
 push @ARGV, "." unless @ARGV;
 
+# are we in a git repo? build scripts are mode 0644 there, plus
+# the junkfile check is skipped.
+$in_git_repo = system("git rev-parse >/dev/null 2>/dev/null") == 0;
+
 for(@ARGV) {
 	run_checks($_);
 	$g_errcount += $errcount;
@@ -526,9 +561,13 @@ sub run_checks {
 		\&check_slackdesc,
 		\&check_info,
 		\&check_script,
-		\&check_junkfiles,
+		\&check_images,
 	);
 
+	# if we're in a git repo, it's assumed we're going to track extra
+	# files with git, and use git to update the build, not tar it up
+	# and use the web form.
+	push @checks, \&check_junkfiles unless $in_git_repo;
 	for(@checks) {
 		$_->($build);
 	}
@@ -895,33 +934,56 @@ sub curl_head_request {
 
 sub check_script {
 	my $file = $buildname . ".SlackBuild";
-	my $wantmode = 0755;
-
-	# are we in a git repo? build scripts are mode 0644 there.
-	my $got = system("git status >/dev/null 2>/dev/null");
-	if($got == 0) {
-		$wantmode = 0644;
-	}
+	my $wantmode = $in_git_repo ? 0644 : 0755;
 
 	my @lines = check_and_read($file, $wantmode);
 	return unless scalar @lines;
 
-	if($lines[0] !~ /^#/) {
+	if($lines[0] !~ /^#!/) {
 		log_error("$file:1: missing or invalid shebang line (should be '#!/bin/sh')");
-	} elsif($lines[0] ne "#!/bin/sh") {
-		log_warning("$file:1: shebang line should be #!/bin/sh (admins always change it to that anyway)");
+	} elsif($lines[0] !~ m,#!/bin/sh(?: (?:-e|-eu|-ue|-e -u|-u -e))?$,) {
+		log_warning("$file:1: shebang line should be #!/bin/sh (possibly with -e/-u arg(s)), not '$lines[0]'");
 	}
 
 	my $lineno = 0;
 	my ($prgnam, $version, $build, $tag, $need_doinst, $slackdesc, $makepkg, $install);
+	my ($cdpkg, $codestart, $lint_enabled);
+	$lint_enabled = 1;
+
 	for(@lines) {
 		$lineno++;
+
+		if(/^\s*[^#]/ && !defined($codestart)) {
+			$codestart = $lineno;
+		}
+
+		if(/^###sbolint\s*(\S+)/) {
+			my $arg = $1;
+			if(lc($arg) eq "on") {
+				$lint_enabled = 1;
+			} elsif(lc($arg) eq "off") {
+				$lint_enabled = 0;
+			} else {
+				log_warning("$file:$lineno: unknown ###sbolint argument '$arg' (should be 'on' or 'off')");
+			}
+		}
+
+		next unless $lint_enabled;
+
+		# TODO: cp without -a (or -p, or a couple other flags) is OK.
+##		if(/^[^#]*cp\s+(?:-\w+\s+)*[\"\$\{]*CWD/) {
+##			log_error("$file:$lineno: copying files from CWD with cp (use cat instead)");
+##		}
+
 		if(/^PRGNAM=(\S+)/) {
+			if($prgnam) {
+				log_error("$file:$lineno: PRGNAM redefined");
+			}
 			$prgnam = dequote($1);
 			if($prgnam ne $buildname) {
 				log_error("$file:$lineno: PRGNAM doesn't match dir name ($prgnam != $buildname)");
 			}
-		} elsif(/^VERSION=(\S+)/) {
+		} elsif(/^VERSION=(\S+)/ && ($lineno <= $codestart + 10)) {
 			$version = dequote($1);
 			if(not ($version =~ s/\$\{VERSION:-([^}]+)\}/$1/)) {
 				log_warning("$file:$lineno: VERSION ignores environment, try VERSION=\${VERSION:-$version}");
@@ -942,17 +1004,25 @@ sub check_script {
 			if($tag !~ /\$\{TAG:-(?:_SBo|("|')_SBo(\1))\}/) {
 				log_error("$file:$lineno: TAG=\${TAG:-_SBo} is required");
 			}
-		} elsif(/^[^#]*\s+\$CWD\/doinst.sh/) {
+		} elsif(/^[^#]*\$\S*CWD\S*\/doinst.sh/) {
 			$need_doinst = $lineno;
 		} elsif(/^[^#]*slack-desc/) {
 			$slackdesc = $lineno;
-		} elsif(/^[^#]*\$PKG\/install/) {
+			$install = $lineno if m,install/,; # assume OK
+		} elsif(/^[^#]*?cd\s+[{\$"]*PKG[}"]*/) {
+			$cdpkg = $lineno;
+		} elsif(/^[^#]*?["{\$]+PKG[}"]*\/install/) {
+			$install = $lineno;
+		} elsif($cdpkg && /^[^#]*mkdir[^#]*install/) {
 			$install = $lineno;
 		} elsif(/^[^#]*makepkg/) {
+			if($makepkg) {
+				log_error("$file:$lineno: makepkg called twice (here and line $makepkg");
+			}
 			$makepkg = $lineno;
 		}
 
-		if(/[^#]*<documentation>/) {
+		if(/^[^#]*<documentation>/) {
 			log_error("$file:$lineno: copy actual documentation, not <documentation>");
 		}
 
@@ -1119,3 +1189,48 @@ sub check_patches {
 		check_and_read($_, 0644);
 	}
 }
+
+# checking an image is a bit of a PITA. "file" can tell us if it's
+# not an image, or has the wrong extension.
+# ImageMagick's "identify" command won't detect truncated images.
+# "convert" will, but it always returns 0/success, so we have to
+# parse its output.
+sub im_check_img {
+	our %ext2mime;
+	my $mime;
+	my $ok = 1;
+
+	%ext2mime = (
+			png => 'image/png',
+			jpg => 'image/jpeg',
+			xpm => 'image/x-xpm',
+			gif => 'image/gif',
+	) unless %ext2mime;
+
+	my $img = shift;
+	my $ext = $img;
+	$ext =~ s,.*\.,,;
+	$ext = lc $ext;
+
+	chomp($mime = `file --brief --mime "$img"`);
+	if($mime !~ /$ext2mime{$ext}/) {
+		log_error("$img has wrong extension $ext (MIME type is $mime)");
+		return;
+	}
+
+	open my $im, "convert \"$img\" png:/dev/null 2>&1 |";
+	while(<$im>) {
+		$ok = 0 if /premature|corrupt/i;
+	}
+	close $im;
+
+	log_error("$img appears to be corrupt") unless $ok;
+}
+
+sub check_images {
+	my $images = `find . \\( -iname '*.jpg' -o -iname '*.png' -o -iname '*.xpm' -o -iname '*.gif' \\) -print0`;
+	for(split /\x00/, $images) {
+		check_mode($_, 0644);
+		im_check_img($_);
+	}
+}