El jue, 03-01-2013 a las 14:29 -0600, William Hubbs escribió:
[...]
> > 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.
> 

Well, I saw it in other eclasses and thought the distinction was
preferred, but have no problems on changing it

> 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.
> 

It exports src_install and pkg_postinst, at least the second one could
be used in other ebuilds that are only setting a package specific one
for showing this kind of messages (for example bumblebee ebuilds). 

For src_install case, probably some other ebuilds could benefit, but
didn't find them when I fast checked yesterday :/. Its purpose is to
ensure doc CONFIGURATION file is created, the ideal would be to be able
to only add it to create it and leave other eclasses or eapi defaults to
handle the rest of src_install, but I think it's not possible :| (and,
then, it will behave like eapi >= 4 default src_install phase + creating
the file)


Anyway, EAPI >= 4 is still needed for REPLACING_VERSIONS usage, that
allows to simplify things preventing us from needing to set pkg_preinst
also

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

They can be needed if ebuild doesn't need to have a custom
src_install/pkg_postinst only for this purposes

> > # @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:
> 

But, how to share CONFIGURATION_INSTRUCTIONS contents then for
create_doc and print_elog? The idea is to share the same message, and
using global variable for this purpose is already done with
kernel-2.eclass (I noticed it when doing last tuxonice-sources bumps)


> > 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
> 

I have no problem but, what is its advantage? (to remember it more
easily next time). Thanks :)

> > 	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

That is interesting as ensures file was created, but, will it be still
there at pkg_postinst phase?

> 
> > 		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.
> 

This is intentional because the kind of messages that are always the
same and, then, need to only be shown with elog first time the package
is installed need this checking. From packages I have seen, those
messages that are related with "updating from version XXX" should still
be handled manually.

The eclass could, of course, be handled to also handle this cases, but I
would like to keep the "automatic" way of it showing the message only
first time package is installed.

[...]
> > 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.

sorry for the ignorance but, how should I pass it as a constant? Using
"readonly"? Also, I am not using here any external tool and, then,
global scope shouldn't be discouraged

> 
> > 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.
> 

I thought about this but, for now, I preferred to only cover the
simplest case for handling messages that are always shown between
versions and can be easily standardized. For the other kind of messages,
I think it would be much more difficult to standardize but I think could
be added later when we start to migrate ebuilds and see the needings.

Best regards