On Thu, Jan 03, 2013 at 02:01:03PM +0100, Pacho Ramos wrote:
> # Copyright 1999-2012 Gentoo Foundation
> # Distributed under the terms of the GNU General Public License v2
> # $Header: $
> 
> # @ECLASS: configuration-doc
> # @MAINTAINER:
> # Pacho Ramos <pacho@gentoo.org>
> # @AUTHOR:
> # Author: Pacho Ramos <pacho@gentoo.org>
> # @BLURB: An eclass for installing a CONFIGURATION doc file recording tips
> # shown via elog messages. With this eclass, those elog messages will only be
> # shown at first package installation and a file for later reviewing will be
> # installed under /usr/share/doc/${PF}
> # @DESCRIPTION:
> # An eclass for installing a CONFIGURATION doc file recording tips           
> # shown via elog messages. With this eclass, those elog messages will only be
> # shown at first package installation and a file for later reviewing will be
> # installed under /usr/share/doc/${PF}
> 
> if [[ ${___ECLASS_ONCE_CONFIGURATION_DOC} != "recur -_+^+_- spank" ]] ; then
> ___ECLASS_ONCE_CONFIGURATION_DOC="recur -_+^+_- spank"
> 
> inherit eutils
> 
> case "${EAPI:-0}" in
> 	0|1|2|3)
> 		die "Unsupported EAPI=${EAPI:-0} (too old) for ${ECLASS}"
> 		;;
> 	4|5)
> 		# EAPI>=4 is required for REPLACING_VERSIONS preventing us
> 		# from needing to export another pkg_preinst phase to save has_version
> 		# result. Also relies on EAPI >=4 default src_install phase.
> 		;;
> 	*)
> 		die "Unsupported EAPI=${EAPI} (unknown) for ${ECLASS}"
> 		;;
> esac

Why does it matter if the unsupported eapi is too old or unknown? I
think you can combine the first and last branches of this case
statement.

On the other hand, if you don't export phase functions, you don't have
to worry about EAPIS; you can just allow ebuild developers to call your
functions directly. This is what I see happening in your sample ebuild
below.

> EXPORT_FUNCTIONS src_install pkg_postinst

Drop this export_functions call. You don't need it based on what I see
in your ebuild.

> # @FUNCTION: configuration_create_doc
> # @DESCRIPTION:
> # Create doc file with CONFIGURATION_INSTRUCTIONS contents.
> # Usually called at src_install phase.

You can pass in the content as $1 instead of using a global variable for
it:

> configuration_create_doc() {
> 	debug-print-function ${FUNCNAME} "${@}"
> 
> 	if [[ -n "${1}" ]]; then
> 		eshopts_push
> 		set -f
> 		echo ${CONFIGURATION_INSTRUCTIONS} | fmt > CONFIGURATION

I would use "${T}"/CONFIGURATION here.

		echo "${1}" | fmt > "${T}"CONFIGURATION

> 		eshopts_pop
> 		dodoc CONFIGURATION

Again, "${T}"/CONFIGURATION

> 	fi
> }
> 
> # @FUNCTION: configuration_print_elog
> # @DESCRIPTION:
> # Print elog messages with CONFIGURATION_INSTRUCTIONS contents.
> # Usually called at pkg_postinst phase.
> configuration_print_elog() {
> 	debug-print-function ${FUNCNAME} "${@}"
> 
> 	if [[ -n "${CONFIGURATION_INSTRUCTIONS}" ]]; then

I would recommend using the CONFIGURATION file here, as follows:

if [ -f "${T}"/CONFIGURATION ]; then

> 		if ! [[ "${REPLACING_VERSIONS}" ]]; then

This is an issue if I want to add display some tips that have changed
between different versions of the software. I would propose not trying
to do this, but let the user call this function.

> 			eshopts_push
> 			set -f
> 			echo ${CONFIGURATION_INSTRUCTIONS} | fmt | while read -r ELINE; do elog "${ELINE}"; done

			cat "${T}"/CONFIGURATION | fmt | while read -r ELINE; do elog "${ELINE}"; done

> 			eshopts_pop
> 		fi
> 	fi
> }

