Re: [gentoo-dev] [RFC] New eclass: mate

[Michał Górny]

Dnia 9 czerwca 2016 14:19:43 CEST, NP-Hardass <NP-Hardass@g.o> napisał(a):
>Greetings all,
>
>Sorry for the delay, had lots of recurrent hardware issues the last
>month or so.
>I will be adding this to the MATE project repo after I get your
>feedback, and then into Gentoo repo after I've had some users test out
>the new packages/eclass.
>
>Just a reminder/summary:
>There are 40-50 ebuilds in the MATE desktop environment, with a fair
>bit
>of overlap in the ebuild code, so I thought it best to create an eclass
>to handle that.  MATE is a fork of GNOME 2 so I used the gnome.org
>eclass as a reference for the mate-desktop.org eclass.  Additionally,
>there is much in MATE that is still very much in line with the gnome2
>eclass.  Rather than having to edit 40-50 ebuilds if we become
>divergent, I thought it more purdent to just create a separete mate_*
>namespace. For functions that are currently equivalent, my phase
>functions are stubs to the gnome2 phase functions.
>
>
>Thanks for taking the time to look these over and give your feedback.
>(Also, apologies for the thrown together email, I was having trouble
>getting git-send working to the mailing list)
>
>--
>NP-Hardass
>
>###############################################################################
>mate-desktop.org.eclass
>###############################################################################
>
>
># Copyright 1999-2016 Gentoo Foundation
># Distributed under the terms of the GNU General Public License v2
># $Id$
>
># @ECLASS: mate-desktop.org.eclass
># @MAINTAINER:
># mate@g.o
># @AUTHOR:
># Authors: NP-Hardass <NP-Hardass@g.o> based upon the gnome.org
>eclass.
># @BLURB: Helper eclass for mate-desktop.org hosted archives
># @DESCRIPTION:
># Provide a default SRC_URI and EGIT_REPO_URI for MATE packages as well
>as
># exporting some useful values like the MATE_BRANCH
>
># Old EAPIs are banned.
>case "${EAPI:-0}" in
>	5|6) ;;
>	*) die "EAPI=${EAPI} is not supported" ;;
>esac
>
>if [[ ${PV} == 9999 ]]; then
>	inherit git-r3
>else
>	inherit versionator
>fi
>
># Ensure availibility of xz-utils on old EAPIs
>if [[ "${EAPI:-0}" -lt "6" ]]; then

EAPI is not a number and must not be treated like a number.

>	DEPEND="app-arch/xz-utils"
>fi
>
># @ECLASS-VARIABLE: MATE_TARBALL_SUFFIX
># @INTERNAL
># @DESCRIPTION:
># All projects hosted on mate-desktop.org provide tarballs as tar.xz.
># Undefined in live ebuilds.
>[[ ${PV} != 9999 ]] && : ${MATE_TARBALL_SUFFIX:="xz"}

You should ask upstream to supply .tar.lz instead, so they don't harm their users.

>
># @ECLASS-VARIABLE: MATE_DESKTOP_ORG_PN
># @DESCRIPTION:
># Name of the package as hosted on mate-desktop.org.
># Leave unset if package name matches PN.
>: ${MATE_DESKTOP_ORG_PN:=$PN}
>
># @ECLASS-VARIABLE: MATE_DESKTOP_ORG_PV
># @DESCRIPTION:
># Package version string as listed on mate-desktop.org.
># Leave unset if package version string matches PV.
>: ${MATE_DESKTOP_ORG_PV:=$PV}
>
># @ECLASS-VARIABLE: MATE_BRANCH
># @DESCRIPTION:
># Major and minor numbers of the version number, unless live.
># If live ebuild, will be set to '9999'.
>if [[ ${PV} == 9999 ]]; then
>	: ${MATE_BRANCH:=9999}
>else
>	: ${MATE_BRANCH:=$(get_version_component_range 1-2)}
>fi

Unless I'm missing something, the 1-2 range will return 9999 in live version as well. Unless you are trying to avoid the inherit in the live ebuild -- then I don't think it's worth the extra code.

>
># Set SRC_URI or EGIT_REPO_URI based on whether live
>if [[ ${PV} == 9999 ]]; then
>	EGIT_REPO_URI="
>		https://github.com/mate-desktop/${MATE_DESKTOP_ORG_PN}.git
>		git://github.com/mate-desktop/${MATE_DESKTOP_ORG_PN}.git
>		http://github.com/mate-desktop/${MATE_DESKTOP_ORG_PN}.git

Does github actually support http?

>	"
>	SRC_URI=""
>else
>	SRC_URI="http://pub.mate-desktop.org/releases/${MATE_BRANCH}/${MATE_DESKTOP_ORG_PN}-${MATE_DESKTOP_ORG_PV}.tar.${MATE_TARBALL_SUFFIX}"
>fi
>
># Set HOMEPAGE for all ebuilds
>HOMEPAGE="http://mate-desktop.org"
>
># Set default SLOT for all ebuilds
>SLOT="0"

Don't do that. Ebuilds without explicit SLOT are less readable.

