pkgsrc-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[pkgsrc/trunk]: pkgsrc/pkgtools/pkglint/files - Improved the heuristics for d...



details:   https://anonhg.NetBSD.org/pkgsrc/rev/cf68d6064203
branches:  trunk
changeset: 504175:cf68d6064203
user:      rillig <rillig%pkgsrc.org@localhost>
date:      Tue Dec 06 15:22:28 2005 +0000

description:
- Improved the heuristics for detecting direct use of ${TOOL}s.
- Moved the whole complicated code from checklines_package_Makefile() to
  checkline_mk_shellcmd() and checkline_mk_varassign().
- Reenabled the -Wdirectcmd options by default.

diffstat:

 pkgtools/pkglint/files/pkglint.pl |  313 +++++++++++++++++++------------------
 1 files changed, 158 insertions(+), 155 deletions(-)

diffs (truncated from 375 to 300 lines):

diff -r 5aa662931757 -r cf68d6064203 pkgtools/pkglint/files/pkglint.pl
--- a/pkgtools/pkglint/files/pkglint.pl Tue Dec 06 15:20:31 2005 +0000
+++ b/pkgtools/pkglint/files/pkglint.pl Tue Dec 06 15:22:28 2005 +0000
@@ -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.422 2005/12/06 14:15:02 rillig Exp $
+# $NetBSD: pkglint.pl,v 1.423 2005/12/06 15:22:28 rillig Exp $
 #
 # This version contains lots of changes necessary for NetBSD packages
 # done by:
@@ -666,7 +666,7 @@
 );
 
 my $opt_warn_absname   = true;
-my $opt_warn_directcmd = false;
+my $opt_warn_directcmd = true;
 my $opt_warn_order     = true;
 my $opt_warn_plist_sort        = false;
 my $opt_warn_types     = true;
@@ -1333,7 +1333,7 @@
                        $includefile = resolve_relative_path($1);
                        if ($includefile =~ regex_unresolved) {
                                if ($file !~ qr"/mk/") {
-                                       $line->log_warning("Skipping include file \"${includefile}\". This may result in false warnings.");
+                                       $line->log_note("Skipping include file \"${includefile}\". This may result in false warnings.");
                                }
 
                        } elsif (exists($seen_Makefile_include->{$includefile})) {
@@ -1538,20 +1538,122 @@
        }
 }
 
