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