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

Only quote php --ini values when out is a tty #18583

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

Closed
schneems wants to merge 1 commit into php:master from schneems:schneems/ini-tty-fyi

Conversation

Copy link
Contributor

@schneems schneems commented May 18, 2025

A comment in #18527 mentioned that many scripts use php—-ini to bootstrap information about the system. Searching on GitHub confirms that many places will not work with quotes out of the box: https://github.com/search?q=content%3A%22php%20--ini%22&type=code.

Many seem to be variants on this pattern

$ echo "extension=mongodb.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"`

To preserve these existing scripts, we can detect when the output is a TTY to optionally quote them. This is the new behavior:

Output is a tty:

$ make -j$(nproc) &> /dev/null && env PHP_INI_SCAN_DIR="/opt/homebrew/etc/php/8.4/conf.d " PHPRC="/opt/homebrew/etc/php/8.4" ./sapi/cli/php --ini
Configuration File (php.ini) Path: "/usr/local/lib"
Loaded Configuration File: "/opt/homebrew/etc/php/8.4/php.ini"
Scan for additional .ini files in: "/opt/homebrew/etc/php/8.4/conf.d "
Additional .ini files parsed: (none)

Output is not a tty:

$ make -j$(nproc) &> /dev/null && env PHP_INI_SCAN_DIR="/opt/homebrew/etc/php/8.4/conf.d " PHPRC="/opt/homebrew/etc/php/8.4" ./sapi/cli/php --ini | tee
Configuration File (php.ini) Path: /usr/local/lib
Loaded Configuration File: /opt/homebrew/etc/php/8.4/php.ini
Scan for additional .ini files in: /opt/homebrew/etc/php/8.4/conf.d
Additional .ini files parsed: (none)

A comment in php#18527 mentioned that many scripts use `php—-ini` to bootstrap information about the system. Searching on GitHub confirms that many places will not work with quotes out of the box: https://github.com/search?q=content%3A%22php%20--ini%22&type=code.
Many seem to be variants on this pattern
```
$ echo "extension=mongodb.so" >> `php --ini | grep "Loaded Configuration" | sed -e "s|.*:\s*||"`
```
To preserve these existing scripts, we can detect when the output is a TTY to optionally quote them. This is the new behavior:
Output is a tty:
```
$ make -j$(nproc) &> /dev/null && env PHP_INI_SCAN_DIR="/opt/homebrew/etc/php/8.4/conf.d " PHPRC="/opt/homebrew/etc/php/8.4" ./sapi/cli/php --ini
Configuration File (php.ini) Path: "/usr/local/lib"
Loaded Configuration File: "/opt/homebrew/etc/php/8.4/php.ini"
Scan for additional .ini files in: "/opt/homebrew/etc/php/8.4/conf.d "
Additional .ini files parsed: (none)
```
Output is not a tty:
```
$ make -j$(nproc) &> /dev/null && env PHP_INI_SCAN_DIR="/opt/homebrew/etc/php/8.4/conf.d " PHPRC="/opt/homebrew/etc/php/8.4" ./sapi/cli/php --ini | tee
Configuration File (php.ini) Path: /usr/local/lib
Loaded Configuration File: /opt/homebrew/etc/php/8.4/php.ini
Scan for additional .ini files in: /opt/homebrew/etc/php/8.4/conf.d
Additional .ini files parsed: (none)
```
Copy link
Member

edorian commented Aug 28, 2025

Context: When reviewing BC issues as part of my 8.5. RM duties I came across #18527 and the follow ups.

As this change breaks existing scripts we should document it and give people alternatives, or revert it. I will sync with @DanielEScherzer on that and gather some feedback.

The introduced inconsistentcy between php --ini, php -i, php-config --ini and php -r 'echo php_ini_loaded_file();' isn't great either.

And while this PR would "fix" the BC issue, I'm against changing the output depending on tty/no-tty as that's another source of non-obvious behavior and confusion.

Copy link
Member

DanielEScherzer commented Aug 28, 2025
edited
Loading

Yeah, I'm also not a big fan of having different output for TTY and not - when I build up a set of piped commands I inspect the output from one and then write the next, so having a different output when I run the command directly and when the output is piped somewhere would be confusing

We can tighten the search a bit to skip php --ini=..., forks, archived repos, and documentation pages:

content:/php --ini(\s|$|['"`])/ -is:fork -is:archived -path:.md -path:.rst -path:.html -path:.txt

only 1.5k hits now

Would it be a break to add something like php --ini --no-quotes where another option controls the quoting?

Copy link
Contributor Author

I'm against changing the output depending on tty/no-tty as that's another source of non-obvious behavior and confusion.

That's fair. I agree it's a bit mind-bendy to introduce a bug in a script that might only exist while you're not looking at it.

Would it be a break to add something like php --ini --no-quotes where another option controls the quoting?

I think the best long-term option is something like this rubygems/rubygems#8728 where the output is in an explicitly machine readable format (--format=json in that case). So then anyone doing scripting has a stable interface and the maintainers of the CLI code don't have to guess based on other ancillary signals (like TTY or not). It depends on the existence of a tool like jq to parse but that's fairly common at this point (my opinion).

But that's a much bigger lift to introduce a new JSON output and associated formatting flag. I'm not confident I could make an acceptable patch to get a feature like that over the line (C is not my day-job lang).

(brainstorming) If there's interest there we could put out a "help wanted" call. In the mean time if there's some desire to allow for a switch of this behavior we could introduce --format=noquote (or some other, better value) and while we wait for an eventual --format=json

Copy link
Member

edorian commented Aug 29, 2025
edited
Loading

Thank you for the quick reply, especially considering the long waiting time. Much appreciated.

Regarding the whole feature: After reaching out to a couple more people nobody else seemed all that concerned with the BC implications of this change. Given that, I'm not going to argue for removing this from 8.5. If anything we'll document it in UPGRADING and suggest a better way of getting that data.

Adding --format=json or maybe --ini=json is something others also suggested as a stable computational API outside of php -r 'echo php_ini_loaded_file();' and `php -r 'echo ini_get("extension_dir");'. This addition likely doesn't even require an RFC. I'm happy to help with that as well, should be rather simple as json_encode is always available since PHP 8.0.

So if nobody objects I'd close this one.

schneems reacted with thumbs up emoji

Copy link
Member

krakjoe commented Sep 1, 2025

Closing in favor of #19655

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

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