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

["Michał Górny" <mgorny@gentoo.org>]

[-- Attachment #1: Type: text/plain, Size: 14474 bytes --]

On Tue, 2019-11-05 at 00:30 +0100, Andreas Sturmlechner wrote:
> Support eclass for packages that use KDE extra-cmake-modules.
> 
> This eclass is intended to streamline the creation of ebuilds for packages
> that follow KDE upstream packaging conventions. It's primarily intended for
> the three upstream release groups (Frameworks, Plasma, Applications) but
> is also for any package that follows similar conventions.
> 
> This eclass unconditionally inherits cmake-utils.eclass and all its public
> variables and helper functions (not phase functions) may be considered as part
> of this eclass's API.
> 
> When used together with kde.org.eclass this will replace kde5.eclass and
> kde5-functions.eclass, most of the latter is becoming obsolete.
> 
> --- /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.

> @@ -0,0 +1,549 @@
> +# Copyright 1999-2019 Gentoo Authors
> +# Distributed under the terms of the GNU General Public License v2
> +
> +# @ECLASS: ecm-utils.eclass
> +# @MAINTAINER:
> +# kde@gentoo.org
> +# @SUPPORTED_EAPIS: 7
> +# @BLURB: Support eclass for packages that use KDE extra-cmake-modules.
> +# @DESCRIPTION:
> +# This eclass is intended to streamline the creation of ebuilds for packages
> +# that follow KDE upstream packaging conventions. It's primarily intended for
> +# the three upstream release groups (Frameworks, Plasma, Applications) but
> +# is also for any package that follows similar conventions.
> +#
> +# This eclass unconditionally inherits cmake-utils.eclass and all its public
> +# variables and helper functions (not phase functions) may be considered as 
> part
> +# of this eclass's API.
> +#
> +# This eclass's phase functions are not intended to be mixed and matched, so 
> if
> +# any phase functions are overridden the version here should also be called.
> +
> +if [[ -z ${_ECM_UTILS_ECLASS} ]]; then
> +_ECM_UTILS_ECLASS=1
> +
> +# @ECLASS-VARIABLE: VIRTUALX_REQUIRED
> +# @DESCRIPTION:
> +# For proper description see virtualx.eclass manpage.
> +# Here we redefine default value to be manual, if your package needs virtualx
> +# for tests you should proceed with setting VIRTUALX_REQUIRED=test.
> +: ${VIRTUALX_REQUIRED:=manual}
> +
> +inherit cmake-utils flag-o-matic toolchain-funcs virtualx xdg
> +
> +case ${EAPI} in
> +	7) ;;
> +	*) die "EAPI=${EAPI:-0} is not supported" ;;
> +esac
> +
> +if [[ -v KDE_GCC_MINIMAL ]]; then
> +	EXPORT_FUNCTIONS pkg_pretend
> +fi
> +
> +EXPORT_FUNCTIONS pkg_setup src_prepare src_configure src_test pkg_preinst 
> pkg_postinst pkg_postrm
> +
> +# @ECLASS-VARIABLE: ECM_KDEINSTALLDIRS
> +# @DESCRIPTION:
> +# If set to "false", do nothing.
> +# For any other value, assume the package is using KDEInstallDirs macro and 
> switch
> +# KDE_INSTALL_USE_QT_SYS_PATHS to ON.
> +: ${ECM_KDEINSTALLDIRS:=true}
> +
> +# @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.

> +
> +# @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.

