FWIW, the motivation here is because I support users with both a Zend and non-Zend PHP. They try to disambiguate the two, see the "Zend Engine" text in -v, and ask if they're using Zend's PHP. I'm hoping showing PHP_BUILD_PROVIDER should make it more obvious that it is not a Zend-provided PHP build.

Looks ok. And possibly useful info. Except that if some parser out there relies on this php -v output. But I'm sure they are aware that this entirety of the info is in possibility to change.

Just out of curiosity, how are they building PHP without Zend engine? You mean like some embed-like SAPI that is separate from this code base with some heavy patching?

Just out of curiosity, how are they building PHP without Zend engine? You mean like some embed-like SAPI that is separate from this code base with some heavy patching?

No, it's just that they see "Zend Engine" and think they're using Zend's PHP distribution.

FWIW I also noticed that CLI and CGI SAPIs have diverged in -v output since the changes to print those variables. Might be worth unifying them as well.

Just out of curiosity, how are they building PHP without Zend engine? You mean like some embed-like SAPI that is separate from this code base with some heavy patching?

No, it's just that they see "Zend Engine" and think they're using Zend's PHP distribution.

I see. Thanks for the info. Ok, yes that makes sense then.

Sorry for polluting your PR here. What I was also thinking is adding a new command-line option -V which would only output the version (perhaps this output 8.4.0-dev only). And another one php --vernum to get 80400.

FWIW I also noticed that CLI and CGI SAPIs have diverged in -v output since the changes to print those variables. Might be worth unifying them as well.

Yes, unification would be very welcome across all these command-line interfaces.

I think those make sense along with unifying the version information. New PR or just append it to this one?

I think those make sense along with unifying the version information. New PR or just append it to this one?

Yes, separate PRs for new command-line options. For the unification, I think it is easier to go into this PR. Whatever you find more comfortable. :D

I'd only add the php_main.h to phpdbg.c so it's included more clearly as needed:

diff --git a/sapi/phpdbg/phpdbg.c b/sapi/phpdbg/phpdbg.c
index 250c9a0660..52e2fa745d 100644
--- a/sapi/phpdbg/phpdbg.c
+++ b/sapi/phpdbg/phpdbg.c
@@ -30,6 +30,7 @@
 #include "phpdbg_arginfo.h"
 #include "zend_vm.h"
 #include "php_ini_builder.h"
+#include "php_main.h"
 
 #include "ext/standard/basic_functions.h"

The rest looks good to me and I think it's a very nice addition. Does someone else have any thought here perhaps?

Unrelated note for the future to be synced: Some SAPIs in CLI mode don't seem to have command-line options much synced yet:

• phpdbg --version and phpdbg -V outputs version
• phpdbg -v is verbosity
• litespeed --version doesn't exist

I would rather have the opinions about release managers for this, as I don't really have any opinions. @ramsey @adoy @SakiTakamachi @bukka (I'm missing some) but what do you think about this?

It doesn't seem like there's any particular problem to me either.

So, we can merge this? Sounds good to everyone?

Bumping to see if this can be merged.

Here is now one warning (-Wformat-security) happening and error on the CI since there are errors enabled:

/php-src/sapi/phpdbg/phpdbg.c: In function ‘main’:
/php-src/sapi/phpdbg/phpdbg.c:1378:33: warning: format not a string literal and no format arguments [-Wformat-security]
 1378 | phpdbg_out(prepended_version_info);
 | ^~~~~~~~~~

I'm just thinking about this a bit. This is the current output here by this PR:

./sapi/cli/php -d extension_dir=modules -d zend_extension=opcache -v
PHP 8.4.0-dev (cli) (built: Jul 29 2024 11:07:25) (ZTS GNU C 13.2 x86_64-linux DEBUG GCOV)
Copyright (c) The PHP Group
Built by <BUILD_PROVIDER>
Zend Engine v4.4.0-dev, Copyright (c) Zend Technologies
 with Zend OPcache v8.4.0-dev, Copyright (c), by Zend Technologies

Another option would be to extend the 1st line:

./sapi/cli/php -d extension_dir=modules -d zend_extension=opcache -v
PHP 8.4.0-dev (cli) (built by <BUILD_PROVIDER>: Jul 29 2024 11:07:25) (ZTS GNU C 13.2 x86_64-linux DEBUG GCOV)
Copyright (c) The PHP Group
Zend Engine v4.4.0-dev, Copyright (c) Zend Technologies
 with Zend OPcache v8.4.0-dev, Copyright (c), by Zend Technologies

or:

