Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: allow empty extension name, fixes #19812 #19816

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

Open
shyim wants to merge 1 commit into php:master
base: master
Choose a base branch
Loading
from shyim:feat/allow-empty-extension-name

Conversation

Copy link

@shyim shyim commented Sep 12, 2025

Allows to load a PHP extension by a php.ini like:

extension=${PROFILER}

If PROFILER env is not there, it loads nothing. if there. it loads that extension

tombojer, mitelg, JanTvrdik, and shivammathur reacted with thumbs up emoji
Copy link
Contributor

Huh, that's a really cool idea.

Copy link
Member

Looks reasonable. Do you mind shooting a quick e-mail to the internals mailing list to ask for feedback? Technically this may still be considered for 8.5 RC1.

shyim and deleugpn reacted with heart emoji

Copy link
Author

shyim commented Sep 12, 2025

Copy link
Contributor

hakre commented Sep 12, 2025

This is already the case. The empty string is ignored, that's why you see the diagnostic message.

What does it diagnose and give you the warning for the benefit: the empty string. It's just not necessary -- don't send an empty directive, it's void. The warning only tells you this already (and not the three+ other permuations that are possible for a non-empty stirng)

The only alternative I can think about is existing with code 254 to fail fast because setting a directive to empty should be diagnosesd as E_DEPRACATED like a since PHP 8.4 function parameter that is just = null without telling per it's type that NULL is included for it's value. Directives are string per their type per the INI directives (at least never NULL) and henceforece PHP 8.4 should have given an E_DEPRECATED error, therefore master should already have rejected with a non-zero status code (likely 254 as it's early) and shutdown.

My 2 cents.

Copy link
Contributor

hakre commented Sep 12, 2025

And to add this report is wrong for the extensions named, the directive is zend_extension not extension (as given).

Copy link
Member

@hakre

This is already the case. The empty string is ignored, that's why you see the diagnostic message.

What do you mean? This is the same message shown for any non-existent extension.

don't send an empty directive, it's void.

Did you read the original issue? The aim is to have something like:

{if hasEnv("FOO")}
extension=foo.so
{/if}

without having to invent new syntax. Tim suggested a simple approach of ignoring empty extension directives. As mentioned on the list, if the fear is that we might accidentally pass empty environment variables without warning, we could use a different sentinel value. Inventing a new if conditional would be significantly more involved, though also solve more problems.

shyim reacted with thumbs up emoji

Copy link
Contributor

hakre commented Sep 28, 2025

What do you mean? This is the same message shown for any non-existent extension.

Yes, right, and rightly so. If you give PHP the directive to try a null string for the directive it resolves it as a relative pathname. PHP does not stop to load here. Don't provide it if you don't mean it.

Did you read the original issue? The aim is to have something like:

{if hasEnv("FOO")}
extension=foo.so
{/if}

Yes I did, this is the equivalent of

.PHONY : conditional
conditional :
# If people set these on the command-line, use them
PHP ?= php
PHPFLAGS ?=
ifdef FOO
 PHPFLAGS += --define extension=foo.so
endif
conditional :
	$(PHP) $(PHPFLAGS) -r 'printf("Hello, world!\n");'

Biased towards an in-work-tree invocation, and it shows more clearly what the extension directive stands for: load the extension and with it, load the extensions' directives. This makes the two directives, extension and zend_extension, special.

The equivalent at the entry-point of a system image packaged FROM the Docker library is much shorter and works since PHP 5.4:

if [ "$FOO" ]
then
 PHP_INI_SCAN_DIR="$PHP_INI_DIR/cfg-foo:$PHP_INI_SCAN_DIR"
fi

Note the colon ":" in the listing; the empty entry stands for the compile time value (ex. "$PHP_INI_DIR/conf.d" in a PHP container from the library). You may want to flip the order (append instead of prepend) or entirely set/replace.

Table: Environment Parameter at the Entry-Point affecting PHP Resource Configuration and Scanning for INI-Files

Parameter Configure Build Argument Comment
PHPRC --with-config-file-path=PATH php.ini 1-20
PHP_INI_SCAN_DIR --with-config-file-scan-dir=PATH >= 5.4, bc6a9ad

To complete the example:

$ cat "$PHP_INI_DIR/cfg-foo/foo.ini"
extension = foo.so

The OPs' configuration directive, now conditionally parsed based on the FOO environment parameter.

Problem solved. Otherwise, there is /tmp in a read-only container.

Nevertheless, to be clear: Our problem is not a read-only file-system here, from a systems engineering perspective we were longtime coming from the standpoint that files in sysconfigdir are not changed by the user nor the program. This is also the reason why the OP's-known solution actually is technically wrong when packaging the system image, and we should avoid any potential x/y problems here.

without having to invent new syntax. Tim suggested a simple approach of ignoring empty extension directives. As mentioned on the list, if the fear is that we might accidentally pass empty environment variables without warning, we could use a different sentinel value. Inventing a new if conditional would be significantly more involved, though also solve more problems.

Yes, right, and if we still want to patch the configuration loading I'd recommend to add two new directives:

  1. extension_ifdef
  2. zend_extension_ifdef

they only delegate to their sibling if the value on the right-hand side represents a non-empty string; that is:

  1. not "" and
  2. not false and
  3. not null

This helps to clearly differentiate between their ground form and the intended use, that is to achieve the command-line directive semantics as outlined in the Makefile listing for PHPFLAGS above.

There is also no need to find a sentinel value; I think empty is the best value for what we want to express here, especially as we have to map environment parameter effective and efficiently.
And the two new proposed directives are speaking a clear language. As those are new directives, they should work on the command line, too.

Other directives should not be affected: we use extension and zend_extension to shortcut the effectiveness of the extensions configuration directives (if it could be loaded) so we move the condition to the place where it belongs in the logical order of the abstractions.

flowchart LR
 A((start))
 A --> C
 A --> d1
 A --> d2
 d1(extension_ifdef) ---> B
 B{defined?} -- no --> C
 C((stop))
 B -- yes --> d2
 d2(extension)
 d2 --> C
Loading

Furthermore, and additionally I propose we should add another long option and an ini-file counterpart.

The --define-env <name>=<envvar> long option that is similar to --define <name>=<value> and set the directive <name> where <envvar> is the name of an environment variable from which to retrieve the value. Unlike -d there is no shortcut for directly setting the value to an empty string, instead the environment variable itself must be set to the empty string. It is an error if the <envvar> does not exist in the environment. (copied from git(1))

[PHP]
example_directive:env = ENVVAR

The right hand side evaluates to the name of an environment variable. It is an error if it does not exist in the environment.

Consequences of an error in these instances: The runtime stops loading the configuration and exits with a non-zero exit status not larger than 254, including.

To share the benefit of the new error handling within the configuration, I'd further-furthermore and additionally suggest to have ':?'/'?' semantics with the environment parameter expansion in php-ini-files:

example_directive = ${ENVVAR?}

Given ENVVAR does not exist in the environment, it is an error with the same consequences as already outlined.

example_directive = ${ENVVAR:?}

Given ENVVAR does not exist in the environment or is null, it is an error with the same consequences as already outlined.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bukka bukka Awaiting requested review from bukka bukka is a code owner

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /