Re: [gentoo-dev] [RFC] New eclass php-pear-r2

["Michał Górny" <mgorny@gentoo.org>]

[-- Attachment #1: Type: text/plain, Size: 5256 bytes --]

W dniu 28.02.2017, wto o godzinie 10∶43 -0500, użytkownik Brian Evans
napisał:
> # Copyright 1999-2017 Gentoo Foundation
> # Distributed under the terms of the GNU General Public License v2
> # $Id$
> 
> # @ECLASS: php-pear-r2.eclass
> # @MAINTAINER:
> # Gentoo PHP Team <php-bugs@gentoo.org>
> # @AUTHOR:
> # Author: Brian Evans <grknight@gentoo.org>
> # @BLURB: Provides means for an easy installation of PEAR packages.
> # @DESCRIPTION:
> # This eclass provides means for an easy installation of PEAR packages.
> # For more information on PEAR, see https://pear.php.net/
> # Note that this eclass doesn't handle dependencies of PEAR packages
> # on purpose; please use (R)DEPEND to define them correctly!
> 
> EXPORT_FUNCTIONS src_install pkg_postinst pkg_postrm
> 
> case "${EAPI:-0}" in

You don't have to quote arguments to 'case', it's a builtin.

> 	6)
> 		;;
> 	*)
> 		die "Unsupported EAPI=${EAPI} for ${ECLASS}"
> 		;;
> esac
> 
> RDEPEND=">=dev-php/pear-1.8.1"
> 
> # @ECLASS-VARIABLE: PHP_PEAR_PKG_NAME
> # @DESCRIPTION:
> # Set this if the PEAR package name differs from ${PN/PEAR-/}
> # (generally shouldn't be the case).
> [[ -z "${PHP_PEAR_PKG_NAME}" ]] && PHP_PEAR_PKG_NAME="${PN/PEAR-/}"

: ${PHP_PEAR_PKG_NAME:=${PN/PEAR-/}}

Funny thing is, you're already using that syntax below ;-).

> 
> # @ECLASS-VARIABLE: PEAR_PV
> # @DESCRIPTION:
> # Set in ebuild if the ${PV} breaks SRC_URI for alpha/beta/rc versions
> : ${PEAR_PV:=${PV}}
> 
> PEAR_PN="${PHP_PEAR_PKG_NAME}-${PEAR_PV}"

Why do you name it 'PN' if it includes version? That's a bit confusing.

> 
> # @ECLASS-VARIABLE: PHP_PEAR_URI
> # @DESCRIPTION:
> # Set in ebuild to the domain name of the channel if not pear.php.net
> : ${PHP_PEAR_URI:=pear.php.net}

Why do you name it 'URI' if it is just the domain? Definitely confusing.

> 
> : ${SRC_URI:=https://${PHP_PEAR_URI}/get/${PEAR_PN}.tgz}
> : ${HOMEPAGE:=https://${PHP_PEAR_URI}/package/${PHP_PEAR_PKG_NAME}}

This triggers undefined behavior since SRC_URI is stacked. The 'if
unset' part is not really meaningful. Depending on the package manager
implementation, the variable will be always set (because the PM will
unset SRC_URI on inherit), or may be randomly unset (e.g. if another
eclass put something in SRC_URI before this one).

> 
> S="${WORKDIR}/${PEAR_PN}"
> 
> # @FUNCTION php-pear-r2_install_packagexml
> # @DESCRIPTION:
> # Copies the package{,2}.xml file and, optionally, the channel.xml file
> # to a hidden directory so that pkg_postinst can install the package
> # to the local PEAR database

This description is confusing. 'package{,2}.xml' suggests that both will
be installed, not either of the two. Also, this 'hidden directory'
sounds like a lot of magic. Maybe that should be 'Gentoo-specific
location' or something like that?

> php-pear-r2_install_packagexml() {
> 	insinto /usr/share/php/.packagexml
> 	if [[ -f "${WORKDIR}/package2.xml" ]] ; then
> 		newins "${WORKDIR}/package2.xml" "${PEAR_PN}.xml"
> 	elif [[ -f "${WORKDIR}/package.xml" ]] ; then
> 		newins "${WORKDIR}/package.xml" "${PEAR_PN}.xml"
> 	fi
> 
> 	if [[ -f "${PHP_PEAR_CHANNEL}" ]] ; then

Unless I'm mistaken, you are not setting this variable.

> 		newins "${PHP_PEAR_CHANNEL}" "${PEAR_PN}-channel.xml"
> 	fi
> }
> 
> # @FUNCTION: php-pear-r2_src_install
> # @DESCRIPTION:
> # Takes care of standard install for PEAR packages.
> # Override src_install if the package installs more than "${PHP_PEAR_PKG_NAME}.php"
> # or "${PHP_PEAR_PKG_NAME%%_*}/" as a directory
> php-pear-r2_src_install() {
> 	insinto /usr/share/php
> 	[[ -f "${PHP_PEAR_PKG_NAME}.php" ]] && doins "${PHP_PEAR_PKG_NAME}.php"
> 	[[ -d "${PHP_PEAR_PKG_NAME%%_*}" ]] && doins -r "${PHP_PEAR_PKG_NAME%%_*}/"
> 	php-pear-r2_install_packagexml
> 	einstalldocs
> }
> 
> # @FUNCTION: php-pear-r2_pkg_postinst
> # @DESCRIPTION:
> # Register package with the local PEAR database.
> php-pear-r2_pkg_postinst() {
> 	# Add unknown channels
> 	if [[ -f "${EROOT}usr/share/php/.packagexml/${PEAR_PN}-channel.xml" ]] ; then
> 		"${EROOT}usr/bin/peardev" channel-info "${PHP_PEAR_URI}" &> /dev/null
> 		if [[ "$?x" != "0x" ]] ; then

Err, we don't do 'the x-thing' in bash. Much better:

  if ! "${EROOT}usr/bin/peardev" ... &> /dev/null; then

> 			"${EROOT}usr/bin/peardev" channel-add \
> 				"${EROOT}usr/share/php/.packagexml/${PEAR_PN}-channel.xml" \
> 				|| einfo "Ignore any errors about existing channels"
> 		fi
> 	fi
> 
> 	# Register the package from the package{,2}.xml file
> 	# It is not critical to complete so only warn on failure
> 	if [[ -f "${EROOT}usr/share/php/.packagexml/${PEAR_PN}.xml" ]] ; then
> 		"${EROOT}usr/bin/peardev" install -nrO --force \
> 			"${EROOT}usr/share/php/.packagexml/${PEAR_PN}.xml" 2> /dev/null \
> 			|| ewarn "Failed to insert package into local PEAR database"
> 	fi
> }
> 
> # @FUNCTION: php-pear-r2_pkg_postrm
> # @DESCRIPTION:
> # Deregister package from the local PEAR database
> php-pear-r2_pkg_postrm() {
> 	# Uninstall known dependency
> 	"${EROOT}usr/bin/peardev" uninstall -nrO "${PHP_PEAR_URI}/${PHP_PEAR_PKG_NAME}"
> }

-- 
Best regards,
Michał Górny

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