./sapi/cli/php -d extension_dir=modules -d zend_extension=opcache -v
PHP 8.4.0-dev (cli) (built: Jul 29 2024 11:07:25) (ZTS GNU C 13.2 x86_64-linux DEBUG GCOV)
Built by <BUILD_PROVIDER>
Copyright (c) The PHP Group
Zend Engine v4.4.0-dev, Copyright (c) Zend Technologies
 with Zend OPcache v8.4.0-dev, Copyright (c), by Zend Technologies

Or if we join these info a bit together:

./sapi/cli/php -d extension_dir=modules -d zend_extension=opcache -v
PHP 8.4.0-dev (cli) (ZTS GNU C 13.2 x86_64-linux DEBUG GCOV)
Built by <BUILD_PROVIDER>: Jul 29 2024 11:07:25
Copyright (c) The PHP Group
Zend Engine v4.4.0-dev, Copyright (c) Zend Technologies
 with Zend OPcache v8.4.0-dev, Copyright (c), by Zend Technologies

Basically, the feature freeze is in ~2 weeks, so there's time for all of this also I think. :)

Basically, the feature freeze is in ~2 weeks, so there's time for all of this also I think. :)

That's just for RFC to get voted on - features not requiring RFC can land in beta. So you have time till RC1. Just to clarify... :)

Fixed that warning.

I'm not too picky on the format, I just didn't want to get too bogged down in details. If there's consensus on doing it a specific way (or not to do it), I'll go with that, otherwise as-is should be fine.

Yes, I think the format is just fine. It's also most properly seen that way and understood. I was just thinking out loud.

Then I think this is ready for merge.

Sounds good to me; does anyone have last-minute comments?

@drupol would this impact how reproducible PHP is?

Thanks for pinging me!

I can test this tonight (3 to 4 hours from now) and check.

IMO, it should not impact anything since Nix builds are in a controlled environment where macros like __DATE__ and __TIME__ are already handled properly.

I don't think it would affect it either, as long as you keep PHP_BUILD_PROVIDER the same value every build.

I confirm that this does not have impact on reproducibility, if anyone is willing to reproduce (pun intended!) at home:

❯ nix build --expr 'let pkgs = (builtins.getFlake "github:NixOS/nixpkgs/staging-next").legacyPackages.${builtins.currentSystem}; in (pkgs.php84.overrideAttrs(oldAttrs: { src = pkgs.fetchFromGitHub { owner = "NattyNarwhal"; repo = "php-src"; rev = "build-provider-in-version-info"; hash = "sha256-Hp0eJ4rH9SGNy/xFf/C+f0U2OYj/m/+be5SsHmxzssA=";};})).withExtensions( {...}: [])' --impure -L
❯ ./result/bin/php -v
PHP 8.4.0-dev (cli) (built: Jan 1 1980 00:00:00) (NTS)
Copyright (c) The PHP Group
Zend Engine v4.4.0-dev, Copyright (c) Zend Technologies
❯ nix build --expr 'let pkgs = (builtins.getFlake "github:NixOS/nixpkgs/staging-next").legacyPackages.${builtins.currentSystem}; in (pkgs.php84.overrideAttrs(oldAttrs: { src = pkgs.fetchFromGitHub { owner = "NattyNarwhal"; repo = "php-src"; rev = "build-provider-in-version-info"; hash = "sha256-Hp0eJ4rH9SGNy/xFf/C+f0U2OYj/m/+be5SsHmxzssA=";};})).withExtensions( {...}: [])' --impure -L --rebuild
❯ # No error after adding the --rebuild flag meaning that the build is reproducible

To add an env. variable in the build process, add it as such:

❯ nix build --expr 'let pkgs = (builtins.getFlake "github:NixOS/nixpkgs/staging-next").legacyPackages.${builtins.currentSystem}; in (pkgs.php84.overrideAttrs(oldAttrs: { env = { "PHP_BUILD_PROVIDER" = "FOOBAR"; }; src = pkgs.fetchFromGitHub { owner = "NattyNarwhal"; repo = "php-src"; rev = "build-provider-in-version-info"; hash = "sha256-Hp0eJ4rH9SGNy/xFf/C+f0U2OYj/m/+be5SsHmxzssA=";};})).withExtensions( {...}: [])' --impure -L
❯ ./result/bin/php -v
PHP 8.4.0-dev (cli) (built: Jan 1 1980 00:00:00) (NTS)
Copyright (c) The PHP Group
Built by FOOBAR
Zend Engine v4.4.0-dev, Copyright (c) Zend Technologies

All good for me then!

I'll merge this then.