pkgsrc-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[pkgsrc/trunk]: pkgsrc/pkgtools/pkglint4/files pkgtools/pkglint4: remove some...
details: https://anonhg.NetBSD.org/pkgsrc/rev/33df4bf299f3
branches: trunk
changeset: 340678:33df4bf299f3
user: rillig <rillig%pkgsrc.org@localhost>
date: Sun Oct 06 11:06:42 2019 +0000
description:
pkgtools/pkglint4: remove some unreliable checks
The warnings about variable permissions were not understandable enough to
be acted upon. The new pkglint does this better.
The languages that are allowed in USE_LANGUAGES are defined differently
in the pkgsrc infrastructure, thus the old parsing code does not work
anymore. Therefore all identifiers are allowed now.
Dependency patterns like 'package>=1.0<2.0' are no longer marked as
wrong.
The debatable warning about plural names is gone.
The order of variables in simple Makefiles is no longer checked. Some new
variables have been added in the meantime, and keeping the consistent
order is not of utmost importance to those pkgsrc developers who work
with pkglint4. They are experienced enough to know the rules.
Missing manual pages are no longer marked in the PLIST files. It's not
the job of pkgsrc to provide these files.
The warning about unnoticed errors in pipelines like 'find | xargs' has
been removed because the shell parser is unreliable. This is solved
better in the new pkglint.
diffstat:
pkgtools/pkglint4/files/PkgLint/Shell.pm | 9 +-
pkgtools/pkglint4/files/makevars.map | 4 +-
pkgtools/pkglint4/files/pkglint.pl | 268 +------------------------------
3 files changed, 4 insertions(+), 277 deletions(-)
diffs (truncated from 366 to 300 lines):
diff -r 50cd6a3e1713 -r 33df4bf299f3 pkgtools/pkglint4/files/PkgLint/Shell.pm
--- a/pkgtools/pkglint4/files/PkgLint/Shell.pm Sun Oct 06 10:46:18 2019 +0000
+++ b/pkgtools/pkglint4/files/PkgLint/Shell.pm Sun Oct 06 11:06:42 2019 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: Shell.pm,v 1.2 2019/10/06 10:33:34 rillig Exp $
+# $NetBSD: Shell.pm,v 1.3 2019/10/06 11:06:42 rillig Exp $
#
# Parsing and checking shell commands embedded in Makefiles
#
@@ -597,13 +597,6 @@
$line->log_warning("Please use \${ECHO_N} instead of \"echo -n\".");
}
- if ($opt_warn_extra && $state != SCST_CASE_LABEL_CONT && $shellword eq "|") {
- $line->log_warning("The exitcode of the left-hand-side command of the pipe operator is ignored.");
- $line->explain_warning(
-"If you need to detect the failure of the left-hand-side command, use",
-"temporary files to save the output of the command.");
- }
-
if ($opt_warn_extra && $shellword eq ";" && $state != SCST_COND_CONT && $state != SCST_FOR_CONT && !$set_e_mode) {
$line->log_warning("Please switch to \"set -e\" mode before using a semicolon to separate commands.");
$line->explain_warning(
diff -r 50cd6a3e1713 -r 33df4bf299f3 pkgtools/pkglint4/files/makevars.map
--- a/pkgtools/pkglint4/files/makevars.map Sun Oct 06 10:46:18 2019 +0000
+++ b/pkgtools/pkglint4/files/makevars.map Sun Oct 06 11:06:42 2019 +0000
@@ -1,4 +1,4 @@
-# $NetBSD: makevars.map,v 1.11 2019/05/21 05:52:31 adam Exp $
+# $NetBSD: makevars.map,v 1.12 2019/10/06 11:06:42 rillig Exp $
#
# This file contains the guessed type of some variables, according to
@@ -741,7 +741,7 @@
USE_IMAKE Yes [m:s]
USE_JAVA { run yes build } [$package]
USE_JAVA2 { YES yes no 1.4 1.5 6 7 8 } [$package]
-USE_LANGUAGES List of { ada c c99 c++ fortran fortran77 java objc } [m:s,c:s,o:s]
+USE_LANGUAGES List of Identifier [m:s,c:s,o:s]
USE_LIBTOOL Yes [$package]
USE_MAKEINFO Yes [$package]
USE_MSGFMT_PLURALS Yes [$package]
diff -r 50cd6a3e1713 -r 33df4bf299f3 pkgtools/pkglint4/files/pkglint.pl
--- a/pkgtools/pkglint4/files/pkglint.pl Sun Oct 06 10:46:18 2019 +0000
+++ b/pkgtools/pkglint4/files/pkglint.pl Sun Oct 06 11:06:42 2019 +0000
@@ -1,5 +1,5 @@
#! @PERL@
-# $NetBSD: pkglint.pl,v 1.8 2019/10/06 10:46:18 rillig Exp $
+# $NetBSD: pkglint.pl,v 1.9 2019/10/06 11:06:42 rillig Exp $
#
# pkglint - static analyzer and checker for pkgsrc packages
@@ -2404,57 +2404,6 @@
$opt_warn_extra and $line->log_warning("${varname} is used but not defined. Spelling mistake?");
}
- if ($opt_warn_perm) {
- my $perms = get_variable_perms($line, $varname);
- my $is_load_time; # Will the variable be used at load time?
- my $is_indirect; # Might the variable be used indirectly at load time,
- # for example by assigning it to another variable
- # which then gets evaluated?
-
- # Don't warn about variables that are not recorded in the
- # pkglint variable definition.
- if (defined($context->type) && $context->type->is_guessed()) {
- $is_load_time = false;
-
- } elsif ($context->time == VUC_TIME_LOAD && $perms !~ m"p") {
- $is_load_time = true;
- $is_indirect = false;
-
- } elsif (defined($context->type) && $context->type->perms_union() =~ m"p" && $perms !~ m"p") {
- $is_load_time = true;
- $is_indirect = true;
-
- } else {
- $is_load_time = false;
- }
-
- if ($is_load_time && !$is_indirect) {
- $line->log_warning("${varname} should not be evaluated at load time.");
- $line->explain_warning(
-"Many variables, especially lists of something, get their values",
-"incrementally. Therefore it is generally unsafe to rely on their value",
-"until it is clear that it will never change again. This point is",
-"reached when the whole package Makefile is loaded and execution of the",
-"shell commands starts, in some cases earlier.",
-"",
-"Additionally, when using the \":=\" operator, each \$\$ is replaced",
-"with a single \$, so variables that have references to shell variables",
-"or regular expressions are modified in a subtle way.");
- }
- if ($is_load_time && $is_indirect) {
- $line->log_warning("${varname} should not be evaluated indirectly at load time.");
- $line->explain_warning(
-"The variable on the left-hand side may be evaluated at load time, but",
-"the variable on the right-hand side may not. Due to this assignment, it",
-"might be used indirectly at load-time, when it is not guaranteed to be",
-"properly defined.");
- }
-
- if ($perms !~ m"p" && $perms !~ m"u") {
- $line->log_warning("${varname} may not be used in this file.");
- }
- }
-
if ($varname eq "LOCALBASE" && !$is_internal) {
$line->log_warning("The LOCALBASE variable should not be used by packages.");
$line->explain_warning(
@@ -2655,24 +2604,6 @@
if (!exists($mkctx_vardef->{$varname})) {
$mkctx_vardef->{$varname} = $line;
}
-
- return unless $opt_warn_perm;
-
- my $perms = get_variable_perms($line, $varname);
- my $needed = { "=" => "s", "!=" => "s", "?=" => "d", "+=" => "a", ":=" => "s" }->{$op};
- if (index($perms, $needed) == -1) {
- $line->log_warning("Permission [" . expand_permission($needed) . "] requested for ${varname}, but only [" . expand_permission($perms) . "] is allowed.");
- $line->explain_warning(
-"The available permissions are:",
-"\tappend\t\tappend something using +=",
-"\tdefault\t\tset a default value using ?=",
-"\tpreprocess\tuse a variable during preprocessing",
-"\truntime\t\tuse a variable at runtime",
-"\tset\t\tset a variable using :=, =, !=",
-"",
-"A \"?\" means that it is not yet clear which permissions are allowed",
-"and which aren't.");
- }
}
# @param $op
@@ -2867,20 +2798,6 @@
} else {
$line->log_error("Unknown dependency pattern \"${value}\".");
}
-
- } elsif ($value =~ m"\{") {
- # Dependency patterns containing alternatives
- # are just too hard to check.
- $opt_debug_unchecked and $line->log_debug("Unchecked dependency pattern: ${value}");
-
- } elsif ($value ne $value_novar) {
- $opt_debug_unchecked and $line->log_debug("Unchecked dependency: ${value}");
-
- } else {
- $line->log_warning("Unknown dependency format: ${value}");
- $line->explain_warning(
-"Typical dependencies have the form \"package>=2.5\", \"package-[0-9]*\"",
-"or \"package-3.141\".");
}
},
@@ -3574,8 +3491,6 @@
if (!$type->may_use_plus_eq()) {
$line->log_warning("The \"+=\" operator should only be used with lists.");
}
- } elsif ($varbase !~ m"^_" && $varbase !~ get_regex_plurals()) {
- $line->log_warning("As ${varname} is modified using \"+=\", its name should indicate plural.");
}
}
@@ -3834,171 +3749,6 @@
}
}
-sub checklines_package_Makefile_varorder($) {
- my ($lines) = @_;
-
- return unless $opt_warn_varorder;
-
- use enum qw(once optional many);
- my (@sections) = (
- [ "Initial comments", once,
- [
- ]
- ],
- [ "Unsorted stuff, part 1", once,
- [
- [ "DISTNAME", optional ],
- [ "PKGNAME", optional ],
- [ "PKGREVISION", optional ],
- [ "CATEGORIES", once ],
- [ "MASTER_SITES", optional ],
- [ "DIST_SUBDIR", optional ],
- [ "EXTRACT_SUFX", optional ],
- [ "DISTFILES", many ],
- [ "SITES.*", many ],
- ]
- ],
- [ "Distribution patches", optional,
- [
- [ "PATCH_SITES", optional ], # or once?
- [ "PATCH_SITE_SUBDIR", optional ],
- [ "PATCHFILES", optional ], # or once?
- [ "PATCH_DIST_ARGS", optional ],
- [ "PATCH_DIST_STRIP", optional ],
- [ "PATCH_DIST_CAT", optional ],
- ]
- ],
- [ "Unsorted stuff, part 2", once,
- [
- [ "MAINTAINER", optional ],
- [ "OWNER", optional ],
- [ "HOMEPAGE", optional ],
- [ "COMMENT", once ],
- [ "LICENSE", once ],
- ]
- ],
- [ "Legal issues", optional,
- [
- [ "LICENSE_FILE", optional ],
- [ "RESTRICTED", optional ],
- [ "NO_BIN_ON_CDROM", optional ],
- [ "NO_BIN_ON_FTP", optional ],
- [ "NO_SRC_ON_CDROM", optional ],
- [ "NO_SRC_ON_FTP", optional ],
- ]
- ],
- [ "Technical restrictions", optional,
- [
- [ "BROKEN_EXCEPT_ON_PLATFORM", many ],
- [ "BROKEN_ON_PLATFORM", many ],
- [ "NOT_FOR_PLATFORM", many ],
- [ "ONLY_FOR_PLATFORM", many ],
- [ "NOT_FOR_COMPILER", many ],
- [ "ONLY_FOR_COMPILER", many ],
- [ "NOT_FOR_UNPRIVILEGED", optional ],
- [ "ONLY_FOR_UNPRIVILEGED", optional ],
- ]
- ],
- [ "Dependencies", optional,
- [
- [ "BUILD_DEPENDS", many ],
- [ "TOOL_DEPENDS", many ],
- [ "DEPENDS", many ],
- ]
- ]
- );
-
- if (!defined($seen_Makefile_common) || $seen_Makefile_common) {
- return;
- }
-
- my ($lineno, $sectindex, $varindex) = (0, -1, 0);
- my ($next_section, $vars, $below, $below_what) = (true, undef, {}, undef);
-
- # If the current section is optional but contains non-optional
- # fields, the complete section may be skipped as long as there
- # has not been a non-optional variable.
- my $may_skip_section = false;
-
- # In each iteration, one of the following becomes true:
- # - new.lineno > old.lineno
- # - new.sectindex > old.sectindex
- # - new.sectindex == old.sectindex && new.varindex > old.varindex
- # - new.next_section == true && old.next_section == false
- while ($lineno <= $#{$lines}) {
- my $line = $lines->[$lineno];
- my $text = $line->text;
-
- $opt_debug_misc and $line->log_debug("[varorder] section ${sectindex} variable ${varindex}.");
-
- if ($next_section) {
- $next_section = false;
- $sectindex++;
- last if ($sectindex > $#sections);
- $vars = $sections[$sectindex]->[2];
- $may_skip_section = ($sections[$sectindex]->[1] == optional);
- $varindex = 0;
- }
-
- if ($text =~ m"^#") {
- $lineno++;
-
- } elsif ($line->has("varcanon")) {
- my $varcanon = $line->get("varcanon");
-
- if (exists($below->{$varcanon})) {
- if (defined($below->{$varcanon})) {
- $line->log_warning("${varcanon} appears too late. Please put it below $below->{$varcanon}.");
- } else {
- $line->log_warning("${varcanon} appears too late. It should be the very first definition.");
- }
- $lineno++;
- next;
- }
-
- while ($varindex <= $#{$vars} && $varcanon ne $vars->[$varindex]->[0] && ($vars->[$varindex]->[1] != once || $may_skip_section)) {
- if ($vars->[$varindex]->[1] == once) {
- $may_skip_section = false;
- }
- $below->{$vars->[$varindex]->[0]} = $below_what;
- $varindex++;
- }
- if ($varindex > $#{$vars}) {
- if ($sections[$sectindex]->[1] != optional) {
- $line->log_warning("Empty line expected.");
Home |
Main Index |
Thread Index |
Old Index