Re: pkg/46570 (infelicities in pkglint)

[David Holland <dholland-pbugs%netbsd.org@localhost>]

The following reply was made to PR pkg/46570; it has been noted by GNATS.

From: David Holland <dholland-pbugs%netbsd.org@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: pkg/46570 (infelicities in pkglint)
Date: Sun, 12 Jun 2016 00:36:48 +0000

 On Sat, Jan 16, 2016 at 12:45:41AM +0000, rillig%NetBSD.org@localhost wrote:
  > Please check whether your issues are fixed.
  > I think I have fixed most of them.
 
 43 of 54 issues described in this PR are fixed. The ones remaining
 follow. (I've repeated them in detail so there shouldn't be any
 further need to look at the parts of the PR before this mail.)
 I also discovered a few more issues while testing these, which I've
 appended. (I haven't gone *looking* for more; if I do that I'll open a
 new PR.)
 
 
 1. It doesn't know about the "|| exit 1" idiom for making shell loops
 safe. For example, it shouldn't complain about this:
 
    post-install:
 	   for x in 1 2 3; do echo "$$x" || exit 1; done
 
 This is ultimately preferred to set -e because there are, at least
 according to the wrong language standardized in POSIX, shell
 constructs that don't trigger -e when failing. (It is a big ugly
 mess.) I wouldn't say it should recommend this over set -e, but it
 shouldn't object to it, and it certainly shouldn't be complaining that
 the semicolon after the "exit 1" isn't checked for error.
 
 To test this, add the above construct to your favorite test package.
 
 
 2. It objects if the include guard in a bl3 file doesn't match the
 package name when the package name contains characters not legal in
 make identifiers. It ought to fold the package name into a legal make
 identifier before comparing, transforming - and + and . and probably a
 few other things to _.
 
 For example: x11/gtk+extra, where it wants the include guard symbol to
 be GTK+EXTRA.
 
 
 3. In lang/perl5 for some reason it thinks that the awk script at line
 76 of packlist.mk ought to be a valid pathname.
 
 
 4. Shell quoting in mail/bulk_mailer causes it to issue bogus
 complaints about compiler arguments. To see this you now need to
 retrieve Makefile -r1.11 from CVS, but if you do that it still
 happens.
 
 
 5. It accepts multiple LICENSEs without boolean algebra, but pkgsrc
 doesn't. For example:
    LICENSE=        gnu-gpl-v2 gnu-lgpl-v2 modified-bsd mit
 
 
 6. It should complain if PKG_OPTIONS_VAR is wrong. It should be
 PKG_OPTIONS.pkgbase; currently you can both set it to e.g.
 PKG_OPTIONZ.pkgbase or to PKG_OPTIONS.someotherpkg and pkglint doesn't
 say anything. There are some potential problems with the latter, but
 it won't generate any more false positives than we routinely get from
 pkglint from other things and it's probably worth trying to enforce.
 
 
 7. It still demands a LICENSE for meta-packages, which is silly.
 
 
 8. apb@ suggested that pkglint should warn if a package with an OWNER
 has been modified and the OWNER isn't the current user. And perhaps
 also for MAINTAINER (if the MAINTAINER is not pkgsrc-users) but at a
 lower level. This seems like a good idea and hasn't been done. Note
 that while using cvs diff to check for modifications is unacceptably
 expenssive, reading the CVS control files (which pkglint already does
 to check for un-CVS'd patches, for example) should let it check by
 timestamp.
 
 
 9. If LICENSE is no-commercial-use it should print something about
 what to do instead (that is, extract the package's own specific
 license) as well as saying that the setting is deprecated.
 
 
 10. It should stop demanding desktop.db unless the .desktop file
 involved declares a MimeType. This is going to be a pain, but it's
 fairly important. It's possible that this check should be migrated out
 of pkglint and into the pkgsrc infrastructure as a check after
 stage-install, because then the desktop files are at least available
 for analysis.
 
 
 11. In textproc/saxon it announces 
    WARN: Makefile:3: The package is being downgraded from 9.5.0.1j (see
    ../../doc/CHANGES-2013:2602) to 1J
 which is quite bogus. (This definitely happens with Makefile -r1.24
 and probably earlier, but who knows about later.)
 
 
 12. (new) It doesn't know about the new per-OS settings, e.g:
    CONFIGURE_ENV.SunOS is defined by not used. (archivers/libarchive)
    CONFIGURE_ARGS.Darwin is defined but not used. (graphics/clutteR)
    MAKE_ENV.Interix is defined but not used. (devel/bmake)
 
 
 13. It doesn't know about the BUILTIN_FIND_HEADERS mechnism:
    BUILTIN_FIND_HEADERS_VAR is defined but not used. (archivers/libarchive)
 
 
 14. It objects to using CHECK_BUILTIN from makefiles, although there's
 a couple dozen packages that do this -- maybe they're all abusive, but
 this seems like it ought to be discussed.
 
 
 15. net/uucp/Makefile has a make loop that's used to substitute
 variable names, and this confuses it.
 
 
 -- 
 David A. Holland
 dholland%netbsd.org@localhost