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
