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

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

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

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@gentoo.org>
> # @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

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