Re: [gentoo-dev] [PATCH 1/3] ecm-utils.eclass: New eclass

[Andreas Sturmlechner]

On Tuesday, 5 November 2019 22:20:46 CET Michał Górny wrote:
> On Tue, 2019-11-05 at 00:30 +0100, Andreas Sturmlechner wrote:
> > --- /dev/null
> > +++ b/eclass/ecm-utils.eclass
>
> I know we historically screwed this up repeatedly but please don't use
> '-utils' for eclasses that export phases.

Fine, I would then choose ecm.eclass instead.

> > +# @ECLASS-VARIABLE: ECM_NONGUI
> > +# @DESCRIPTION:
> > +# If set to "false", add dependency on kde-frameworks/breeze-icons
> > +# or kde-frameworks/oxygen-icons and run the xdg.eclass routines for
> > +# pkg_preinst, pkg_postinst and pkg_postrm.
> > +# For any other value, do nothing.
> > +if [[ ${CATEGORY} = kde-frameworks ]]; then
> > +	: ${ECM_NONGUI:=true}
> > +fi
> > +: ${ECM_NONGUI:=false}
>
> I don't think eclassdoc is going to parse this correctly.

Can we do something about that? I need to be able to set (overrideable) 
defaults for a category without being limited by eclassdoc. @DEFAULT_UNSET 
would not be precise. Same as ECM_QTHELP, this is what we do in kde5.eclass 
already.

> > +# @ECLASS-VARIABLE: ECM_DEBUG
> > +# @DESCRIPTION:
> > +# If set to "false", add -DNDEBUG (via cmake-utils_src_configure) and
> > +# -DQT_NO_DEBUG to CPPFLAGS.
> > +# Otherwise, add debug to IUSE.
> > +: ${ECM_DEBUG:=true}
>
> To be honest, I don't really like this 'anything-or-false' logic.  It's
> rather confusing and error-prone.  For example, if I misspell 'false'
> the eclass is going to silently assume true.

Making all options explicit then and erroring out on unknown input.

> > +# @FUNCTION: ecm_punt_bogus_dep
> > +# @USAGE: <prefix> <dependency>
> > +# @DESCRIPTION:
> > +# Removes a specified dependency from a find_package call with multiple
> > components.
> > +ecm_punt_bogus_dep() {
> > +	local prefix=${1}
> > +	local dep=${2}
> > +
> > +	if [[ ! -e "CMakeLists.txt" ]]; then
>
> Can this really ever happen in a valid use case?  Maybe it should be
> an error instead.

Even cmake-utils.eclass makes that check in cmake_comment_add_subdirectory and 
leaves the erroring out if the file's missing central to src_prepare(), I 
guess is why it was done that way.


Thanks for looking over it!

Regards,
Andreas