pkgsrc-WIP-discuss archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Annoncement: New pkglint warnings



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


-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=103432&bid=230486&dat=121642
_______________________________________________
pkgsrc-wip-discuss mailing list
pkgsrc-wip-discuss%lists.sourceforge.net@localhost
https://lists.sourceforge.net/lists/listinfo/pkgsrc-wip-discuss



Home | Main Index | Thread Index | Old Index