Re: [gentoo-dev] RFC: emboss.eclass as replacement for embassy.eclass

[Mike Frysinger <vapier@gentoo.org>]

[-- Attachment #1: Type: Text/Plain, Size: 4292 bytes --]

> # Copyright 1999-2004 Gentoo Foundation

s/2004/2011/

have you looked at the man page output for this to make sure the white spacing 
you're putting into the comments properly carries over ?

> # @ECLASS-VARIABLE: EBO_DESCRIPTION
> # @DESCRIPTION:
> # Should be set. Completes the describtion of the embassy module as follows:

i dont understand what this is trying to say (ignoring the spelling mistake in 
description)

> # EMBOSS integrated version of EBO_DESCRIPTION"

i see just one quote.  it needs a friend.

> # @ECLASS-VARIABLE: EBO_PATCH
> # @DEFAULT_UNSET
> # @DESCRIPTION: Specify the patch level of EMBOSS. Only available for the 
emboss ebuild.
> # The patch wil be fetch from:

wil -> will

> # @ECLASS-VARIABLE: NO_RECONF
> # @DEFAULT_UNSET
> # @DESCRIPTION:
> # Set this, if you do not want to have eautoreconf be run after patching.

please namespace all variables consistently

> # @ECLASS-VARIABLE: EBO_ECONF

EBO_EXTRA_ECONF is better for consistency sake too

> EAPI="4"

erp!

> HOMEPAGE="http://emboss.sourceforge.net"

could do with a trailing "/"

> IUSE="mysql pdf png postgres static-libs X "

no trailing space.  let the person appending the variable add the space:
	IUSE+=" moo"

> DOCS="AUTHORS ChangeLog NEWS README "

no need for that trailing space, and usually DOCS is left for end ebuilds isnt 
it ?  eclasses should append it.

> 
> if [[ ${PN} == "emboss" ]] ; then
> 	EBOV=${PV/_p*}
> 	DESCRIPTION="The European Molecular Biology Open Software Suite - A 
sequence analysis package"
> 	SRC_URI="ftp://emboss.open-bio.org/pub/EMBOSS/EMBOSS-${EBOV}.tar.gz"
> 	[[ -n ${EBO_PATCH} ]] && SRC_URI+=" ftp://${PN}.open-
bio.org/pub/EMBOSS/fixes/patches/patch-1-${EBO_PATCH}.gz -> ${P}.patch.gz"
> 	IUSE+="minimal "
> 	RDEPEND+="
> 		!sys-devel/cons
> 		"
> 	PDEPEND+="
> 		!minimal? (
> 				sci-biology/aaindex
> 				sci-biology/cutg
> 				sci-biology/prints
> 				sci-biology/prosite
> 				sci-biology/rebase
> 				sci-biology/transfac
> 				)
> 		"
> 	S=${WORKDIR}/EMBOSS-${EBOV}
> 	DOCS+="FAQ THANKS "

this doesnt make much sense to me.  shouldnt this all be in the emboss ebuild 
instead of the eclass ?

> 	# The EMBASSY package name, retrieved from the inheriting ebuild's name
> 	EN=${PN:8}

should there be some checking here to make sure it starts with "embassy-" or 
whatever you're expecting ?

> 	# The full name and version of the EMBASSY package (excluding the Gentoo
> 	# revision number)
> 	EF=$(echo ${EN} | tr "[:lower:]" "[:upper:]")-${PV}

ugh, but i guess we cant do much until we have newer bash

> 	EBO_DESCRIPTION=${EBO_DESCRIPTION:=${EN}}

tricky, but wrong.  i guess you actually want:
	: ${EBO_DESCRIPTION:=${EN}}

> 	DEPEND+=">=sci-biology/emboss-6.3.1_p4[mysql=,pdf=,png=,postgres=,static-
libs=,X=] "

space should be at the start of the string, not the end

> # @FUNCTION: emboss_src_prepare
> # @USAGE:
> # @RETURN:
> # @MAINTAINER:

i dont think these three tags make sense.  not even sure if this last one is 
even allowed.  best to just punt.

same goes for all the other funcs below.

> emboss_src_prepare() {
> 	[[ ${PN} == emboss ]] && [[ -n ${EBO_PATCH} ]] && epatch 
"${WORKDIR}"/${P}.patch

guess that would be moved to the emboss ebuild

> 	[[ -f "${FILESDIR}"/${PF}.patch ]] && epatch "${FILESDIR}"/${PF}.patch

technically, no need to quote FILESDIR here as the [[...]] takes care of it

> #  $(use_with png pngdriver "${EPREFIX}/usr")
> #  $(use_with pdf hpdf "${EPREFIX}/usr")
> #  $(use_with mysql mysql "${EPREFIX}/usr/bin/mysql_config")
> #  $(use_with postgres postgresql "${EPREFIX}/usr/bin/pg_config")

do you really need full paths here ?  are the configure scripts that screwed 
up ?  ideally libs would be found via .pc files.

> #  $(use_enable amd64 64)

oh god it burns so bad when i poo

> emboss_src_install() {
> 	default
> 	mv "${ED}"/usr/share/EMBOSS/doc/* "${ED}"/usr/share/doc/${PF}/
> 	rm -rf "${ED}"/usr/share/EMBOSS/doc

does the configure script respect --docdir= ?  then you wouldnt need this 
clean up logic ...

> [[ ${PN} == embassy ]] || EXPORT_FUNCTIONS src_prepare src_configure 
src_install

this $PN logic should get punted
-mike

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