> # @FUNCTION: configuration-doc_src_install
> # @DESCRIPTION:
> # Show elog messages from CONFIGURATION_INSTRUCTIONS variable, that will be
> # shared with /usr/share/doc/${PF}/CONFIGURATION content.
> configuration-doc_src_install() {
> 	debug-print-function ${FUNCNAME} "${@}"
> 
> 	default
> 	configuration_create_doc
> }
> 
> # @FUNCTION: configuration-doc_pkg_postinst
> # @DESCRIPTION:
> # Show elog messages from CONFIGURATION_INSTRUCTIONS variable, that will be
> # shared with /usr/share/doc/${PF}/CONFIGURATION content.
> configuration-doc_pkg_postinst() {
> 	debug-print-function ${FUNCNAME} "${@}"
> 	configuration_print_elog
> }
> 
> fi

These can go; your example ebuild doesn't need them since it calls the
eclass functions.


Below, I am showing how the ebuild would change based on my changes to
the eclass.

> # Copyright 1999-2012 Gentoo Foundation
> # Distributed under the terms of the GNU General Public License v2
> # $Header: /var/cvsroot/gentoo-x86/sys-power/acpid/acpid-2.0.17.ebuild,v 1.6 2012/11/25 18:59:25 armin76 Exp $
> 
> EAPI=4
> inherit configuration-doc systemd
> 
> DESCRIPTION="Daemon for Advanced Configuration and Power Interface"
> HOMEPAGE="http://tedfelix.com/linux/acpid-netlink.html"
> SRC_URI="http://tedfelix.com/linux/${P}.tar.xz"
> 
> LICENSE="GPL-2"
> SLOT="0"
> KEYWORDS="amd64 ia64 -ppc x86"
> IUSE="selinux"
> 
> RDEPEND="selinux? ( sec-policy/selinux-apm )"
> DEPEND="${RDEPEND}"
> 
> CONFIGURATION_INSTRUCTIONS="
> 	You may wish to read the Gentoo Linux Power Management Guide,
> 	which can be found online at:
> 	http://www.gentoo.org/doc/en/power-management-guide.xml"

Delete this and make it a local below if you want the variable,
otherwise pass it as a constant.

> src_configure() {
> 	econf --docdir=/usr/share/doc/${PF}
> }
> 
> src_install() {
> 	emake DESTDIR="${D}" install
> 
> 	newdoc kacpimon/README README.kacpimon
> 	dodoc -r samples
> 	rm -f "${D}"/usr/share/doc/${PF}/COPYING
> 
> 	exeinto /etc/acpi
> 	newexe "${FILESDIR}"/${PN}-1.0.6-default.sh default.sh
> 	insinto /etc/acpi/events
> 	newins "${FILESDIR}"/${PN}-1.0.4-default default
> 
> 	newinitd "${FILESDIR}"/${PN}-2.0.16-init.d ${PN}
> 	newconfd "${FILESDIR}"/${PN}-2.0.16-conf.d ${PN}
> 
> 	systemd_dounit "${FILESDIR}"/systemd/${PN}.{service,socket}

	local CONFIGURATION_INSTRUCTIONS="
	You may wish to read the Gentoo Linux Power Management Guide,
	which can be found online at:
	http://www.gentoo.org/doc/en/power-management-guide.xml"

> 	configuration_create_doc

 	configuration_create_doc "${CONFIGURATION_INSTRUCTIONS}"
> }
> 
> pkg_postinst() {
> 	configuration_print_elog
 
 The thing that would change here would be that you would have to make
 postinst smart enough to know when to call configuration_print_elog.
 Maybe you do it all the time like this, or maybe you do it the first
 time the package is installed, or you do it when you are upgrading from
 an older version and adding or removing some configuration information.

> 	# files/systemd/acpid.socket -> ListenStream=/run/acpid.socket
> 	mkdir -p "${ROOT}"/run
> 
> 	if ! grep -qs "^tmpfs.*/run " "${ROOT}"/proc/mounts ; then
> 		echo
> 		ewarn "You should reboot the system now to get /run mounted with tmpfs!"
> 	fi
> }

William