tech-pkg archive

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

pkglint is broken and harmful



  Hello,

Since these problems recur each time some new developer joins the project,
I have to reiterate it.

pkglint in its current state is broken and harmful.

If anyone wants or wishes to use it, one ought to use it with care.
Blindly following its messages doesn't help anything.

There're multiple reasons for it.

First, pkglint doesn't actually handle make syntax so that it could be
used as analysis tool.

Second, pkglint doesn't handle make semantics at all, even though this
is required to analyze code to issue recommendations like those it does.

Third, pkglint doesn't actually follow recent developments in pkgsrc.

Fourth, pkglint doesn't actually follow the documented style.
What is worth, in many cases it insists on style inconsistent with or
opposite to the documented one.


Here's longer explanation what and why.

pkglint operates by matching _some_ (not all) files against a set of
regular expressions and using some sort of heuristic, almost undocumented,
to warn about potential problems. This alone is a strong reason
to be critical of warnings rather than act so as to silence them.
Matching regular expressions is too limited in power to be
an authorative guidance.

pkglint doesn't handle make syntax. This problem arises each time it
meets .include directive. This causes pkglint not to analyze files
that are used to handle programs in languages like O'Caml, Common Lisp,
Python, Perl, Ruby, and so on. As one of consequence pkglint emits
false warning about "broken" code that works for a huge number of Python
packages but allegedly "doesn't" for Racket.

pkglint doesn't handle semantics of make language. At all.
This leads to "fixes" where correct code like
  $(MKDIR) $(CONFIGURE_DIRS)
is replaced with incorrect one like
  $(MKDIR) $(WRKSRC)/$(CONFIGURE_DIRS)
Human reader notices immediatly that this violates typing considerations:
you're trying to treat a list of strings as a simple string while make
doesn't implement it as implicit "map". This is how working around
a problem in pkglint to silence warning leads to conceptually broken code.

To add to the latter, pkglint embeds knowledge that CONFIGURE_DIRS is a list,
but it doesn't perform any checks that such variables entry operations
allowed by type.

It took a while to get pkglint to stop emitting warnings about no longer
mandatory PKG_DESTDIR_SUPPORT, but it still carries incorrect information
on allowed values for the variable. Thus pkglint doesn't follow code
development closely enough to take it as a rule for an action.

And last but not least. When it comes to style, pkglint diverges from
documented one and sometimes it does that severely.

For instance, take the above CONFIGURE_DIRS example (real life one, see
lang/squeak-vm). Here CONFIGURE_DIRS usage is documented in code as:

$ make help topic=CONFIGURE_DIRS
===> mk/configure/bsd.configure-vars.mk (keywords: SCRIPTS_ENV CONFIGURE_DIRS):
# CONFIGURE_DIRS is the list of directories in which to run the
#       configure process.  If the directories are relative paths,
#       then they are assumed to be relative to ${WRKSRC}.
#
# SCRIPTS_ENV is the shell environment passed to xmkmf (used by
#       the configure process).
#
CONFIGURE_DIRS?=        ${WRKSRC}
SCRIPTS_ENV?=           # empty

Note that the name of variable, the help text, and the default value
suggest that it is a list of absolute directories. pkglint insists on
using relative directories ignoring the fact that some outside
developers use this help text as directions.

The last, but not the least, pkglint seems to be written by people who
have never tried to learn basics of typography. This alone is not bad
except that in this case reasonable person should not pretend to be an
expert and suggest stupid formatting rules that affect how text is typeset
and presented to end user.

In particular, pkgsrc/biology/mpqc/DESCR ends as

http://ftp.NetBSD.org/pub/pkgsrc/current/pkgsrc/biology/mpqc/DESCR

which is reachable and supposed to be reachable from the web site.
What is more important, it is supposed to be read by end user in his
web browser. Thus it should be typeset reasonably enough for a web user.

80 characters per line requirement pkglint insists upon has no basis
nor support among typographic designers and other specialists in
cognitive sciences.

Classic recommendations are 45-60 characters per line here, and these
are based on field observations (e.g. classic data from newspaper sales)
and experiments. (Though there exist modern data that favour _longer_ lines
under some conditions, but those around 90-95 characters per line,
still different from 80.)

In addition to that, there exist strong indications that structured text
is perceived better than less structured one. It has better readability,
where "readability" is a typographical _term_ rather than some handwaving
about the number of columns in IBM PC text mode. (The latter matches
the number of columns on punch card which was made to be equal to
classical number of characters per line plus allowance to accomodate
unusually long lines plus space needed to carry punch card position
in a deck.)

Readability is the reason why databases/postgresql91-datatypes/DESCR should
list modules one per line rather than piled one after another.
The reason is that end user should be able to see it easily what modules
it provides. The goal here is to emphasize the presence of particular modules
rather than to overwhelm reader with a number of those. An example
of usage for overwhelming reader with number of items on the list can be found,
for instance, in The Postman's Tale by Karel Capek. The latter is definitly
not meant for a package which creation was funded by some customer
(which is the case for postgresql*-datatypes packages as well as for some 
others).

In conclusion, the list above is not exclusive. It is long enough to
demonstrate that pkglint is at least not ready to be used as a tool to
check style of code and/or documentation like DESCR in pkgsrc and at most
it should be declared as broken and fixed before entering use at all.
In addition to that, some style recommendations pkglint encodes should be
revised to follow the state of art rather than enforcing arbitrary constraints.


-- 
BECHA...
   CKOPO CE3OH...



Home | Main Index | Thread Index | Old Index