[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: pkg/46570 (infelicities in pkglint)
The following reply was made to PR pkg/46570; it has been noted by GNATS.
From: David Holland <dholland-pbugs%netbsd.org@localhost>
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
1. It doesn't know about the "|| exit 1" idiom for making shell loops
safe. For example, it shouldn't complain about this:
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
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
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
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
11. In textproc/saxon it announces
WARN: Makefile:3: The package is being downgraded from 188.8.131.52j (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
Main Index |
Thread Index |