Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add installed php extensions to temporary created ini file #3898

Closed
wants to merge 1 commit into from

Conversation

5 participants
@rhabacker
Copy link
Contributor

rhabacker commented Feb 28, 2019

In php extensions configured with phpize, a temporarily generated
php.ini is used for testing, but currently contains no installed
PHP extensions, which is required by the mailparse extension,
for example.

Installed extensions must be added with their absolute path,
because the extension_dir parameter is already occupied.

See https://bugs.php.net/bug.php?id=77609&edit=2

@rhabacker

This comment has been minimized.

Copy link
Contributor Author

rhabacker commented Feb 28, 2019

The mentioned build failure seems not to be related to this patch - how to proceed ?

@carusogabriel

This comment has been minimized.

Copy link
Member

carusogabriel commented Feb 28, 2019

@rhabacker Please, rebase with master, I believe we got some fixes for the Windows CI.

@nikic

This comment has been minimized.

Copy link
Member

nikic commented Feb 28, 2019

You can just ignore the failure. A change to the Linux build system is definitely not going to break the Windows build ;)

@beberlei

This comment has been minimized.

Copy link
Contributor

beberlei commented Feb 28, 2019

I love this change, working around this problem myself right now with the tideways extension, where tests depend on a lot of other extensions to verify instrumentation works.

@rhabacker

This comment has been minimized.

Copy link
Contributor Author

rhabacker commented Feb 28, 2019

Anyone there who can merge this pull request ?

@petk petk added the Bugfix label Feb 28, 2019

Show resolved Hide resolved Makefile.global Outdated
@petk

This comment has been minimized.

Copy link
Member

petk commented Feb 28, 2019

So, this is happening when the mbstring is built as shared extension, right? Yes, the shared extensions have many quirks... /o\

This seems to work ok, except one minor thing, I think. When the path to the extension is set as absolute path in the original *.ini file (/etc/php/conf.d/mbstring.ini) file, for example, extension=/usr/local/php/lib/php/20180731/mbstring.so (a bit dummy and made up case), the extension dir adds too many to the exported tmp-php.ini file.

Yes, as for the regex part of (zend_)?extension(_debug)?(_ts)? it needs to be refactored. Basically, that is nothing deprecated nor something to exclude from the test ini file itself. At least not extension= parts...So probably the new correction is fine unless there is a need to somewhere load some of those special extensions such as zend or debug ones.

@rhabacker

This comment has been minimized.

Copy link
Contributor Author

rhabacker commented Mar 1, 2019

You suggest to remove the extension related part
|(zend_)?extension(_debug)?(_ts)?)[\t\ ]*='
completly ?

Show resolved Hide resolved Makefile.global Outdated
Add installed php extensions to temporary created ini file
In php extensions configured with phpize, a temporarily generated
php.ini is used for testing, but currently contains no installed
PHP extensions, which is required by the mailparse extension,
for example.

Installed extensions must be added with their absolute path,
because the extension_dir parameter is already occupied.

See https://bugs.php.net/bug.php?id=77609&edit=2
@petk

This comment has been minimized.

Copy link
Member

petk commented Mar 1, 2019

Applied via 3ead672 to PHP-7.2+ with few slight adjustments...

Thank you @rhabacker for the patch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.