> +
> +# @ECLASS-VARIABLE: ECM_DESIGNERPLUGIN
> +# @DESCRIPTION:
> +# If set to "false", do nothing.
> +# Otherwise, add "designer" to IUSE to toggle build of designer plugins
> +# and add the necessary BDEPEND.
> +: ${ECM_DESIGNERPLUGIN:=false}
> +
> +# @ECLASS-VARIABLE: ECM_EXAMPLES
> +# @DESCRIPTION:
> +# If set to "false", unconditionally ignore a top-level examples 
> subdirectory.
> +# Otherwise, add "examples" to IUSE to toggle adding that subdirectory.
> +: ${ECM_EXAMPLES:=false}
> +
> +# @ECLASS-VARIABLE: ECM_HANDBOOK
> +# @DESCRIPTION:
> +# If set to "false", do nothing.
> +# Otherwise, add "+handbook" to IUSE, add the appropriate dependency, and let
> +# KF5DocTools generate and install the handbook from docbook file(s) found in
> +# ECM_HANDBOOK_DIR. However if USE handbook is disabled, disable build of
> +# ECM_HANDBOOK_DIR in CMakeLists.txt.
> +# If set to "optional", config with -
> DCMAKE_DISABLE_FIND_PACKAGE_KF5DocTools=ON
> +# when USE=!handbook. In case package requires KF5KDELibs4Support, see next:
> +# If set to "forceoptional", remove a KF5DocTools dependency from the root
> +# CMakeLists.txt in addition to the above.
> +: ${ECM_HANDBOOK:=false}
> +
> +# @ECLASS-VARIABLE: ECM_HANDBOOK_DIR
> +# @DESCRIPTION:
> +# Specifies the directory containing the docbook file(s) relative to ${S} to 
> be
> +# processed by KF5DocTools (kdoctools_install) if not the default.
> +: ${ECM_HANDBOOK_DIR:=doc}
> +
> +# @ECLASS-VARIABLE: ECM_PO_DIRS
> +# @DESCRIPTION:
> +# Specifies the top-level directories of l10n files relative to ${S} to be
> +# processed by KF5I18n (ki18n_install) if not the default. If IUSE nls exists
> +# and is disabled then disable build of these directories in CMakeLists.txt.
> +: ${ECM_PO_DIRS:="po poqm"}
> +
> +# @ECLASS-VARIABLE: ECM_QTHELP
> +# @DESCRIPTION:
> +# If set to "false", do nothing.
> +# Otherwise, add "doc" to IUSE, add the appropriate dependency, generate
> +# and install Qt compressed help files with -DBUILD_QCH=ON when USE=doc.
> +if [[ ${CATEGORY} = kde-frameworks ]]; then
> +	: ${ECM_QTHELP:=true}
> +fi
> +: ${ECM_QTHELP:=false}

Again, I believe this and below won't be processed by eclassdoc
correctly.

> +
> +# @ECLASS-VARIABLE: ECM_TEST
> +# @DESCRIPTION:
> +# If set to "false", do nothing.
> +# For any other value, add test to IUSE and add a dependency on dev-qt/
> qttest:5.
> +# If set to "optional", configure with -
> DCMAKE_DISABLE_FIND_PACKAGE_Qt5Test=ON
> +# when USE=!test.
> +# If set to "forceoptional", remove a Qt5Test dependency and comment test
> +# subdirs from the root CMakeLists.txt in addition to the above.
> +# If set to "forceoptional-recursive", remove Qt5Test dependencies and make
> +# autotest(s), unittest(s) and test(s) subdirs from *any* CMakeLists.txt in $
> {S}
> +# and below conditional on BUILD_TESTING. This is always meant as a short-
> term
> +# fix and creates ${T}/${P}-tests-optional.patch to refine and submit 
> upstream.
> +if [[ ${CATEGORY} = kde-frameworks ]]; then
> +	: ${ECM_TEST:=true}
> +fi
> +: ${ECM_TEST:=false}
> +
> +case ${ECM_NONGUI} in

There's a @PRE_INHERIT thingie to indicate variables that need to be set
before 'inherit' to work correctly.

> +	false)
> +		# gui applications need breeze or oxygen icons for basic iconset, 
> bug #564838
> +		RDEPEND+=" || ( kde-frameworks/breeze-icons:5 kde-frameworks/
> oxygen-icons:* )"
> +		;;
> +	*)	;;
> +esac
> +
> +case ${ECM_DEBUG} in
> +	false)	;;
> +	*)
> +		IUSE+=" debug"
> +		;;
> +esac
> +
> +case ${ECM_DESIGNERPLUGIN} in
> +	false)	;;
> +	*)
> +		IUSE+=" designer"
> +		BDEPEND+=" designer? ( dev-qt/designer:5 )"
> +		;;
> +esac
> +
> +# @ECLASS-VARIABLE: KDE_DESIGNERPLUGIN
> +# @DESCRIPTION:
> +# If set to "false", do nothing.
> +# Otherwise, add "designer" to IUSE to toggle build of designer plugins
> +# and add the necessary BDEPEND.
> +# TODO: drop after KDE Applications 19.08.3 removal
> +: ${KDE_DESIGNERPLUGIN:=false}
> +case ${KDE_DESIGNERPLUGIN} in
> +	false)	;;
> +	*)
> +		IUSE+=" designer"
> +		BDEPEND+=" designer? ( kde-frameworks/kdesignerplugin:5 )"
> +		;;
> +esac
> +
> +case ${ECM_EXAMPLES} in
> +	false)	;;
> +	*)
> +		IUSE+=" examples"
> +		;;
> +esac
> +
> +case ${ECM_HANDBOOK} in
> +	false)	;;
> +	*)
> +		IUSE+=" +handbook"
> +		BDEPEND+=" handbook? ( kde-frameworks/kdoctools:5 )"
> +		;;
> +esac
> +
> +case ${ECM_QTHELP} in
> +	false)	;;
> +	*)
> +		IUSE+=" doc"
> +		COMMONDEPEND+=" doc? ( dev-qt/qt-docs:5 )"
> +		BDEPEND+=" doc? (
> +			>=app-doc/doxygen-1.8.13-r1
> +			dev-qt/qthelp:5
> +		)"
> +		;;
> +esac
> +
> +case ${ECM_TEST} in
> +	false)	;;
> +	*)
> +		IUSE+=" test"
> +		DEPEND+=" test? ( dev-qt/qttest:5 )"
> +		RESTRICT+=" !test? ( test )"
> +		;;
> +esac
> +
> +BDEPEND+=" >=kde-frameworks/extra-cmake-modules-5.60.0"
> +RDEPEND+=" >=kde-frameworks/kf-env-4"
> +COMMONDEPEND+=" dev-qt/qtcore:5"
> +
> +DEPEND+=" ${COMMONDEPEND}"
> +RDEPEND+=" ${COMMONDEPEND}"
> +unset COMMONDEPEND
> +
> +# @ECLASS-VARIABLE: KDE_GCC_MINIMAL
> +# @DEFAULT_UNSET
> +# @DESCRIPTION:
> +# Minimum version of active GCC to require. This is checked in
> +# ecm-utils_pkg_pretend and ecm-utils_pkg_setup.
> +
> +# @FUNCTION: _check_gcc_version

Would be nice to prefix it with _ecm*.

