Date: Wed, 20 Aug 2008 17:43:10 +0400 (MSD) From: Eygene Ryabinkin <rea-fbsd@codelabs.ru> To: FreeBSD-gnats-submit@FreeBSD.org Cc: pgollucci@FreeBSD.org, marcus@FreeBSD.org Subject: ports/126681: [patch] ports-mgmt/portlint: chase wrong *_DEPENDS statements in more cases Message-ID: <20080820134310.B89AB1AF41C@void.codelabs.ru> Resent-Message-ID: <200808201350.m7KDo3Xe051564@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 126681 >Category: ports >Synopsis: [patch] ports-mgmt/portlint: chase wrong *_DEPENDS statements in more cases >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-ports-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Wed Aug 20 13:50:03 UTC 2008 >Closed-Date: >Last-Modified: >Originator: Eygene Ryabinkin >Release: FreeBSD 7.0-STABLE i386 >Organization: Code Labs >Environment: System: FreeBSD XXX 7.0-STABLE FreeBSD 7.0-STABLE #18: Wed Aug 13 11:50:11 MSD 2008 root@XXX:/usr/obj/usr/src/sys/XXX i386 >Description: portlint is very good in chasing errorneous *_DEPENDS statements in the fifth section of port Makefiles, but it does not handle the cases where *_DEPENDS lines were generated basing on the WITH_/WITHOUT_ values (and possibly other cases). I had just seen it during the conversation with Philip Gollucci on my initial patch in ports/126672. >How-To-Repeat: Try apply the patch http://www.freebsd.org/cgi/query-pr.cgi?prp=126672-1-txt&n=/viewvc-correct-dependency-path.diff to the revision 1.31 of ports/devel/viewvc/Makefile and spawn portlint. It won't notice that RUN_DEPENDS includes ${PREFIX}. >Fix: The patch below fixes the things and works for me. It certainly needs some more testing -- may be I had overlooked something. --- portlint-chase_late_DEPENDS.diff begins here --- Sometimes *_DEPENDS line(s) can appear after the fifth section. For example, when port uses OPTIONS, it tends to add some dependencies after the inclusion of bsd.port.pre.mk basing on the WITH_/WITHOUT_ values as DESCRIBED in the Porter's Handbook, http://www.freebsd.org/doc/en/books/porters-handbook/makefile-options.html#AEN2435 This patch does two things. - It makes *_DEPENDS check to be a function check_depends_syntax. - It adds the *_DEPENDS check to the 'rest of the file' checking block. --- /usr/local/bin/portlint.orig 2008-08-20 16:21:02.000000000 +0400 +++ /usr/local/bin/portlint 2008-08-20 16:42:09.000000000 +0400 @@ -1046,6 +1046,155 @@ close(IN); } +sub check_depends_syntax { + die "Wrong arguments" unless defined($_[0]); + my $href = $_[0]; + die "Wrong arguments" unless defined($$href{'tmp'}); + die "Wrong arguments" unless defined($$href{'file'}); + + my $tmp = $$href{'tmp'}; + my $file = $$href{'file'}; + my (%seen_depends, $j); + + if (!defined $ENV{'PORTSDIR'}) { + $ENV{'PORTSDIR'} = $portsdir; + } + foreach my $i (grep(/^(PATCH_|EXTRACT_|LIB_|BUILD_|RUN_|FETCH_)*DEPENDS[?+]?=/, split(/\n/, $tmp))) { + $i =~ s/^((PATCH_|EXTRACT_|LIB_|BUILD_|RUN_|FETCH_)*DEPENDS)[?+]?=[ \t]*//; + $j = $1; + $seen_depends{$j}++; + if ($j ne 'DEPENDS' && + $i =~ /^\${([A-Z_]+DEPENDS)}\s*$/ && + $seen_depends{$1} && + $j ne $1) + { + print "OK: $j refers to $1, skipping checks.\n" + if ($verbose); + next; + } + print "OK: checking ports listed in $j.\n" + if ($verbose); + foreach my $k (split(/\s+/, $i)) { + my @l = split(':', $k); + + print "OK: checking dependency value for $j.\n" + if ($verbose); + if ($k =~ /\${((PATCH_|EXTRACT_|LIB_|BUILD_|RUN_|FETCH_)*DEPENDS)}/) { + &perror("WARN", $file, -1, "do not set $j to $k. ". + "Instead, explicity list out required $j dependencies."); + } + + if (($j ne 'DEPENDS' + && scalar(@l) != 2 && scalar(@l) != 3)) { + &perror("WARN", $file, -1, "wrong dependency value ". + "for $j. $j requires ". + "2 or 3 ". + "colon-separated tuples."); + next; + } + my %m = (); + $m{'dep'} = $l[0]; + $m{'dir'} = $l[1]; + $m{'tgt'} = $l[2]; + print "OK: dep=\"$m{'dep'}\", ". + "dir=\"$m{'dir'}\", tgt=\"$m{'tgt'}\"\n" + if ($verbose); + + # check USE_PERL5 + if ($m{'dep'} =~ /^perl5(\.\d+)?$/) { + &perror("WARN", $file, -1, "dependency to perl5 ". + "listed in $j. consider using ". + "USE_PERL5."); + } + + # check USE_ICONV + if ($m{'dep'} =~ /^(iconv\.\d+)$/) { + &perror("WARN", $file, -1, "dependency to $1 ". + "listed in $j. consider using ". + "USE_ICONV."); + } + + # check USE_GETTEXT + if ($m{'dep'} =~ /^(intl\.\d+)$/) { + &perror("WARN", $file, -1, "dependency to $1 ". + "listed in $j. consider using ". + "USE_GETTEXT."); + } + + # check USE_GMAKE + if ($m{'dep'} =~ /^(gmake|\${GMAKE})$/) { + &perror("WARN", $file, -1, "dependency to $1 ". + "listed in $j. consider using ". + "USE_GMAKE."); + } + + # check USE_QT + if ($m{'dep'} =~ /^(qt\d)+$/) { + &perror("WARN", $file, -1, "dependency to $1 ". + "listed in $j. consider using ". + "USE_QT."); + } + + # check LIBLTDL + if ($m{'dep'} =~ /^(ltdl\.\d)+$/) { + &perror("WARN", $file, -1, "dependency to $1 ". + "listed in $j. consider using ". + "USE_LIBLTDL."); + } + + # check CDRTOOLS + if ($m{'dir'} =~ /(cdrtools|cdrtools-cjk)$/) { + &perror("WARN", $file, -1, "dependency to $1 ". + "listed in $j. consider using ". + "USE_CDRTOOLS."); + } + + # check GHOSTSCRIPT + if ($m{'dep'} eq "gs") { + &perror("WARN", $file, -1, "dependency to gs ". + "listed in $j. consider using ". + "USE_GHOSTSCRIPT(_BUILD|_RUN)."); + } + + # check JAVALIBDIR + if ($m{'dep'} =~ m|share/java/classes|) { + &perror("FATAL", $file, -1, "you should use \${JAVALIBDIR} ". + "in BUILD_DEPENDS/RUN_DEPENDS to define ". + "dependencies on JAR files installed in ". + "\${JAVAJARDIR}"); + } + + # check backslash in LIB_DEPENDS + if ($osname eq 'NetBSD' && $j eq 'LIB_DEPENDS' + && $m{'dep'} =~ /\\\\./) { + &perror("WARN", $file, -1, "use of backslashes in ". + "$j is deprecated."); + } + + # check for PREFIX + if ($m{'dep'} =~ /\${PREFIX}/) { + &perror("FATAL", $file, -1, "\${PREFIX} must not be ". + "contained in *_DEPENDS. ". + "use \${LOCALBASE} or ". + "\${X11BASE} instead."); + } + + # check port dir existence + $k = $m{'dir'}; + $k =~ s/\${PORTSDIR}/$ENV{'PORTSDIR'}/; + $k =~ s/\$[\({]PORTSDIR[\)}]/$ENV{'PORTSDIR'}/; + if (! -d $k) { + &perror("WARN", $file, -1, "no port directory $k ". + "found, even though it is ". + "listed in $j."); + } else { + print "OK: port directory $k found.\n" + if ($verbose); + } + } + } +} + # # Makefile # @@ -2365,145 +2514,10 @@ if ($tmp =~ /^(PATCH_|EXTRACT_|LIB_|BUILD_|RUN_|FETCH_)DEPENDS/m) { &checkearlier($file, $tmp, @varnames); - my %seen_depends; - - if (!defined $ENV{'PORTSDIR'}) { - $ENV{'PORTSDIR'} = $portsdir; - } - foreach my $i (grep(/^(PATCH_|EXTRACT_|LIB_|BUILD_|RUN_|FETCH_)*DEPENDS[?+]?=/, split(/\n/, $tmp))) { - $i =~ s/^((PATCH_|EXTRACT_|LIB_|BUILD_|RUN_|FETCH_)*DEPENDS)[?+]?=[ \t]*//; - $j = $1; - $seen_depends{$j}++; - if ($j ne 'DEPENDS' && - $i =~ /^\${([A-Z_]+DEPENDS)}\s*$/ && - $seen_depends{$1} && - $j ne $1) - { - print "OK: $j refers to $1, skipping checks.\n" - if ($verbose); - next; - } - print "OK: checking ports listed in $j.\n" - if ($verbose); - foreach my $k (split(/\s+/, $i)) { - my @l = split(':', $k); - - print "OK: checking dependency value for $j.\n" - if ($verbose); - if ($k =~ /\${((PATCH_|EXTRACT_|LIB_|BUILD_|RUN_|FETCH_)*DEPENDS)}/) { - &perror("WARN", $file, -1, "do not set $j to $k. ". - "Instead, explicity list out required $j dependencies."); - } - - if (($j ne 'DEPENDS' - && scalar(@l) != 2 && scalar(@l) != 3)) { - &perror("WARN", $file, -1, "wrong dependency value ". - "for $j. $j requires ". - "2 or 3 ". - "colon-separated tuples."); - next; - } - my %m = (); - $m{'dep'} = $l[0]; - $m{'dir'} = $l[1]; - $m{'tgt'} = $l[2]; - print "OK: dep=\"$m{'dep'}\", ". - "dir=\"$m{'dir'}\", tgt=\"$m{'tgt'}\"\n" - if ($verbose); - - # check USE_PERL5 - if ($m{'dep'} =~ /^perl5(\.\d+)?$/) { - &perror("WARN", $file, -1, "dependency to perl5 ". - "listed in $j. consider using ". - "USE_PERL5."); - } - - # check USE_ICONV - if ($m{'dep'} =~ /^(iconv\.\d+)$/) { - &perror("WARN", $file, -1, "dependency to $1 ". - "listed in $j. consider using ". - "USE_ICONV."); - } - - # check USE_GETTEXT - if ($m{'dep'} =~ /^(intl\.\d+)$/) { - &perror("WARN", $file, -1, "dependency to $1 ". - "listed in $j. consider using ". - "USE_GETTEXT."); - } - - # check USE_GMAKE - if ($m{'dep'} =~ /^(gmake|\${GMAKE})$/) { - &perror("WARN", $file, -1, "dependency to $1 ". - "listed in $j. consider using ". - "USE_GMAKE."); - } - - # check USE_QT - if ($m{'dep'} =~ /^(qt\d)+$/) { - &perror("WARN", $file, -1, "dependency to $1 ". - "listed in $j. consider using ". - "USE_QT."); - } - - # check LIBLTDL - if ($m{'dep'} =~ /^(ltdl\.\d)+$/) { - &perror("WARN", $file, -1, "dependency to $1 ". - "listed in $j. consider using ". - "USE_LIBLTDL."); - } - - # check CDRTOOLS - if ($m{'dir'} =~ /(cdrtools|cdrtools-cjk)$/) { - &perror("WARN", $file, -1, "dependency to $1 ". - "listed in $j. consider using ". - "USE_CDRTOOLS."); - } - - # check GHOSTSCRIPT - if ($m{'dep'} eq "gs") { - &perror("WARN", $file, -1, "dependency to gs ". - "listed in $j. consider using ". - "USE_GHOSTSCRIPT(_BUILD|_RUN)."); - } - - # check JAVALIBDIR - if ($m{'dep'} =~ m|share/java/classes|) { - &perror("FATAL", $file, -1, "you should use \${JAVALIBDIR} ". - "in BUILD_DEPENDS/RUN_DEPENDS to define ". - "dependencies on JAR files installed in ". - "\${JAVAJARDIR}"); - } - - # check backslash in LIB_DEPENDS - if ($osname eq 'NetBSD' && $j eq 'LIB_DEPENDS' - && $m{'dep'} =~ /\\\\./) { - &perror("WARN", $file, -1, "use of backslashes in ". - "$j is deprecated."); - } - - # check for PREFIX - if ($m{'dep'} =~ /\${PREFIX}/) { - &perror("FATAL", $file, -1, "\${PREFIX} must not be ". - "contained in *_DEPENDS. ". - "use \${LOCALBASE} or ". - "\${X11BASE} instead."); - } - - # check port dir existence - $k = $m{'dir'}; - $k =~ s/\${PORTSDIR}/$ENV{'PORTSDIR'}/; - $k =~ s/\$[\({]PORTSDIR[\)}]/$ENV{'PORTSDIR'}/; - if (! -d $k) { - &perror("WARN", $file, -1, "no port directory $k ". - "found, even though it is ". - "listed in $j."); - } else { - print "OK: port directory $k found.\n" - if ($verbose); - } - } - } + check_depends_syntax({ + tmp => $tmp, + file => $file + }); foreach my $i (@linestocheck) { $tmp =~ s/$i[?+]?=[^\n]+\n//g; @@ -2527,6 +2541,13 @@ &checkearlier($file, $tmp, @varnames); + # Check depends that might be specified based on the WITH_/WITHOUT_ + # arguments and other external variables. + check_depends_syntax({ + tmp => $tmp, + file => $file + }); + # check WRKSRC/NO_WRKSUBDIR # # do not use DISTFILES/DISTNAME to control over WRKSRC. --- portlint-chase_late_DEPENDS.diff begins here --- >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080820134310.B89AB1AF41C>