Subject: Re: RFC: Removing redundancy from packages that use bsd.options.mk
To: Roland Illig <rillig@NetBSD.org>
From: Dieter Baron <dillo@danbala.tuwien.ac.at>
List: tech-pkg
Date: 06/06/2007 11:27:47
In article <466665EB.9020502@NetBSD.org> Roland wrote:
: [-- text/plain, encoding 7bit, charset: us-ascii, 23 lines --]
: Hi,
: we have a fairly established convention of how the variables for
: package-specific options are called: PKG_OPTIONS.${pkgbase}. In most
: cases, this value can be computed from either PKGNAME or DISTNAME (in
: that order), so that most of the currently 570 definitions for that
: variable can be removed. The appended patch does the work to enable the
: removal. May I commit it?
: I'm asking because I am not sure about other's opinions. Maybe some
: developers want to keep that redundancy to make the reader aware of the
: variable name. But to those I can say that the variable name is printed
: in an eye-catching way when building the package.
The reason it is mandatory is that PKGBASE is not yet defined when
bsd.options.mk is parsed and I did not want to duplicate the code for
deriving PKGBASE. Otherwise it would default to
PKG_OPTIONS.${PKGBASE}.
: Index: bsd.options.mk
: ===================================================================
: RCS file: /cvsroot/pkgsrc/mk/bsd.options.mk,v
: retrieving revision 1.60
: diff -u -p -r1.60 bsd.options.mk
: --- bsd.options.mk 6 Jun 2007 07:12:31 -0000 1.60
: +++ bsd.options.mk 6 Jun 2007 07:35:48 -0000
: @@ -11,9 +11,11 @@
: #
: # PKG_SUPPORTED_OPTIONS= kerberos ldap ssl
: #
: -# PKG_OPTIONS_VAR (must be defined)
: +# PKG_OPTIONS_VAR
: # The variable the user can set to enable or disable
: -# options specifically for this package.
: +# options specifically for this package. If it is not
: +# given, it is derived from either PKGNAME or DISTNAME,
: +# and is of the form PKG_OPTIONS.${PKGBASE}.
: #
: # PKG_OPTIONS_OPTIONAL_GROUPS
: # This is a list of names of groups of mutually exclusive
: @@ -155,6 +157,22 @@
: PKG_OPTIONS= # empty
:
: # Check for variable definitions required before including this file.
: +.if defined(PKG_OPTIONS_VAR) && defined(PKGNAME)
: +. if ${PKG_OPTIONS_VAR} == "PKG_OPTIONS.${PKGNAME:C/-[0-9].*//}"
: +WARNINGS+= "[bsd.options.mk] Setting PKG_OPTIONS_VAR is redundant, since it can be derived from PKGNAME."
: +. endif
: +.endif
: +.if defined(PKG_OPTIONS_VAR) && defined(DISTNAME)
: +. if ${PKG_OPTIONS_VAR} == "PKG_OPTIONS.${DISTNAME:C/-[0-9].*//}"
: +WARNINGS+= "[bsd.options.mk] Setting PKG_OPTIONS_VAR is redundant, since it can be derived from DISTNAME."
: +. endif
: +.endif
This results in two warnings if PKGNAME and DISTNAME are set and
match. Use an .elsif here.
: +.if defined(PKGNAME)
: +PKG_OPTIONS_VAR?= PKG_OPTIONS.${PKGNAME:C/-[0-9].*//}
This pattern is wrong. It should be /-[^-]*$//. This is precicly
the reason I did not want to duplicate it. If you go ahead with this
(and I would prefer you didn't), please add comments here and
wherever PKGBASE is defined that the two versions must be kept in
sync.
yours,
dillo