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

[schneems]

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)

[edorian]

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.

[DanielEScherzer]

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?

[schneems]

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

[edorian]

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.

[krakjoe]

Closing in favor of #19655