Re: [gentoo-dev] new eclass: meson.eclass for the meson build system

[Michał Górny]

On śro, 2017-05-03 at 22:11 -0500, William Hubbs wrote:
> # Copyright 2017 Gentoo Foundation
> # Distributed under the terms of the GNU General Public License v2
>
> # @ECLASS: meson.eclass
> # @MAINTAINER:
> # William Hubbs <williamh@g.o>
> # @BLURB: common ebuild functions for meson-based packages
> # @DESCRIPTION:
> #
> # @EXAMPLE:
> # Typical ebuild using meson.eclass:
> #
> # @CODE
> # EAPI=6
> #
> # inherit meson
> #
> # ...
> #
> # src_configure() {
> # 	local mymesonargs=(
> # 		-Dqt4=$(usex qt4 true false)
> # 		-Dthreads=$(usex threads true false)
> # 		-Dtiff=$(usex tiff true false)
> # 	)
> # 	meson_src_configure
> # }
> #
> # ...
> #
> # @CODE
>
> case ${EAPI:-0} in
> 	6) ;;
> 	*) die "EAPI=${EAPI} is not supported" ;;
> esac
>
> EXPORT_FUNCTIONS src_configure src_compile src_install src_test

It's usually better to order them in run order, i.e. test before
install.

>
> if [[ -z ${_MESON} ]]; then
> _MESON=1

_MESON_ECLASS would fit the common naming (and reduce risk of accidental
collisions).

>
> 	inherit ninja-utils toolchain-funcs
>
> DEPEND=">=dev-util/meson-0.39.1
> 	>=dev-util/ninja-1.7.2"
>
> # @ECLASS-VARIABLE: BUILD_DIR
> # @DEFAULT_UNSET
> # @DESCRIPTION:
> # Build directory, location where all generated files should be placed.
> # If this isn't set, it defaults to ${WORKDIR}/${P}_build.
>
> # @ECLASS-VARIABLE: EMESON_SOURCE
> # @DEFAULT_UNSET
> # @DESCRIPTION:
> # The location of the source files for the project;this is the source
> # directory to pass to meson.
> # If this isn't set, it defaults to ${S}
>
> # @VARIABLE: mymesonargs
> # @DEFAULT_UNSET
> # @DESCRIPTION:
> # Optional meson arguments as Bash array; this should be defined before
> # calling meson_src_configure.
>
> # create a cross file for meson
> # fixme: populate one instead of just touching it
> _create_cross_file() {

You definitely want to prefix those functions with '_meson', otherwise
there's high risk of collisions.

> 	touch "${T}"/meson.crossfile
> }
>
> # set the build directory
> _set_build_dir(){
> 		BUILD_DIR="${BUILD_DIR:-${WORKDIR}/${P}-build}"

: "${BUILD_DIR:=${WORKDIR}/${P}-build}"

Plus mis-indent. Plus the doc before said it's using underscore between
${P} and build, so you may want to unify that.

> }
>
> # @FUNCTION: meson_src_configure
> # @DESCRIPTION:
> # this is the meson_src_configure function
> meson_src_configure() {
> 	debug-print-function ${FUNCNAME} "$@"
>
> 	# Common args
> 	local mesonargs=(
> 		--buildtype plain
> 		--libdir "$(get_libdir)"
> 		--localstatedir "${EPREFIX}/var/lib"
> 		--prefix "${EPREFIX}"/usr
> 		--sysconfdir "${EPREFIX}/etc"
> 		)
>
> 	if tc-is-cross-compiler; then
> 		_create_cross_file || die "unable to write meson cross file"
> 		mesonargs+=(
> 			--cross-file "${T}"/meson.crossfile
> 		)
> 	fi
>
> 	# Append additional arguments from ebuild
> 	mesonargs+=("${mymesonargs[@]}")
>
> 	_set_build_dir
> 	set -- meson "${mesonargs[@]}" "$@" \
> 		"${EMESON_SOURCE:-${S}}"  "${BUILD_DIR}"

You've got double space between the paths.

> 	echo "$@"
> 	"$@" || die
> }
>
> # @FUNCTION: meson_src_compile
> # @DESCRIPTION:
> # This is the meson_src_compile function.
> meson_src_compile() {
> 	debug-print-function ${FUNCNAME} "$@"
>
> 	eninja -v -C "${BUILD_DIR}" || die

eninja dies on its own in EAPI 4+.

> }
>
> # @FUNCTION: meson_src_test
> # @DESCRIPTION:
> # this is the meson_src_test function.
> meson_src_test() {
> 	debug-print-function ${FUNCNAME} "$@"
>
> 	eninja -C "${BUILD_DIR}" test || die

Why no -v here?

> }
>
> # @FUNCTION: meson_src_install
> # @DESCRIPTION:
> # this is the meson_src_install function.
> meson_src_install() {
> 	debug-print-function ${FUNCNAME} "$@"
>
> 	DESTDIR="${ED}" eninja -C "${BUILD_DIR}" install || die

Wouldn't this result in double EPREFIX, since you're passing EPREFIX to
configure already?

> }
>
> fi

-- 
Best regards,
Michał Górny