tech-pkg archive

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

Re: Bring BLAS into pkgsrc, pt. II

> # $NetBSD$
> #
> # This Makefile fragment is meant to be included by packages
> # that use any BLAS implementation instead of one particular one.

In this introductory paragraph, it would be nice to describe what BLAS
actually means. A simple "that use any BLAS (Basic Linear Algebra
Subprograms) implementation" already suffices.

> # Since we always ship BLAS and LAPACK together (as upstream variants

Who is "we", and how does the reader know that?

> # TODO: Also set a variable to find a matching pkg-config file
> # to avoid duplicating its contents in BLAS_LIBS?

There should be as few TODO lines in pkgsrc/mk as possible. (I know I'm
guilty of adding several TODOs to pkgsrc myself as well, though.)

> # === Per-package variables ===

This should rather be the established "Package-settable variables", as
in many other files. Someday pkglint will parse these descriptions and
base its checks on them. Therefore it's nice if the wording is
consistent. This also benefits the human readers.

Currently, "bmake help topic=blas" only says:

    No help found for blas, but it appears in:


To change this, you can add a comment "# Keywords: blas lapack" at the
end of the introductory comment block.

> .include "../../mk/"

This should simply be '.include ""', as will be
in the same directory as

> .if(!empty(_BLAS_MATCH))

Usually we put a space between the .if and the condition. There's no
pkglint check for that yet since I am seeing this for the very first
time. Also the outer parentheses are redundant.

> .if defined(_BLAS_PACKAGE)
> .include "../../${_BLAS_PACKAGE}/"
> .endif

This variable is closely related to the PKGPATH variable. Since the name
"PACKAGE" is highly ambiguous (it could mean one of PKGNAME,
prefer this variable to be called _BLAS_PKGPATH instead.

This variable name probably originated from _MPI_PACKAGE, so I see it's
picking up existing practice. Apart from that, the word PACKAGE is not
used in variable names to identify a package, probably because of this

Home | Main Index | Thread Index | Old Index