> +# @INTERNAL
> +# @DESCRIPTION:
> +# Determine if the current GCC version is acceptable, otherwise die.
> +_check_gcc_version() {
> +	if [[ ${MERGE_TYPE} != binary && -v KDE_GCC_MINIMAL ]] && tc-is-gcc; 
> then
> +
> +		local version=$(gcc-version)
> +		local major=${version%.*}
> +		local minor=${version#*.}
> +		local min_major=${KDE_GCC_MINIMAL%.*}
> +		local min_minor=${KDE_GCC_MINIMAL#*.}
> +
> +		debug-print "GCC version check activated"
> +		debug-print "Version detected:"
> +		debug-print "	- Full: ${version}"
> +		debug-print "	- Major: ${major}"
> +		debug-print "	- Minor: ${minor}"
> +		debug-print "Version required:"
> +		debug-print "	- Major: ${min_major}"
> +		debug-print "	- Minor: ${min_minor}"
> +
> +		[[ ${major} -lt ${min_major} ]] || \
> +				( [[ ${major} -eq ${min_major} && ${minor} -lt $
> {min_minor} ]] ) \
> +			&& die "Sorry, but gcc-${KDE_GCC_MINIMAL} or later is 
> required for this package (found ${version})."

Why not use ver_cmp from EAPI 7?

> +	fi
> +}
> +
> +# @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.

> +		return
> +	fi
> +
> +	pcregrep -Mni "(?s)find_package\s*\(\s*${prefix}[^)]*?${dep}.*?\)" 
> CMakeLists.txt > "${T}/bogus${dep}"
> +
> +	# pcregrep returns non-zero on no matches/error
> +	if [[ $? != 0 ]] ; then

-ne

> +		return
> +	fi
> +
> +	local length=$(wc -l "${T}/bogus${dep}" | cut -d " " -f 1)

'wc -l < ...' and you won't have to cut.

> +	local first=$(head -n 1 "${T}/bogus${dep}" | cut -d ":" -f 1)
> +	local last=$(( ${length} + ${first} - 1))

$(( length + first - 1 ))

> +
> +	sed -e "${first},${last}s/${dep}//" -i CMakeLists.txt || die
> +
> +	if [[ ${length} = 1 ]] ; then

-eq

> +		sed -e "/find_package\s*(\s*${prefix}\(\s\+\(REQUIRED\|CONFIG\|
> COMPONENTS\|\${[A-Z0-9_]*}\)\)\+\s*)/Is/^/# removed by kde5-functions.eclass - 
> /" -i CMakeLists.txt || die
> +	fi
> +}
> +
> +# @FUNCTION: ecm-utils_pkg_pretend
> +# @DESCRIPTION:
> +# Checks if the active compiler meets the minimum version requirements.
> +# phase function is only exported if KDE_GCC_MINIMAL is defined.
> +ecm-utils_pkg_pretend() {
> +	debug-print-function ${FUNCNAME} "$@"
> +	_check_gcc_version
> +}
> +
> +# @FUNCTION: ecm-utils_pkg_setup
> +# @DESCRIPTION:
> +# Checks if the active compiler meets the minimum version requirements.
> +ecm-utils_pkg_setup() {
> +	debug-print-function ${FUNCNAME} "$@"
> +	_check_gcc_version
> +}
> +
> +# @FUNCTION: ecm-utils_src_prepare
> +# @DESCRIPTION:
> +# Wrapper for cmake-utils_src_prepare with lots of extra logic for magic
> +# handling of linguas, tests, handbook etc.
> +ecm-utils_src_prepare() {
> +	debug-print-function ${FUNCNAME} "$@"
> +
> +	cmake-utils_src_prepare
> +
> +	# only build examples when required
> +	if ! { in_iuse examples && use examples; } ; then
> +		cmake_comment_add_subdirectory examples
> +	fi
> +
> +	# only enable handbook when required
> +	if in_iuse handbook && ! use handbook ; then
> +		cmake_comment_add_subdirectory ${ECM_HANDBOOK_DIR}
> +
> +		if [[ ${ECM_HANDBOOK} = forceoptional ]] ; then
> +			punt_bogus_dep KF5 DocTools
> +			sed -i -e "/kdoctools_install/ s/^/#DONT/" CMakeLists.txt || 
> die
> +		fi
> +	fi
> +
> +	# drop translations when nls is not wanted
> +	if in_iuse nls && ! use nls ; then
> +		local po
> +		for po in ${ECM_PO_DIRS}; do
> +			if [[ -d ${po} ]] ; then

Do you have a valid case for ${po} not being a directory?  If not,
I think the whole thing could be simplified to:

rm -rf ${ECM_PO_DIRS}

> +				rm -r ${po} || die
> +			fi
> +		done
> +	fi
> +
> +	# enable only the requested translations when required
> +	# always install unconditionally for kconfigwidgets - if you use 
> language
> +	# X as system language, and there is a combobox with language names, the
> +	# translated language name for language Y is taken from
> +	# /usr/share/locale/Y/kf5_entry.desktop
> +	if [[ -v LINGUAS && ${PN} != kconfigwidgets ]] ; then
> +		local po
> +		for po in ${ECM_PO_DIRS}; do
> +		if [[ -d ${po} ]] ; then

Missing indent?

> +			pushd ${po} > /dev/null || die
> +			local lang
> +			for lang in *; do
> +				if [[ -e ${lang} ]] && ! has ${lang/.po/} ${LINGUAS} ; 
> then
> +					case ${lang} in
> +						cmake_modules | \
> +						CMakeLists.txt | \
> +						${PN}.pot)	;;
> +						*) rm -r ${lang} || die	;;
> +					esac
> +					if [[ -e CMakeLists.txt ]] ; then
> +						cmake_comment_add_subdirectory ${lang}
> +						sed -e "/add_subdirectory([[:space:]]*$
> {lang}\/.*[[:space:]]*)/d" \
> +							-i CMakeLists.txt || die
> +					fi
> +				fi
> +			done
> +			popd > /dev/null || die
> +		fi
> +		done
> +	fi
> +

Overall, I find this whole thing disgusting and fragile but up to you.

> 

-- 
Best regards,
Michał Górny


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 618 bytes --]