Subject: Re: Major oops in PKG_OPTIONS_LEGACY_VARS
To: Todd Vierling <tv@duh.org>
From: Dieter Baron <dillo@danbala.tuwien.ac.at>
List: tech-pkg
Date: 11/02/2005 19:32:37
hi,

> It seems that setting USE_INET6=NO does not actually remove "inet6" from the
> default options, and thus it does not do what the user expects.  This is
> rather bad.  The reason is the block doing the mapping:

  What do you mean by ``default options''?  USE_INET6=NO overrides
	PKG_SUGGESTED_OPTIONS=inet6
(in a packages' options.mk or Makefile), but does not override
	PKG_DEFAULT_OPTIONS=inet6
in /etc/mk.conf (or equivalent).

  And that is how legacy variable handling is supposed to work: it
overrides the packager's suggestions, but does not override the
settings the user did using the options framework.

  I just verified that it does indeed work this way:

$ make show-options USE_INET6=NO
Any of the following general options may be selected:
        inet6    Enable support for IPv6.
        lua     
        ssl      Enable SSL support.

These options are enabled by default: lua ssl
These options are currently enabled: lua ssl

Deprecated variable USE_INET6 set to NO, use PKG_DEFAULT_OPTIONS+=-inet6 instead.

> =====
> .for _m_ in ${PKG_OPTIONS_LEGACY_VARS}
> _var_:=	${_m_:C/:.*//}
> _opt_:=	${_m_:C/.*://}
> _popt_:=${_opt_:C/^-//}
> .  if !empty(PKG_SUPPORTED_OPTIONS:M${_popt_})
> .    if defined(${_var_})
> .      if empty(${_var_}:M[nN][oO])
> PKG_LEGACY_OPTIONS:=${PKG_LEGACY_OPTIONS} ${_opt_}
> PKG_OPTIONS_DEPRECATED_WARNINGS:=${PKG_OPTIONS_DEPRECATED_WARNINGS} "Deprecated variable "${_var_:Q}" set to "${${_var_}:Q}", use PKG_DEFAULT_OPTIONS+="${_opt_:Q}" instead."
> .      elif empty(_opt_:M-*)
> PKG_LEGACY_OPTIONS:=${PKG_LEGACY_OPTIONS} -${_popt_}
> PKG_OPTIONS_DEPRECATED_WARNINGS:=${PKG_OPTIONS_DEPRECATED_WARNINGS} "Deprecated variable "${_var_:Q}" set to "${${_var_}:Q}", use PKG_DEFAULT_OPTIONS+=-"${_popt_:Q}" instead."
> .      else
> PKG_LEGACY_OPTIONS:=${PKG_LEGACY_OPTIONS} ${_popt_}
> PKG_OPTIONS_DEPRECATED_WARNINGS:=${PKG_OPTIONS_DEPRECATED_WARNINGS} "Deprecated variable "${_var_:Q}" set to "${${_var_}:Q}", use PKG_DEFAULT_OPTIONS+="${_popt_:Q}" instead."
> .      endif
> .    endif
> .  endif
> .endfor
> .undef _var_
> .undef _opt_
> .undef _popt_
> =====
> 
> Now, obsolete.mk sets USE_INET6:inet6.  In the USE_INET6=NO case, where
> "-inet6" is not in the list of options set by the user, we fall through to
> the third setting block.

  I don't understand.  What list of options set by the user do you mean?

> If you look carefully, you'll see that "_popt_" simply removes any leading
> "-" from the option name -- but does not add one if one did not exist in the
> first place.

  Indeed. _popt_ stands for ``positive option'', i. e. the option with
any leading ``-'' removed.

>  So, in a case like USE_INET6:inet6, the third setting block is
> identical to the first one, turning the option *on* rather than off.

  I think you misunderstood the code:

  The first block is used if the variable is defined but not set to
NO, i. e. logically set to YES.  It adds _opt_, the option with any
leading ``-'' (inet6 in this case).

  The second block is used if the variable is considered set to NO
and _opt_ does not contain a leading ``-'' (as is the case with
USE_INET6:inet).  It adds ``-${_popt_}'' (-inet in this case).

  The second block is used if the variable is considered set to NO and
_opt_ *does* contain a leading ``-''.  It adds ``${_popt_}'' (thus
enabling the option that the legacy variable would disable if set to
YES).

  Does this clarify how things are supposed to work?  Do you still
think bsd.options.mk is in error?  If so, how?

					yours,
					dillo