From 2014283bb112cd20ffab0c8180b1974ed8c3110d Mon Sep 17 00:00:00 2001 From: "B. Watson" Date: Sat, 4 Jan 2020 12:37:54 -0500 Subject: sbolint: major update, see changelog comments --- sbolint | 155 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file 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 to make HTTP HEAD requests for the B, B, and B 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, B, or B). 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(/[^#]*/) { + if(/^[^#]*/) { log_error("$file:$lineno: copy actual documentation, not "); } @@ -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($_); + } +} -- cgit v1.2.3