On Wed, 2005-06-01 at 14:07 +0200, Thomas Klausner wrote:
> On Tue, May 31, 2005 at 08:49:17PM +0000, Julio M. Merino Vidal wrote:
> > the attached patch converts mplayer to the package options framework.
> > More specifically, it gets rid of the MPLAYER_DISABLE_DRIVERS,
> > MPLAYER_ENABLE_RUNTIME_CPU_DETECTION and MPLAYER_USE_MEDIALIB variables.
> > (How do I mark them as deprecated?  mk/defaults/obsolete.mk does not
> > seem the right place, as it is says "global vars".)
> 
> From mk/bsd.options.mk:
> #       PKG_OPTIONS_LEGACY_VARS
> #               This is a list of USE_VARIABLE:option pairs that
> #               map legacy /etc/mk.conf variables to their option
> #               counterparts.
> 
> Just add a PKG_OPTIONS_LEGACY_VARS line to the options.mk file.
> It's also possible to add negated options this way, i.e.
> PKG_OPTIONS_LEGACY_VARS+=	NO_FOO:-foo

I've added:

.if ${MACHINE_ARCH} == "i386"
PKG_OPTIONS_LEGACY_VARS+= \
    MPLAYER_ENABLE_RUNTIME_CPU_DETECTION:mplayer-runtime-cpudetection
.endif

But it seems to do nothing.  At least, the corresponding option is not
added by default, although the variable is explicitly set to YES by
mk/defaults/mk.conf.

> > However, note that I've not added neither arts, nor esound, nor nas to
> > the default options.  I'm not sure this is the way to go, but reduces
> > the amount of dependencies by default.  A reason in favor is that we
> > already have several packages that support options to enable these
> > backends, and they are disabled by default.  The "bad" side is that
> > binary packages won't have any of these audio output plugins compiled in
> > (but we don't have binary packages in FTP yet, do we?  See below,
> > anyway.)
> 
> I don't think you should change the default options. People
> who don't want them have already adjusted...

Hmm... OK, I almost left it as it was (added a mplayer-menu option,
turned on by default, as it seems to do no harm and was requested in a
PR).

> > Any objections to this?  Any detail I should improve?
> 
> Yes:
> PKG_DEFAULT_OPTIONS is user-setable and should _not_ be used
> in options.mk files. The variable you want to use instead is
> PKG_SUGGESTED_OPTIONS (see mk/bsd.options.mk and the guide).

Yeah, I saw that minutes after sending the mail (and reading another
message from Dieter (IIRC), who explained this).

> The option names for at least runtime-cpudetection, win32, xvid,
> and real should be prefixed with "mplayer-" because I don't think
> other packages will use them.

Makes sense.  The following ones are prefixed:

        mplayer-menu     Enable support for user-defined menus.
        mplayer-real     Enable usage of Real codecs.
        mplayer-runtime-cpudetection    Enable CPU detection at run
time.
        mplayer-win32    Enable usage of Win32 DLLs (codecs).
        mplayer-xvid     Enable usage of XVid codecs.

while all others are not (basically because they are already used, or
may be used some day by other packages).

> Option descriptions for all options (including the package-specific
> ones) should be added to mk/defaults/option.descriptions.

Nice!  I wasn't aware of that file; it makes 'make show-options' output
quite user-friendly :)

I think I'll commit what I have this evening.

Cheers,

-- 
Julio M. Merino Vidal <jmmv84@gmail.com>
http://www.livejournal.com/users/jmmv/
The NetBSD Project - http://www.NetBSD.org/