Subject: poor/inappropriate factoring of options in many "related" packages....
To: NetBSD Packages Technical Discussion List <tech-pkg@NetBSD.ORG>
From: Greg A. Woods <woods@weird.com>
List: tech-pkg
Date: 10/17/2002 13:39:53
This should probably be a pair of PRs, one a bug report and another a
change request, but I wanted to point out the issue here because this is
really just an example of many similar cases now getting "worse" in
some senses in pkgsrc.  Since I don't build and use all packages there's
really no way that I alone could supply a comprehensive set of similar
PRs for all packages where these issues arise.

The good thing is that many packages are being created which are
optional ways to configure and build core packages, and this is being
done in a nicely maintainable way with common bits put into a
"Makefile.common" and all the relevant patches and PLIST files and such
being shared as well.

The bad thing is that this way of splitting some kinds of options into
separate packages makes it very hard for a site using pkgsrc to choose a
common way of doing things across many/all packages.  I.e. I think too
many options are being split out into separate packages where the better
choice would be to provide one control variable in bsd.pkg.defaults.mk.
This is especially important when the separate options packages directly
conflict with each other (i.e. it wouldn't be so bad to have separate
options packages if they could all be installed simultaneously, but
usually that is not the case).

Unfortunately it's also often being done without a proper perspective of
the larger picture of multiple NetBSD architectures and without apparent
concern for other already available options within pkgsrc.  The worst
examples are of packages like chat/gaim here which don't need Perl (or
some other add-on language used only for rarely wanted scripts or other
non-core extensions), but which have it enabled anyway, and also with
packages where the maintainer has chosen subsystem options which are not
necessarily appropriate for all platforms, such as a choice of audio
output system as in gaim (and a plethora of other packages which already
have NAS support but it is disabled).  I realize there are issues with
available volunteer support to test all the options not currently
supported, but that's why I'm trying to offer some guidance and suggest
changes where I have experience with them.

Sometimes the control of common options could be much better "factored"
between the common and option-specific makefile components too.

To illustrate these points with a graphic example here are some changes
for chat/gaim that I have in my local pkgsrc.

In my local mk/bsd.pkg.defaults.mk I use a mechanism like this to turn
on NAS and turn off Perl (in all places were it is an option), SSL,
etc. with one big switch:

	DEFAULT_USE_PERL?= YES
	# Used to specify generic Perl support for a number of packages.
	# Possible: YES, NO
	# Default: YES

	GAIM_USE_PERL?=	${DEFAULT_USE_PERL}
	# Enable Perl in chat/gaim*
	# Possible: YES, NO
	# Default: YES

Note that if one tried to even just expand Perl, NAS, and SSL support
into separate packages there'd be quite an explosion of packages and for
no good reason since they'd all conflict with each other.  Testing would
actually be harder, and site-wide choice and use would be almost
impossible without extensive one-by-one choice.

(note there can be PLIST complications, but I'm ignoring them for this
example....)

Index: chat/gaim/Makefile.common
===================================================================
RCS file: /cvs/master/m-NetBSD/main/pkgsrc/chat/gaim/Makefile.common,v
retrieving revision 1.4
diff -c -r1.4 Makefile.common
*** chat/gaim/Makefile.common	28 Sep 2002 03:49:18 -0000	1.4
--- chat/gaim/Makefile.common	17 Oct 2002 17:11:28 -0000
***************
*** 23,36 ****
  LIBTOOL_OVERRIDE=	${WRKSRC}/libtool
  
  GNU_CONFIGURE=		YES
  CONFIGURE_ARGS+=	--disable-artsc
! CONFIGURE_ARGS+=	--disable-nas
- CONFIGURE_ARGS+=	--disable-perl
  CONFIGURE_ARGS+=	--disable-screensaver
  CONFIGURE_ARGS+=	--with-libiconv-prefix=/nonexistant
  
- CONFIGURE_ARGS+=	--enable-esd
- CONFIGURE_ARGS+=	--enable-perl
  CONFIGURE_ARGS+=	--enable-pixbuf
  
  # Newer versions of gaim use gettext checks that gettext-lib/buildlink2.mk
--- 23,45 ----
  LIBTOOL_OVERRIDE=	${WRKSRC}/libtool
  
  GNU_CONFIGURE=		YES
+ 
+ .include "../../mk/bsd.prefs.mk"
+ 
  CONFIGURE_ARGS+=	--disable-artsc
! .if  defined(GAIM_USE_PERL) && ${GAIM_USE_PERL} == "YES"
  CONFIGURE_ARGS+=	--disable-perl
+ .else
+ CONFIGURE_ARGS+=	--enable-perl
+ .endif
  CONFIGURE_ARGS+=	--disable-screensaver
+ 
+ .if 0	# XXX why not this too?  NOTE: it is in ../gaim-gnome!
+ .include "../../converters/libiconv/buildlink2.mk"
+ .else
  CONFIGURE_ARGS+=	--with-libiconv-prefix=/nonexistant
+ .endif
  
  CONFIGURE_ARGS+=	--enable-pixbuf
  
  # Newer versions of gaim use gettext checks that gettext-lib/buildlink2.mk
***************
*** 38,40 ****
--- 47,54 ----
  #
  CONFIGURE_ARGS+=	--without-included-gettext
  CONFIGURE_ENV+=		gt_cv_func_gnugettext1_libc=yes
+ 
+ .include "../../graphics/gdk-pixbuf/buildlink2.mk"
+ .if 0	# GAIM_USE_PERL
+ .include "../../lang/perl5/buildlink2.mk"
+ .endif
Index: chat/gaim/Makefile
===================================================================
RCS file: /cvs/master/m-NetBSD/main/pkgsrc/chat/gaim/Makefile,v
retrieving revision 1.21
diff -c -r1.21 Makefile
*** chat/gaim/Makefile	28 Aug 2002 07:43:52 -0000	1.21
--- chat/gaim/Makefile	17 Oct 2002 17:10:21 -0000
***************
*** 9,19 ****
  
  CONFIGURE_ARGS+=	--disable-gnome
  CONFIGURE_ARGS+=	--disable-panel
  
  PLIST_SRC+=		${.CURDIR}/../gaim/PLIST
  
  .include "../../audio/esound/buildlink2.mk"
! .include "../../graphics/gdk-pixbuf/buildlink2.mk"
! .include "../../lang/perl5/buildlink2.mk"
  
  .include "../../mk/bsd.pkg.mk"
--- 9,28 ----
  
  CONFIGURE_ARGS+=	--disable-gnome
  CONFIGURE_ARGS+=	--disable-panel
+ CONFIGURE_ARGS+=	--disable-screensaver
+ 
+ .if defined(GAIM_USE_NAS) && ${GAIM_USE_NAS} == "YES"
+ CONFIGURE_ARGS+=	--enable-nas
+ .else
+ CONFIGURE_ARGS+=	--disable-esd
+ .endif
  
  PLIST_SRC+=		${.CURDIR}/../gaim/PLIST
  
+ .if defined(GAIM_USE_NAS) && ${GAIM_USE_NAS} == "YES"
+ .include "../../audio/nas/buildlink2.mk"
+ .else
  .include "../../audio/esound/buildlink2.mk"
! .endif
  
  .include "../../mk/bsd.pkg.mk"

-- 
								Greg A. Woods

+1 416 218-0098;            <g.a.woods@ieee.org>;           <woods@robohack.ca>
Planix, Inc. <woods@planix.com>; VE3TCP; Secrets of the Weird <woods@weird.com>