Re: [gentoo-dev] Announcement of The G Palmtop Environment ebuilds

[Angelo Arrifano <miknix@gentoo.org>]

On Qua, 2009-02-04 at 18:36 +0200, Petteri Räty wrote:
> Angelo Arrifano wrote:
> > 
> > # Copyright 2008 Gentoo Foundation
> > # Distributed under the terms of the GNU General Public License v2
> > # $Header: $
> > #
> > # Authors:
> > # Rene Wagner <rw@handhelds.org>
> > # Ned Ludd <solar@gentoo.org>
> > # Angelo Arrifano <miknix@gentoo.org>
> > 
> 
> Should use eclass-manpages syntax.

Thanks, fixed on next revision
> 
> > 
> > # GPE ECLASS
> > #GPECONF=""             # extra configure opts passed to econf
> > ELTCONF=""              # extra options passed to elibtoolize
> > DOCS=""                 # documentation files to be installed with dodoc
> > 
> 
> If other eclass that comes before in the inherit hierarchy and sets
> DOCS, do we want to override it?

Yes, we want. If we will make dodoc die by default like you proposed
below, DOCS must be explicitly set by each ebuild sourcing any common
DOC provided by the eclass.
> 
> > [ -z "${GPE_MIRROR}" ] && export GPE_MIRROR="http://gpe.linuxtogo.org/download/source"
> > 
> > [ -z "${GPE_TARBALL_SUFFIX}" ] && export GPE_TARBALL_SUFFIX="gz"
> > 
> 
> Is there a binary called that makes use of those two?
Yes, some packages uses bz2 but most of them gz. Ebuilds fetching bz2
from the default URI will use both.

> 
> > 
> > IUSE="${IUSE} nls"
> >
> 
> This is the first use of IUSE in the eclass so there is nothing to
> append to.

True, fixed on next revision.
> 
> > 
> > gpe_src_configure() {
> > 	tc-export CC
> > 	if [ -f configure ]; then
> > 		elibtoolize ${ELTCONF}
> > 		econf "$@" ${GPECONF} || die "./configure failure"
> > 	fi
> > }
> > 
> 
> Ebuilds/Eclasses should use [[ instead of [ and econf dies on it's own
> any way.

Fixed on next revision.
> 
> 
> > gpe_src_install() {
> > 	USE_NLS=yes
> > 	use nls || USE_NLS=no
> > 
> 
> I don't see USE_NLS used outside install so it should be local and
> written in lower case.

This is an ancient issue where almost (but not all) packages provides an
--enable-nls flag. I'll discuss with solar about the usefulness of this
code. Thanks.

> 
> > 	if [ -f configure ]; then
> > 		einstall "$@"
> > 	else
> 
> If you really need to use einstall, it would be best to add a comment
> about why it's needed.

Some packages are not automake driven. We have to detect those.
> 
> > 		make DESTDIR=${D} PREFIX=/usr \
> > 			STRIP=true ENABLE_NLS=${USE_NLS} \
> > 			"$@" install
> > 	fi
> > 
> 
> Should use emake. Stripping should be left to the package manager.

Stripping is problematic when cross-compiling. I'll do some more tests
to figure out the best way. Although, we are doing this for a long time
now and it works. IMHO, changing things in the last "hour" usually leads
to breakage.
> 
> > 	# manual document installation
> > 	[ -n "${DOCS}" ] && dodoc ${DOCS}
> > 
> > }
> > 
> 
> dodoc should have || die with it
There are some ebuilds that don't provide all the DOCS, I'll try to fix
the ebuilds first and then we'll see..
> 
> >
> > EXPORT_FUNCTIONS src_compile src_install src_unpack
> > 
> 
> Never exports configure for EAPI 2.

Already fixed, thanks to loki_val for his patch.
> 
> Regards,
> Petteri
> 
> 
> 

Thank you,
-- 
Angelo Arrifano <miknix@gentoo.org>
Gentoo Linux ARM/OMAP850 Developer