Thanks for the detailed review. I followed every suggestion except the
doexe thing for *.so files (only because I don't understand the
reasoning yet). The new version is attached.
On 06/01/2016 01:47 PM, Michał Górny wrote:
>> +DEPEND=">=sys-devel/m4-1.4.3
>> + >=sys-devel/libtool-1.5.18"
>> +RDEPEND=""
>
> Please move all of this below EAPI check. And I think *DEPEND would be
> better set in one place.
I moved them all after the REQUIRED_USE mess.
>> +case ${EAPI} in
>> + 6) ;;
>> + *)
>> + die "php-ext-source-r3 is not compatible with EAPI=${EAPI}"
>
> I think you can use ${ECLASS} here btw.
Done.
>> +# @DESCRIPTION:
>> +# Lists the PHP slots compatibile the extension is compatibile with
>
> typo: compatible, and also appears twice ;-).
Fixed.
>> +# @ECLASS-VARIABLE: PHP_EXT_SAPIS
>> +# @DESCRIPTION:
>> +# A list of SAPIs for which we'll install this extension. Formerly
>> +# called PHPSAPILIST.
>> +[[ -z "${PHP_EXT_SAPIS}" ]] && PHP_EXT_SAPIS="apache2 cli cgi fpm embed
>> phpdbg"
>
> Is this default something that might get extended in the future? I
> think it might be worth noting that here.
Yeah, I think we'll keep every SAPI currently used in the tree in the
list by default. We added "phpdbg" even though php:5.5 doesn't support
it. I documented that.
>> +# Make sure at least one target is installed. First, start a USE
>> +# conditional like "php?", but only when PHP_EXT_OPTIONAL_USE is
>> +# non-null. The option group "|| (..." is always started here.
>> +REQUIRED_USE="${PHP_EXT_OPTIONAL_USE}${PHP_EXT_OPTIONAL_USE:+? ( }|| ( "
>> +for target in ${USE_PHP}; do
>
> Environment pollution! unset target, or make this local.
I renamed it to _php_target and unset it after the loop.
>> + # Now loop through each USE_PHP target and add the corresponding
>> + # dev-lang/php slot to PHPDEPEND.
>> + IUSE="${IUSE} php_targets_${target}"
>
> Any reason not to use += here as well?
Nope, and likewise with PHPDEPEND in the same loop. I changed them both.
>> +# @ECLASS-VARIABLE: PHP_EXT_EXTRA_ECONF
>> +# @DESCRIPTION:
>> +# Set this in the ebuild to pass configure options to econf. Formerly
>> +# called my_conf.
>
> Please don't name this *EXTRA_ECONF. EXTRA_ECONF is a Portage variable
> that's meant to be set by user in make.conf/env, and not ebuilds.
It's PHP_EXT_ECONF_ARGS now, and I updated the migration guide.
>> + # Let's put the default module away. Strip $EPREFIX from
>> + # $EXT_DIR before calling newins (which handles EPREFIX itself).
>> + insinto "${EXT_DIR#$EPREFIX}"
>> + newins "modules/${PHP_EXT_NAME}.so" "${PHP_EXT_NAME}.so"
>
> Wouldn't it be more appropriate to use exeinto/doexe to have the shared
> libs +x?
I'd never heard this before... why? I suppose the only trade-off is that
having them -x prevents them from showing up in bash's tab-completion
for executables.
>> +# @FUNCTION: php_init_slot_env
>> +# @DESCRIPTION:
>> +# Takes a slot name, and initializes some global variables to values
>> +# corresponding to that slot. For example, it sets the path to the "php"
>> +# and "phpize" binaries, which will differ for each slot. This function
>> +# is intended to be called while looping through a list of slots
>> +# obtained from php_get_slots().
>
> Please make it explicit that it changes working directory.
Done.
>> +# @FUNCTION: php_slot_ini_files
>> +# @INTERNAL
>> +# @DESCRIPTION:
>> +# Output a list of relative paths to INI files for the given
>> +# slot. Usually there will be one INI file per SAPI.
>
> Please add @USAGE to make clear what ${1} is. In fact, many functions
> seem to miss @USAGE.
I added USAGE for every function that takes an argument. I also figured
out where the eclass manpages come from and made sure that the other doc
issues (like DEFAULT_UNSET) are fixed w.r.t. eclass-to-manpage.sh.
>> + cat "${FILESDIR}/${PHP_EXT_INIFILE}" >>
>> "${ED}/${file}"
>
> || die
Fixed all of these.
>> + # Add support for installing PHP files into a version dependent
>> + # directory
>> + PHP_EXT_SHARED_DIR="${EPREFIX}/usr/share/php/${PHP_EXT_NAME}"
>
> Should this be repeated inside the loop?
There's a longer answer to that question, but the fact that it's outside
of the loop is intentional and consistent with -r2.
>> +php-ext-source-r3_addextension() {
>> + if [[ "${PHP_EXT_ZENDEXT}" = "yes" ]] ; then
>> + ext_type="zend_extension"
>> + ext_file="${EXT_DIR}/${1}" # Zend extensions need the path...
>> + else
>> + ext_type="extension"
>> + ext_file="${1}"
>> + fi
>
> I think you want those variables local.
Fixed all of the local variable issues.
>> +php-ext-source-r3_addtoinifile() {
>> + local inifile="${WORKDIR}/${1}"
>> + local inidir="$(dirname ${inifile})"
>
> I would suggest avoiding external tools like dirname when simple bash
> subst can do the work (e.g. ${inifile%/*}).
Fixed both `dirname` calls.
>> + if [[ ! -d "${inidir}" ]] ; then
>> + mkdir -p "${inidir}" || die "failed to create INI directory
>> ${inidir}"
>
> 'mkdir -p' is safe to call when the directory exists, so I don't think
> you need the conditional.
Fixed.
> All above considered, wouldn't it be simpler to set my_added first,
> and then just echo it to the file outside the conditional?
Yes, good catch. I cleaned that up.
>> +# @FUNCTION: php-ext-source-r3_addtoinifiles
>> +# @USAGE: <setting name> <setting value> [message to output]; or just
>> [section name]
>
> I think you are mixing '[]' meaning optional and meaning literal '[]'
> here.
Yeah, I changed the first parameter to <setting-or-section-name> and
made the other two optional.
>> +# php-ext-source-r3_addtoinifiles "zend_optimizer.disable_licensing" "0"
>
> Hmm... just to make it clear... is there any reason you use two
> arguments instead of the more obvious 'foo=15'?
It's weird, but it's not wrong, and that's the way -r2 did it. There are
a few ebuilds (not maintained by the PHP team) that call
*addtoinifiles() themselves, and I don't want to annoy those people too
much for cosmetic changes.
# Copyright 1999-2016 Gentoo Foundation
# Distributed under the terms of the GNU General Public License v2
# $Id$
# @ECLASS: php-ext-source-r3.eclass
# @MAINTAINER:
# Gentoo PHP team <php-b...@gentoo.org>
# @BLURB: Compile and install standalone PHP extensions.
# @DESCRIPTION:
# A unified interface for compiling and installing standalone PHP
# extensions.
inherit autotools
EXPORT_FUNCTIONS src_unpack src_prepare src_configure src_compile src_install
case ${EAPI} in
6) ;;
*)
die "${ECLASS} is not compatible with EAPI=${EAPI}"
esac
# @ECLASS-VARIABLE: PHP_EXT_NAME
# @REQUIRED
# @DESCRIPTION:
# The extension name. This must be set, otherwise the eclass dies.
# Only automagically set by php-ext-pecl-r3.eclass, so unless your ebuild
# inherits that eclass, you must set this manually before inherit.
[[ -z "${PHP_EXT_NAME}" ]] && \
die "no extension name specified for the php-ext-source-r3 eclass"
# @ECLASS-VARIABLE: PHP_EXT_INI
# @DESCRIPTION:
# Controls whether or not to add a line to php.ini for the extension.
# Defaults to "yes" and should not be changed in most cases.
[[ -z "${PHP_EXT_INI}" ]] && PHP_EXT_INI="yes"
# @ECLASS-VARIABLE: PHP_EXT_ZENDEXT
# @DESCRIPTION:
# Controls whether the extension is a ZendEngine extension or not.
# Defaults to "no". If you don't know what this is, you don't need it.
[[ -z "${PHP_EXT_ZENDEXT}" ]] && PHP_EXT_ZENDEXT="no"
# @ECLASS-VARIABLE: USE_PHP
# @REQUIRED
# @DESCRIPTION:
# Lists the PHP slots compatible the extension is compatible with.
# Example:
# @CODE
# USE_PHP="php5-6 php7-0"
# @CODE
[[ -z "${USE_PHP}" ]] && \
die "USE_PHP is not set for the php-ext-source-r3 eclass"
# @ECLASS-VARIABLE: PHP_EXT_OPTIONAL_USE
# @DEFAULT_UNSET
# @DESCRIPTION:
# If set, all of the dependencies added by this eclass will be
# conditional on USE=${PHP_EXT_OPTIONAL_USE}. This is needed when
# ebuilds have to inherit this eclass unconditionally, but only
# actually use it when (for example) the user has USE=php.
# @ECLASS-VARIABLE: PHP_EXT_S
# @DESCRIPTION:
# The relative location of the temporary build directory for the PHP
# extension within the source package. This is useful for packages that
# bundle the PHP extension. Defaults to ${S}.
[[ -z "${PHP_EXT_S}" ]] && PHP_EXT_S="${S}"
# @ECLASS-VARIABLE: PHP_EXT_SAPIS
# @DESCRIPTION:
# A list of SAPIs for which we will install this extension. Formerly
# called PHPSAPILIST. The default includes every SAPI currently used in
# the tree.
[[ -z "${PHP_EXT_SAPIS}" ]] && PHP_EXT_SAPIS="apache2 cli cgi fpm embed phpdbg"
# Make sure at least one target is installed. First, start a USE
# conditional like "php?", but only when PHP_EXT_OPTIONAL_USE is
# non-null. The option group "|| (..." is always started here.
REQUIRED_USE="${PHP_EXT_OPTIONAL_USE}${PHP_EXT_OPTIONAL_USE:+? ( }|| ( "
for _php_target in ${USE_PHP}; do
# Now loop through each USE_PHP target and add the corresponding
# dev-lang/php slot to PHPDEPEND.
IUSE+=" php_targets_${_php_target}"
REQUIRED_USE+="php_targets_${_php_target} "
slot=${_php_target/php}
slot=${slot/-/.}
PHPDEPEND+=" php_targets_${_php_target}? ( dev-lang/php:${slot} )"
done
# Don't pollute the environment with our loop variable...
unset _php_target
# Finally, end the optional group that we started before the loop. Close
# the USE-conditional if PHP_EXT_OPTIONAL_USE is non-null.
REQUIRED_USE+=") ${PHP_EXT_OPTIONAL_USE:+ )}"
RDEPEND="${RDEPEND}
${PHP_EXT_OPTIONAL_USE}${PHP_EXT_OPTIONAL_USE:+? ( }
${PHPDEPEND}
${PHP_EXT_OPTIONAL_USE:+ )}"
DEPEND="${DEPEND}
sys-devel/m4
sys-devel/libtool
${PHP_EXT_OPTIONAL_USE}${PHP_EXT_OPTIONAL_USE:+? ( }
${PHPDEPEND}
${PHP_EXT_OPTIONAL_USE:+ )}
"
# @ECLASS-VARIABLE: PHP_EXT_SKIP_PHPIZE
# @DEFAULT_UNSET
# @DESCRIPTION:
# By default, we run "phpize" in php-ext-source-r3_src_unpack(). Set
# PHP_EXT_SKIP_PHPIZE="yes" in your ebuild if you do not want to run
# phpize (and the autoreconf that becomes necessary afterwards).
# @FUNCTION: php-ext-source-r3_src_unpack
# @DESCRIPTION:
# Runs the default src_unpack and then makes a copy for each PHP slot.
php-ext-source-r3_src_unpack() {
default
local slot orig_s="${PHP_EXT_S}"
for slot in $(php_get_slots); do
cp -r "${orig_s}" "${WORKDIR}/${slot}" || \
die "failed to copy sources from ${orig_s} to
${WORKDIR}/${slot}"
done
}
# @FUNCTION: php-ext-source-r3_src_prepare
# @DESCRIPTION:
# For each PHP slot, we initialize the environment, run the default
# src_prepare() for PATCHES/eapply_user support, and then call
# php-ext-source-r3_phpize.
php-ext-source-r3_src_prepare() {
for slot in $(php_get_slots); do
php_init_slot_env "${slot}"
default
php-ext-source-r3_phpize
done
}
# @FUNCTION: php-ext-source-r3_phpize
# @DESCRIPTION:
# Subject to PHP_EXT_SKIP_PHPIZE, this function runs phpize and
# autoreconf in a manner that avoids warnings.
php-ext-source-r3_phpize() {
if [[ "${PHP_EXT_SKIP_PHPIZE}" != 'yes' ]] ; then
# Create configure out of config.m4. We use autotools_run_tool
# to avoid some warnings about WANT_AUTOCONF and
# WANT_AUTOMAKE (see bugs #329071 and #549268).
autotools_run_tool "${PHPIZE}"
# Force libtoolize to run and regenerate autotools files (bug
# #220519).
rm aclocal.m4 || die "failed to remove aclocal.m4"
eautoreconf
fi
}
# @ECLASS-VARIABLE: PHP_EXT_ECONF_ARGS
# @DEFAULT_UNSET
# @DESCRIPTION:
# Set this in the ebuild to pass additional configure options to
# econf. Formerly called my_conf.
# @FUNCTION: php-ext-source-r3_src_configure
# @DESCRIPTION:
# Takes care of standard configure for PHP extensions (modules).
php-ext-source-r3_src_configure() {
# net-snmp creates these, bug #385403.
addpredict /usr/share/snmp/mibs/.index
addpredict /var/lib/net-snmp/mib_indexes
local slot
for slot in $(php_get_slots); do
php_init_slot_env "${slot}"
# Set the correct config options
econf --with-php-config=${PHPCONFIG} ${PHP_EXT_ECONF_ARGS}
done
}
# @FUNCTION: php-ext-source-r3_src_compile
# @DESCRIPTION:
# Compile a standard standalone PHP extension.
php-ext-source-r3_src_compile() {
# net-snmp creates these, bug #324739.
addpredict /usr/share/snmp/mibs/.index
addpredict /var/lib/net-snmp/mib_indexes
# shm extension creates a semaphore file, bug #173574.
addpredict /session_mm_cli0.sem
local slot
for slot in $(php_get_slots); do
php_init_slot_env "${slot}"
emake
done
}
# @FUNCTION: php-ext-source-r3_src_install
# @DESCRIPTION:
# Install a standard standalone PHP extension. Uses einstalldocs()
# to support the DOCS variable/array.
php-ext-source-r3_src_install() {
local slot
for slot in $(php_get_slots); do
php_init_slot_env "${slot}"
# Let's put the default module away. Strip $EPREFIX from
# $EXT_DIR before calling newins (which handles EPREFIX itself).
insinto "${EXT_DIR#$EPREFIX}"
newins "modules/${PHP_EXT_NAME}.so" "${PHP_EXT_NAME}.so"
INSTALL_ROOT="${D}" emake install-headers
done
einstalldocs
php-ext-source-r3_createinifiles
}
# @FUNCTION: php_get_slots
# @DESCRIPTION:
# Get a list of PHP slots contained in both the ebuild's USE_PHP and the
# user's PHP_TARGETS.
php_get_slots() {
local s=""
local slot
for slot in ${USE_PHP}; do
use php_targets_${slot} && s+=" ${slot/-/.}"
done
echo $s
}
# @FUNCTION: php_init_slot_env
# @USAGE: <slot>
# @DESCRIPTION:
# Takes a slot name, and initializes some global variables to values
# corresponding to that slot. For example, it sets the path to the "php"
# and "phpize" binaries, which will differ for each slot. This function
# is intended to be called while looping through a list of slots
# obtained from php_get_slots().
#
# Calling this function will change the working directory to the
# temporary build directory for the given slot.
php_init_slot_env() {
local libdir=$(get_libdir)
PHPIZE="${EPREFIX}/usr/${libdir}/${1}/bin/phpize"
PHPCONFIG="${EPREFIX}/usr/${libdir}/${1}/bin/php-config"
PHPCLI="${EPREFIX}/usr/${libdir}/${1}/bin/php"
PHPCGI="${EPREFIX}/usr/${libdir}/${1}/bin/php-cgi"
PHP_PKG="$(best_version =dev-lang/php-${1:3}*)"
PHPPREFIX="${EPREFIX}/usr/${libdir}/${slot}"
EXT_DIR="$(${PHPCONFIG} --extension-dir 2>/dev/null)"
PHP_CURRENTSLOT=${1:3}
PHP_EXT_S="${WORKDIR}/${1}"
cd "${PHP_EXT_S}" || die "failed to change directory to ${PHP_EXT_S}"
}
# @FUNCTION: php_slot_ini_files
# @USAGE: <slot>
# @INTERNAL
# @DESCRIPTION:
# Output a list of relative paths to INI files for the given
# slot. Usually there will be one INI file per SAPI.
php_slot_ini_files() {
local slot_ini_files=""
local x
for x in ${PHP_EXT_SAPIS} ; do
if [[ -f "${EPREFIX}/etc/php/${x}-${1}/php.ini" ]] ; then
slot_ini_files+="
etc/php/${x}-${1}/ext/${PHP_EXT_NAME}.ini"
fi
done
echo "${slot_ini_files}"
}
# @FUNCTION: php-ext-source-r3_createinifiles
# @DESCRIPTION:
# Builds INI files for every enabled slot and SAPI.
php-ext-source-r3_createinifiles() {
local slot
for slot in $(php_get_slots); do
php_init_slot_env "${slot}"
local file
for file in $(php_slot_ini_files "${slot}") ; do
if [[ "${PHP_EXT_INI}" = "yes" ]] ; then
# Add the needed lines to the <ext>.ini files
php-ext-source-r3_addextension
"${PHP_EXT_NAME}.so" "${file}"
fi
if [[ -n "${PHP_EXT_INIFILE}" ]] ; then
cat "${FILESDIR}/${PHP_EXT_INIFILE}" >>
"${ED}/${file}" \
|| die "failed to append to
${ED}/${file}"
einfo "Added contents of
${FILESDIR}/${PHP_EXT_INIFILE}" \
"to ${file}"
fi
inidir="${file/${PHP_EXT_NAME}.ini/}"
inidir="${inidir/ext/ext-active}"
dodir "/${inidir}"
dosym "/${file}" "/${file/ext/ext-active}"
done
# Add support for installing PHP files into a version dependent
# directory
PHP_EXT_SHARED_DIR="${EPREFIX}/usr/share/php/${PHP_EXT_NAME}"
done
}
# @FUNCTION: php-ext-source-r3_addextension
# @USAGE: <extension-path> <ini-file>
# @INTERNAL
# @DESCRIPTION:
# Add a line to an INI file that will enable the given extension. The
# first parameter is the path to the extension (.so) file, and the
# second parameter is the name of the INI file in which it should be
# loaded. This function determines the setting name (either
# "extension=..." or "zend_extension=...") and then calls
# php-ext-source-r3_addtoinifile to do the actual work.
php-ext-source-r3_addextension() {
local ext_type="extension"
local ext_file="${1}"
if [[ "${PHP_EXT_ZENDEXT}" = "yes" ]] ; then
ext_type="zend_extension"
ext_file="${EXT_DIR}/${1}" # Zend extensions need the path...
fi
php-ext-source-r3_addtoinifile "${2}" "${ext_type}" "${ext_file}"
}
# @FUNCTION: php-ext-source-r3_addtoinifile
# @USAGE: <relative-ini-path> <setting-or-section-name> [setting-value]
# @INTERNAL
# @DESCRIPTION:
# Add a setting=value to one INI file. The first argument is the
# relative path to the INI file. The second argument is the setting
# name, and the third argument is its value.
#
# You can also pass "[Section]" as the second parameter, to create a new
# section in the INI file. In that case, the third parameter (which
# would otherwise be the value of the setting) is ignored.
php-ext-source-r3_addtoinifile() {
local inifile="${WORKDIR}/${1}"
local inidir="${inifile%/*}"
mkdir -p "${inidir}" || die "failed to create INI directory ${inidir}"
# Are we adding the name of a section? Assume not by default.
local my_added="${2}=${3}"
if [[ ${2:0:1} == "[" ]] ; then
# Ok, it's a section name.
my_added="${2}"
fi
echo "${my_added}" >> "${inifile}" || die "failed to append to
${inifile}"
einfo "Added '${my_added}' to /${1}"
insinto "/${1%/*}"
doins "${inifile}"
}
# @FUNCTION: php-ext-source-r3_addtoinifiles
# @USAGE: <setting-or-section-name> [setting-value] [message]
# @DESCRIPTION:
# Add settings to every php.ini file installed by this extension.
# You can also add new [Section]s -- see the example below.
#
# @CODE
# Add some settings for the extension:
#
# php-ext-source-r3_addtoinifiles "zend_optimizer.optimization_level" "15"
# php-ext-source-r3_addtoinifiles "zend_optimizer.enable_loader" "0"
# php-ext-source-r3_addtoinifiles "zend_optimizer.disable_licensing" "0"
#
# Adding values to a section in php.ini file installed by the extension:
#
# php-ext-source-r3_addtoinifiles "[Debugger]"
# php-ext-source-r3_addtoinifiles "debugger.enabled" "on"
# php-ext-source-r3_addtoinifiles "debugger.profiler_enabled" "on"
# @CODE
php-ext-source-r3_addtoinifiles() {
local slot
for slot in $(php_get_slots); do
for file in $(php_slot_ini_files "${slot}") ; do
php-ext-source-r3_addtoinifile "${file}" "${1}" "${2}"
done
done
}
