Re: [gentoo-dev] Please review: poppler.eclass

[Tomáš Chvátal]

Dne středa 25 Březen 2009 19:25:02 Peter Alfredsen napsal(a):
I will just pick parts with notes. Some of them apply on more places :]

> # @ECLASS-VARIABLE: HOMEPAGE
> # @DESCRIPTION:
> # HOMEPAGE is set to http://poppler.freedesktop.org
> HOMEPAGE="http://poppler.freedesktop.org/"
Why you describe the variable with default value? it shows up default value in 
the eclassdoc based on the real HOMEPAGE value, so it is just duplicating :]

> # @FUNCTION: poppler_src_unpack
> # @DESCRIPTION:
> # Runs unpack ${A}
> poppler_src_unpack() {
> 	unpack ${A}
> }
Why not just use base_src_unpack
>
> # @FUNCTION: poppler_src_prepare
> # @DESCRIPTION:
> # Runs autopatch from base.eclass.
> # Uses sed to replace libpoppler.la references with -lpoppler
> poppler_src_prepare() {
> 	base_src_util autopatch
Why not rather use base_src_prepare
> 	sed -i  \
> 		-e 's#$(top_builddir)/poppler/libpoppler.la#-lpoppler#' \
> 		$(find . -type f -name 'Makefile.in') || die "Failed to sed proper lib
> into Makefile.am" }
Are you sure that the find is safe with soemthing like space in the path? ;]
>
> # @FUNCTION: poppler_src_configure
> # @DESCRIPTION:
> # Makes sure we get a uniform Makefile environment by using
> pkg_check_modules_override to # fill out some blanks that configure wants
> filled. Probably not really needed, but # insures against future breakage.
> # Calls econf with some defaults.
> poppler_src_configure() {
> 	pkg_check_modules_override CAIRO cairo
> 	pkg_check_modules_override POPPLER_GLIB glib-2.0
> 	pkg_check_modules_override POPPLER_QT4 QtCore QtGui QtXml
> 	pkg_check_modules_override POPPLER_QT4_TEST QtTest
> 	pkg_check_modules_override ABIWORD libxml-2.0
> 	pkg_check_modules_override GTK_TEST gtk+-2.0 gdk-pixbuf-2.0 libglade-2.0
> gthread-2.0 pkg_check_modules_override GDK gdk-2.0
> 	pkg_check_modules_override POPPLER_GLIB glib-2.0 gobject-2.0
>
> 	econf 	--disable-static		\
> 		--enable-gdk			\
> 		--enable-poppler-qt4		\
> 		--enable-poppler-glib		\
> 		--enable-poppler-qt		\
> 		--enable-xpdf-headers		\
> 		--enable-libjpeg		\
> 		--enable-libopenjpeg		\
> 		--enable-zlib			\
> 		--enable-splash-output		\
> 		${POPPLER_CONF}			\
>
> 		|| die "configuration failed"
No need for die, it dies itself.
>
> }

Cheers
Tomas