-sub checkline_mk_dollar($$) {
+sub checkline_mk_text($$) {
        my ($line, $text) = @_;
 
        if ($text =~ qr"^[^#]*[^\$]\$(\w+)") {
                my ($varname) = ($1);
                $line->log_warning("\$$varname is ambiguous. Use \${$varname} if you mean a Makefile variable or \$\$$varname if you mean a shell variable.");
        }
+
+       if ($line->lines eq "1") {
+               checkline_rcsid_regex($line, qr"#\s+", "# ");
+       }
+
+       checkline_trailing_whitespace($line);
+
+       if ($text =~ qr"\$\{WRKSRC\}/\.\./") {
+               $line->log_error("Using \"\${WRKSRC}/..\" is conceptually wrong. Use a combination of WRKSRC, CONFIGURE_DIRS and BUILD_DIRS instead.");
+       }
 }
 
 sub checkline_mk_shellcmd($$) {
        my ($line, $shellcmd) = @_;
-
-       checkline_mk_direct_tool_use($line, $shellcmd, "in shell command");
-       checkline_mk_dollar($line, $shellcmd);
+       my ($rest, $state, $vartools);
+
+       use constant SCST_START         =>  0;
+       use constant SCST_CONT          =>  1;
+       use constant SCST_INSTALL       => 10;
+       use constant SCST_INSTALL_D     => 11;
+       use constant SCST_MKDIR         => 20;
+       use constant SCST_PAX           => 30;
+       use constant SCST_PAX_S         => 31;
+       use constant SCST_SED           => 40;
+       use constant SCST_SED_E         => 41;
+
+       checkline_mk_text($line, $shellcmd);
+
+       if ($shellcmd =~ qr"^\@*-(.*(MKDIR|INSTALL.*-d|INSTALL_.*_DIR).*)") {
+               my ($mkdir_cmd) = ($1);
+
+               $line->log_note("You don't need to use \"-\" before ${mkdir_cmd}.");
+       }
+
+       $vartools = get_vartool_names();
+       $rest = $shellcmd;
+       $state = SCST_START;
+       while ($rest =~ s/^$regex_shellword//) {
+               my ($shellword) = ($1);
+
+               #
+               # Actions associated with the current state
+               # and the symbol on the "input tape".
+               #
+
+               if ($state == SCST_START && exists($vartools->{$shellword})) {
+                       $line->log_warning("Possible direct use of tool \"${shellword}\". Please use \$\{$vartools->{$shellword}\} instead.");
+               }
+
+               if (($state != SCST_PAX_S && $state != SCST_SED_E) && $shellword =~ qr"^/" && $shellword ne "/dev/null") {
+                       $line->log_warning("Found absolute pathname: ${shellword}");
+                       $line->explain(
+                               "Absolute pathnames are often an indicator for unportable code. As",
+                               "pkgsrc aims to be a portable system, absolute pathnames should be",
+                               "avoided whenever possible.");
+               }
+
+               if (($state == SCST_INSTALL_D || $state == SCST_MKDIR) && $shellword =~ qr"^\$\{PREFIX\}/") {
+                       $line->log_warning("Please use one of the INSTALL_*_DIR commands instead of "
+                               . (($state == SCST_MKDIR) ? "\${MKDIR}" : "\${INSTALL} -d")
+                               . ".");
+                       $line->explain(
+                               "Choose one of INSTALL_PROGRAM_DIR, INSTALL_SCRIPT_DIR, INSTALL_LIB_DIR,",
+                               "INSTALL_MAN_DIR, INSTALL_DATA_DIR.");
+               }
+
+               #
+               # State transition.
+               #
+
+               if ($shellword =~ qr"^[;&\|]+$") {
+                       $state = SCST_START;
+
+               } elsif ($state == SCST_START) {
+                       if ($shellword eq "\${INSTALL}") {
+                               $state = SCST_INSTALL;
+                       } elsif ($shellword eq "\${MKDIR}") {
+                               $state = SCST_MKDIR;
+                       } elsif ($shellword eq "\${PAX}") {
+                               $state = SCST_PAX;
+                       } elsif ($shellword eq "\${SED}") {
+                               $state = SCST_SED;
+                       } else {
+                               $state = SCST_CONT;
+                       }
+
+               } elsif ($state == SCST_INSTALL && $shellword eq "-d") {
+                       $state = SCST_INSTALL_D;
+
+               } elsif (($state == SCST_INSTALL || $state == SCST_INSTALL_D) && $shellword =~ qr"^-[ogm]$") {
+                       $state = SCST_START;
+
+               } elsif ($state == SCST_PAX && $shellword eq "-s") {
+                       $state = SCST_PAX_S;
+
+               } elsif ($state == SCST_PAX_S) {
+                       $state = SCST_PAX;
+
+               } elsif ($state == SCST_SED && $shellword eq "-e") {
+                       $state = SCST_SED_E;
+
+               } elsif ($state == SCST_SED_E) {
+                       $state = SCST_SED;
+               }
+       }
+
+       if ($rest !~ qr"^\s*$") {
+               $line->log_warning("Invalid shell word \"${shellcmd}\".");
+       }
 }
 
 sub checkline_mk_varassign($$$$$) {
@@ -1574,8 +1676,54 @@
        if (!exists(direct_tools_ok_vars->{$varbase}) && !exists(direct_tools_ok_vars->{$varname})) {
                checkline_mk_direct_tool_use($line, $value, "in variable ${varname}");
        }
-       checkline_mk_dollar($line, $value);
+       checkline_mk_text($line, $value);
        checkline_mk_vartype($line, $varname, $op, $value, $comment);
+
+                       
+       if ($varname =~ qr"^_") {
+               $line->log_error("Variable names starting with an underscore are reserved for internal pkgsrc use.");
+       }
+
+       if ($varname eq "PERL5_PACKLIST" && defined($pkgname) && $pkgname !~ regex_unresolved && $pkgname =~ qr"^p5-(.*)-[0-9].*") {
+               my ($guess) = ($1);
+               $guess =~ s/-/\//g;
+               $guess = "auto/${guess}/.packlist";
+
+               my ($ucvalue, $ucguess) = (uc($value), uc($guess));
+               if ($ucvalue ne $ucguess && $ucvalue ne "\${PERL5_SITEARCH\}/${ucguess}") {
+                       $line->log_warning("Unusual value for PERL5_PACKLIST -- \"${guess}\" expected.");
+               }
+       }
+
+       if ($varname eq "SVR4_PKGNAME") {
+               if ($value =~ regex_unresolved) {
+                       $line->log_error("SVR4_PKGNAME must not contain references to other variables.");
+               } elsif (length($value) > 5) {
+                       $line->log_error("SVR4_PKGNAME must not be longer than 5 characters.");
+               }
+       }
+
+       if (defined($comment) && $comment eq "# defined" && $varname !~ qr".*(?:_MK|_COMMON)$") {
+               $line->log_warning("Please use \"# empty\", \"# none\" or \"yes\" instead of \"# defined\".");
+       }
+
+       if ($varname =~ qr"^NO_(.*)_ON_(.*)$") {
+               my ($what, $where) = ($1, $2);
+               if (($what ne "SRC" && $what ne "BIN") || ($where ne "FTP" && $where ne "CDROM")) {
+                       $line->log_error("Misspelled variable: Valid names are USE_{BIN,SRC}_ON_{FTP,CDROM}.");
+               }
+       }
+
+       if ($value =~ qr"\$\{(PKGNAME|PKGVERSION)[:\}]") {
+               my ($pkgvarname) = ($1);
+               if ($varname =~ qr"^PKG_.*_REASON$") {
+                       # ok
+               } elsif ($varname =~ qr"^(?:DIST_SUBDIR|WRKSRC)$") {
+                       $line->log_warning("${pkgvarname} should not be used in ${varname}, as it sometimes includes the PKGREVISION. Please use ${pkgvarname}_NOREV instead.");
+               } else {
+                       $line->log_info("Use of PKGNAME in ${varname}.");
+               }
+       }
 }
 
 sub checkline_mk_vartype_basic($$$$$) {
@@ -2253,159 +2401,14 @@
        foreach my $line (@{$lines}) {
                my $text = $line->text;
 
-               if ($line->lines eq "1") {
-                       checkline_rcsid_regex($line, qr"#\s+", "# ");
-               }
-
-               checkline_trailing_whitespace($line);
-
-               if ($text =~ /^\040{8}/) {
-                       $line->log_warning("Please use tab (not spaces) to make indentation.");
-               }
-
-               if ($text =~ qr"\$\{WRKSRC\}/\.\./") {
-                       $line->log_error("Using \"\${WRKSRC}/..\" is conceptually wrong. Use a combination of WRKSRC, CONFIGURE_DIRS and BUILD_DIRS instead.");
-               }
-
                if ($text =~ qr"^\s*$" || $text =~ qr"^#") {
                        # Ignore empty lines and comments
 
                } elsif ($text =~ regex_varassign) {
-                       my ($varname, $op, $value, $comment) = ($1, $2, $3, $4);
-
-                       if ($varname =~ qr"^_") {
-                               $line->log_error("Variable names starting with an underscore are reserved for internal pkgsrc use.");
-                       }
-
-                       if ($varname eq "PERL5_PACKLIST" && defined($pkgname) && $pkgname !~ regex_unresolved && $pkgname =~ qr"^p5-(.*)-[0-9].*") {
-                               my ($guess) = ($1);
-                               $guess =~ s/-/\//g;
-                               $guess = "auto/${guess}/.packlist";
-
-                               my ($ucvalue, $ucguess) = (uc($value), uc($guess));
-                               if ($ucvalue ne $ucguess && $ucvalue ne "\${PERL5_SITEARCH\}/${ucguess}") {
-                                       $line->log_warning("Unusual value for PERL5_PACKLIST -- \"${guess}\" expected.");
-                               }
-                       }
-
-                       if ($varname eq "SVR4_PKGNAME") {
-                               if ($value =~ regex_unresolved) {
-                                       $line->log_error("SVR4_PKGNAME must not contain references to other variables.");
-                               } elsif (length($value) > 5) {
-                                       $line->log_error("SVR4_PKGNAME must not be longer than 5 characters.");
-                               }
-                       }
-
-                       if (defined($comment) && $comment eq "# defined" && $varname !~ qr".*(?:_MK|_COMMON)$") {
-                               $line->log_warning("Please use \"# empty\", \"# none\" or \"yes\" instead of \"# defined\".");
-                       }
-
-                       if ($varname =~ qr"^NO_(.*)_ON_(.*)$") {
-                               my ($what, $where) = ($1, $2);
-                               if (($what ne "SRC" && $what ne "BIN") || ($where ne "FTP" && $where ne "CDROM")) {
-                                       $line->log_error("Misspelled variable: Valid names are USE_{BIN,SRC}_ON_{FTP,CDROM}.");
-                               }
-                       }
-
-                       if ($value =~ qr"\$\{(PKGNAME|PKGVERSION)[:\}]") {
-                               my ($pkgvarname) = ($1);
-                               if ($varname =~ qr"^PKG_.*_REASON$") {
-                                       # ok
-                               } elsif ($varname =~ qr"^(?:DIST_SUBDIR|WRKSRC)$") {
-                                       $line->log_warning("${pkgvarname} should not be used in ${varname}, as it sometimes includes the PKGREVISION. Please use ${pkgvarname}_NOREV instead.");
-                               } else {
-                                       $line->log_info("Use of PKGNAME in ${varname}.");
-                               }
-                       }
+                       # Is already checked by checklines_mk().
 
                } elsif ($text =~ regex_shellcmd) {
-                       my ($shellcmd) = ($1);
-                       my ($rest, $state);
-
-                       use constant SCST_START         =>  0;
-                       use constant SCST_INSTALL       => 10;
-                       use constant SCST_INSTALL_D     => 11;
-                       use constant SCST_MKDIR         => 20;
-                       use constant SCST_PAX           => 30;
-                       use constant SCST_PAX_S         => 31;
-                       use constant SCST_SED           => 40;
-                       use constant SCST_SED_E         => 41;
-
-                       if ($shellcmd =~ qr"^\@*-(.*(MKDIR|INSTALL.*-d|INSTALL_.*_DIR).*)") {
-                               my ($mkdir_cmd) = ($1);
-
-                               $line->log_note("You don't need to use \"-\" before ${mkdir_cmd}.");



Home | Main Index | Thread Index | Old Index