During review of php-ext-source-r3.eclass, a number of improvements to the eclass were suggested that will be put off until a later revision (because they will annoy consumers of the eclass). 1. The addtoinifiles() design is overcomplicated. We could simplify it greatly by having it accept only one string as a parameter and it would add that string to the INI files. To add a foo=bar setting, you pass in "foo=bar". To add a [Baz] section, you pass in "[Baz]". And so on. 2. Move the definition of PHP_EXT_SHARED_DIR outside of the createinifiles() function (why is it in there?). If defined in a function, then that should probably happen in php_init_slot_env(). 3. We currently make copies of the source code in src_unpack(), and then apply patches to each copy in src_prepare(). It would make much more sense to apply the patches first and then make the copies. 4. Try to avoid calling doins() every time we add a line in addtoinifiles(). We could probably use ${T}/whatever.ini to store the lines, but then we need to be explicit about what the function does. Does it "add to INI files" even if they've already been doins'ed? Food for thought.
I would like to see an order prefix added to the ini file creation. For instance, http.ini would become 40-http.ini. Where the 40 would be configurable and maybe default to 10. This way a dependency chain can be established for extensions that rely on another.
And we should properly namespace the php_get_slots() and php_init_slot_env() functions...
When the PV of a php-ext-pecl-r3 ebuild is not standard, I have to jump through hoops to get things working correctly. First, set the PHP_EXT_PECL_FILENAME before the inherit. Then, reset S after the inherit because the eclass makes an assumption based on PHP_EXT_PECL_PKG_V which cannot be overridden. Can we simplify this?
(In reply to Brian Evans from comment #3) > When the PV of a php-ext-pecl-r3 ebuild is not standard, I have to jump > through hoops to get things working correctly. > > First, set the PHP_EXT_PECL_FILENAME before the inherit. > > Then, reset S after the inherit because the eclass makes an assumption based > on PHP_EXT_PECL_PKG_V which cannot be overridden. > > Can we simplify this? And third, because I must reset S, PHP_EXT_S must also be reset. PHP_EXT_PECL_PKG_V should be allowed to be set or derived from a variable in the ebuild.
In bug 612054 we need to specify the order in which extensions get loaded. That could be solved in the easy cases by adding a new eclass variable, e.g. PHP_INI_NAME_PREFIX="" (no prefix by default) Or maybe we could let the ebuilds set the whole name, and have it default to the extension name, PHP_INI_NAME="${PHP_EXT_NAME}.ini" Either one should get the job done.
In the php-ext-source-r3_createinifiles() function, I see 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 The variable PHP_EXT_INIFILE isn't mentioned anywhere. Is it ever defined? If so, it should be documented. If not, that chunk of code can go.
We should also add a needed USE mechanism similar to the python eclasses for each PHP slot
(In reply to Michael Orlitzky from comment #6) > In the php-ext-source-r3_createinifiles() function, I see > > 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 > > The variable PHP_EXT_INIFILE isn't mentioned anywhere. Is it ever defined? > If so, it should be documented. If not, that chunk of code can go. The only consumer of this currently is dev-php/xdebug
(In reply to Michael Orlitzky from comment #0) > 3. We currently make copies of the source code in src_unpack(), and then > apply patches to each copy in src_prepare(). It would make much more sense > to apply the patches first and then make the copies. > I have a patch for this.. but it breaks mapserver's src_unpack :( (In reply to Michael Orlitzky from comment #5) > In bug 612054 we need to specify the order in which extensions get loaded. > That could be solved in the easy cases by adding a new eclass variable, e.g. > > PHP_INI_NAME_PREFIX="" (no prefix by default) > > Or maybe we could let the ebuilds set the whole name, and have it default to > the extension name, > > PHP_INI_NAME="${PHP_EXT_NAME}.ini" > > Either one should get the job done. Submitted an RFC patch for this (In reply to Brian Evans from comment #7) > We should also add a needed USE mechanism similar to the python eclasses for > each PHP slot Submitted an RFC patch for this
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=24e8fadab71d00fffab4206a6aa08c663d919e2b commit 24e8fadab71d00fffab4206a6aa08c663d919e2b Author: Brian Evans <grknight@gentoo.org> AuthorDate: 2018-01-26 15:29:06 +0000 Commit: Brian Evans <grknight@gentoo.org> CommitDate: 2018-01-29 13:47:45 +0000 php-ext-source-r3.eclass: Introduce PHP_EXT_NEEDED_USE This simplifies the dependencies in an ebuild @DESCRIPTION: A list of USE flags to append to each PHP target selected as a valid USE-dependency string. The value should be valid for all targets so USE defaults may be necessary. Example: PHP_EXT_NEEDED_USE="mysql?,pdo,pcre(+)" The PHP dependencies will result in: php_targets_php7-0? ( dev-lang/php:7.0[mysql?,pdo,pcre(+)] ) Bug: https://bugs.gentoo.org/586446 eclass/php-ext-source-r3.eclass | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=2c3113fdedacb36b9b38482640ca6d14dd006438 commit 2c3113fdedacb36b9b38482640ca6d14dd006438 Author: Brian Evans <grknight@gentoo.org> AuthorDate: 2018-01-26 14:52:27 +0000 Commit: Brian Evans <grknight@gentoo.org> CommitDate: 2018-01-29 13:47:12 +0000 php-ext-source-r3.eclass: Introduce PHP_INI_NAME variable Currently php-ext-source-r3 saves the enabling ini file as "${PHP_EXT_NAME}.ini". This is problematic when foo module needs to be loaded before bar module as things are read in directory order. This patch introduces PHP_INI_NAME which defaults to PHP_EXT_NAME for backwards-compatibility. Bug: https://bugs.gentoo.org/586446 eclass/php-ext-source-r3.eclass | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)}
In bug 665362 we have a request to make the PHPPREFIX environment variable part of a public API. It wouldn't be too hard; we could simply factor it out into a function: php_get_slot_prefix() { local slot="${1}" echo "${EPREFIX}/usr/$(get_libdir)/${slot}" } and then we would standardize the slot names insofar as they're the only valid arguments to that public function.