>
>
>###############################################################################
>mate.eclass
>###############################################################################
>
>
># Copyright 1999-2016 Gentoo Foundation
># Distributed under the terms of the GNU General Public License v2
># $Id$
>
># @ECLASS: mate.eclass
># @MAINTAINER:
># mate@g.o
># @AUTHOR:
># Authors: NP-Hardass <NP-Hardass@g.o> based upon the gnome2
># and autotools-utils eclasses
># @BLURB: Provides phases for MATE based packages.
># @DESCRIPTION:
># Exports portage base functions used by ebuilds written for packages
>using the
># MATE framework. Occassionally acts as a wrapper to gnome2 due to the
># fact that MATE is a GNOME fork. For additional functions, see
>gnome2-utils.eclass.
>
># Check EAPI only
>case "${EAPI:-0}" in
>	5|6) ;;
>	*) die "EAPI=${EAPI} is not supported" ;;
>esac
>
># Inherit happens below after declaration of GNOME2_LA_PUNT
>
># @ECLASS-VARIABLE: MATE_LA_PUNT
># @DESCRIPTION:
># Available values for MATE_LA_PUNT:
># - "no": will not clean any .la files
># - "yes": will run prune_libtool_files --modules
># - If it is not set, it will run prune_libtool_files
># MATE_LA_PUNT is a stub to GNOME2_LA_PUNT
>GNOME2_LA_PUNT=${MATE_LA_PUNT:-""}
>
>inherit gnome2 autotools mate-desktop.org
>
>case "${EAPI:-0}" in
>	5|6)
>		EXPORT_FUNCTIONS src_prepare src_configure src_install pkg_preinst
>pkg_postinst pkg_postrm
>		;;
>	*) die "EAPI=${EAPI} is not supported" ;;

It is already dead after the first check.

>esac
>
># Autotools requires our MATE m4 files
>DEPEND=">=mate-base/mate-common-${MATE_BRANCH}"
>
># @FUNCTION: python_cond_func_wrap
># @DESCRIPTION: Wraps a function for conditional python use, to run for
>each
># python implementation in the build directory.
>python_cond_func_wrap() {
>	if use python; then
>		python_foreach_impl run_in_build_dir "$@"
>	else
>		$@
>	fi
>}
>
># @ECLASS-VARIABLE: FORCE_AUTORECONF

Namespace needed.

># @DESCRIPTION:
># Available values for FORCE_AUTORECONF:
># - true: will always run eautoreconf
># - false: will default to automatic detect
># - If it is not set, it will default to false
>FORCE_AUTORECONF=${FORCE_AUTORECONF:-""}
>
># @FUNCTION: ematedocize
># @DESCRIPTION: A wrapper around mate-doc-common
>ematedocize() {
>	ebegin "Running mate-doc-common --copy"
>		mate-doc-common --copy || die
>	eend $?
>}
>
># @FUNCTION: want_mate_doc
># @DESCRIPTION:
># Returns true/false based on whether eautoreconf should call
># ematedocize
>want_mate_doc() {
>	grep -q USE_COMMON_DOC_BUILD autogen.sh

Did you really intend to match autogen.sh and not configure.ac or alike?

>}
>
># @FUNCTION: mate_src_prepare
># @DESCRIPTION:
># Call gnome2_src_prepare to handle environment setup and patching,
>then
># call eautoreconf if necessary
>mate_src_prepare() {
>	debug-print-function ${FUNCNAME} "$@"
>
>	local force_autoreconf=${FORCE_AUTORECONF:-false}
>	[[ ${PV} == 9999 ]] && force_autoreconf=true
>
>	gen_chksum() {
>		find '(' -name 'Makefile.am' \
>			-o -name 'configure.ac' \
>			-o -name 'configure.in' ')' \
>			-exec cksum {} + | sort -k2
>	}
>
>	local chksum=$(gen_chksum)
>
>	gnome2_src_prepare "$@"
>
>	if ${force_autoreconf}

Don't execute random user-provided data.

> || [[ ${chksum} != $(gen_chksum) ]]; then
>		[[ want_mate_doc ]] && ematedocize

Missing ${}.

>		eautoreconf
>	fi
>}
>
># @FUNCTION: mate_src_configure
># @DESCRIPTION:
># MATE specific configure handling
># Stub to gnome2_src_configure()
>mate_src_configure() {
>	gnome2_src_configure "$@"
>}
>
># @FUNCTION: mate_src_install
># @DESCRIPTION:
># MATE specific install. Stub to gnome2_src_install
>mate_src_install() {
>	gnome2_src_install "$@"
>}
>
># @FUNCTION: mate_pkg_preinst
># @DESCRIPTION:
># Finds Icons, GConf and GSettings schemas for later handling in
>pkg_postinst
># Stub to gnome2_pkg_preinst
>mate_pkg_preinst() {
>	gnome2_pkg_preinst "$@"
>}
>
># @FUNCTION: mate_pkg_postinst
># @DESCRIPTION:
># Handle scrollkeeper, GConf, GSettings, Icons, desktop and mime
># database updates.
># Stub to gnome2_pkg_postinst
>mate_pkg_postinst() {
>	gnome2_pkg_postinst "$@"
>}
>
># @FUNCTION: mate_pkg_postrm
># @DESCRIPTION:
># Handle scrollkeeper, GSettings, Icons, desktop and mime database
>updates.
># Stub to gnome2_pkg_postrm
>mate_pkg_postrm() {
>	gnome2_pkg_postrm "$@"
>}


-- 
Best regards,
Michał Górny (by phone)