Subject: Annoncement: New pkglint warnings
To: None <tech-pkg@netbsd.org, pkgsrc-wip-discuss@lists.sourceforge.net>
From: Roland Illig <rillig@NetBSD.org>
List: tech-pkg
Date: 02/03/2006 04:06:40
Dear pkgsrc developers,

recently there have appeared some new pkglint warnings that I'd like to 
explain in more detail here. As an example package, I have taken 
audio/oss, since the warnings it produces are common among all pkgsrc 
packages.

 > RESTRICTED=           "No re-distribution."
WARN: audio/oss/Makefile:25: RESTRICTED should not be quoted.

One class of warnings concerns the quoting of variables. In December, 
pkglint had marked many variables as not being enough quoted, now it 
warns about unneeded quotes. So you may wonder about when to quote and 
when not to quote.

The basic principle is quite simple. Keep the definitions as simple as 
possible. The one and only data type that make(1) knows is the String. 
Strings are simply lists of characters that are written without quotes. 
Like in the COMMENT variable, the RESTRICTED variable contains a single 
string, so it does not need quoting. The COMMENT variable has been 
defined without quotes since the beginning of pkgsrc, so there's no 
reason why RESTRICTED should be defined with quotes. To remove this 
unnecessary inconsistency, RESTRICTED should not be quoted as well.

 > .for FILE in ${SFILES}
WARN: audio/oss/Makefile:49: .for variable names should not contain 
uppercase letters.

Another class of warnings concerns naming conventions for variables. 
There are a lot of conventions in pkgsrc, but up to now, no one has had 
the time to write them down.

One basic rule is that every variable name that starts with an 
underscore is reserved for the pkgsrc infrastructure. For package 
developers, this means: Never use them. Never define them. And if 
there's no other way to achieve what you want, file a bug report.

Independent from that, variable names consisting of only uppercase 
letters and underscores are used for global definitions, as well as for 
imported environment variables. The example above thus possibly 
redefines an environment variable, hiding it from the commands that are 
called from within make(1). Therefore, the .for variable names should 
consist of only lowercase letters and underscores.

 > chroot:
WARN: audio/oss/Makefile:77: Unusual target "chroot".

The third class of warnings has been introduced to catch possible typos. 
Imagine you had written a target "post-cnofigure", and now you are 
wondering why it has no effect. The good thing is that there are very 
few predefined make(1) targets that can be overwritten by package 
authors. All other targets are free for individual use. But when you 
write a target like "my-post-configure", this might conflict with a 
_file_ having the same name, so make(1) must be told that you are 
defining a "virtual" target that does not create a file of its own name. 
This can be done using the .PHONY target, which serves like a variable 
declaration in other common programming languages.

 > What to do now?

Please have a look at your packages if you are getting any of these 
warnings. If you do, please fix them. Please also have a look at the 
advanced pkglint options that have been made available in the last 
months. The most useful are:

$ pkglint --explain                # or pkglint -e

This prints additional explanations for some of the warnings. Try it 
with the third warning above, for example.

$ pkglint --source                 # or pkglint -s

This prints the source code where the warning has been anchored, in the 
format that has been used in this mail. It can be used to quickly see 
whether a warning is really useful (they mostly are) and what the 
criticized code is, without needing to open the file in an editor.

Roland