Show build provider and unify version information printing #14657

NattyNarwhal · 2024-06-24T20:33:39Z

Vendors can set the PHP_BUILD_PROVIDER variable, that gets printed in phpinfo. However, I find that users check php -v more often than phpinfo to see what PHP they're running. The problem with this is that it does not show that build provider information.

This change makes the build provider information printed on an additional line of the version information.

Vendors such as distributions can set the `PHP_BUILD_PROVIDER` variable, that gets printed in phpinfo. However, I find that users check `php -v` more often than phpinfo to see what PHP they're running. The problem with this is that it does not show that build provider information. This change makes the build provider information printed on an additional line of the version information.

NattyNarwhal · 2024-06-24T20:35:32Z

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.

Unbreaks build without PHP_BUILD_PROVIDER set.

petk · 2024-06-25T14:18:04Z

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?

sapi/cli/php_cli.c

NattyNarwhal · 2024-06-25T16:09:30Z

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.

NattyNarwhal · 2024-06-25T16:12:57Z

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.

better grammatically; many different possibilities here though

petk · 2024-06-25T16:31:45Z

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.

petk · 2024-06-25T16:34:51Z

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.

NattyNarwhal · 2024-06-25T17:15:43Z

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

petk · 2024-06-25T17:18:51Z

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

This makes it so that all of the SAPIs share the same code for printing version information. This is useful in case of any future changes to the version information, such as i.e. adding build provider to the output.

petk · 2024-06-27T18:00:10Z

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

petk

From my PoV all looks good here. But I'll wait for someone else to check if that's fine to merge already as it is and if the main.c is the place for php_print_version. Perhaps also cc-ing @devnexen, @Girgias

Girgias · 2024-06-29T14:13:35Z

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?

SakiTakamachi · 2024-06-30T12:13:43Z

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

petk · 2024-07-14T20:23:16Z

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

sapi/phpdbg/phpdbg.c

php_printf doesn't have same semantics, as phpdbg_out could be on a different output than stdout/err. Also add the phpdbg version (in case it differs from PHP's, to keep similar output before this PR)

we don't use them and CI doesn't like unused variables

NattyNarwhal · 2024-07-25T16:10:15Z

Bumping to see if this can be merged.

petk · 2024-07-29T08:57:29Z

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);
      |                                 ^~~~~~~~~~

petk · 2024-07-29T09:21:09Z

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. :)

bukka · 2024-07-29T12:45:48Z

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... :)

NattyNarwhal · 2024-08-05T23:17:22Z

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.

petk · 2024-08-06T05:39:42Z

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

petk · 2024-08-11T18:19:21Z

Then I think this is ready for merge.

NattyNarwhal · 2024-08-12T14:45:40Z

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

Girgias · 2024-08-12T14:52:18Z

@drupol would this impact how reproducible PHP is?

drupol · 2024-08-12T14:55:14Z

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.

NattyNarwhal · 2024-08-12T14:57:21Z

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

drupol · 2024-08-12T18:41:19Z

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!

NattyNarwhal · 2024-08-13T02:23:58Z

I'll merge this then.

github-actions bot added the SAPI: cli label Jun 24, 2024

Put on same line so it works with or without env var

1ff39e0

Unbreaks build without PHP_BUILD_PROVIDER set.

petk reviewed Jun 25, 2024

View reviewed changes

sapi/cli/php_cli.c Outdated Show resolved Hide resolved

change wording in provider version text

3ac991c

better grammatically; many different possibilities here though

Unify SAPI version printing

f1e4adb

This makes it so that all of the SAPIs share the same code for printing version information. This is useful in case of any future changes to the version information, such as i.e. adding build provider to the output.

NattyNarwhal requested a review from bukka as a code owner June 25, 2024 18:49

github-actions bot added SAPI: fpm SAPI: cgi SAPI: litespeed SAPI: phpdbg labels Jun 25, 2024

NattyNarwhal changed the title ~~Show build provider information in "php -v"~~ Show build provider and unify version information printing Jun 25, 2024

Make include for php_print_version explicit

c8e9fea

petk approved these changes Jun 27, 2024

View reviewed changes

bukka reviewed Jul 15, 2024

View reviewed changes

sapi/phpdbg/phpdbg.c Outdated Show resolved Hide resolved

NattyNarwhal added 2 commits July 15, 2024 11:46

Preserve phpdbg version and output channel

cf362d3

php_printf doesn't have same semantics, as phpdbg_out could be on a different output than stdout/err. Also add the phpdbg version (in case it differs from PHP's, to keep similar output before this PR)

remove size variables

2ac7cdd

we don't use them and CI doesn't like unused variables

Fix format string insecurity

29b81cf

NattyNarwhal merged commit afc5738 into php:master Aug 13, 2024
10 of 11 checks passed

TimWolla mentioned this pull request Mar 28, 2025

Add php_build_provider() #18168

Merged

Show build provider and unify version information printing #14657

Show build provider and unify version information printing #14657

Uh oh!

Conversation

NattyNarwhal commented Jun 24, 2024

Uh oh!

NattyNarwhal commented Jun 24, 2024

Uh oh!

petk commented Jun 25, 2024

Uh oh!

Uh oh!

NattyNarwhal commented Jun 25, 2024

Uh oh!

NattyNarwhal commented Jun 25, 2024

Uh oh!

petk commented Jun 25, 2024

Uh oh!

petk commented Jun 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NattyNarwhal commented Jun 25, 2024

Uh oh!

petk commented Jun 25, 2024

Uh oh!

petk commented Jun 27, 2024

Uh oh!

petk left a comment

Choose a reason for hiding this comment

Uh oh!

Girgias commented Jun 29, 2024

Uh oh!

SakiTakamachi commented Jun 30, 2024

Uh oh!

petk commented Jul 14, 2024

Uh oh!

Uh oh!

NattyNarwhal commented Jul 25, 2024

Uh oh!

petk commented Jul 29, 2024

Uh oh!

petk commented Jul 29, 2024

Uh oh!

bukka commented Jul 29, 2024

Uh oh!

NattyNarwhal commented Aug 5, 2024

Uh oh!

petk commented Aug 6, 2024

Uh oh!

petk commented Aug 11, 2024

Uh oh!

NattyNarwhal commented Aug 12, 2024

Uh oh!

Girgias commented Aug 12, 2024

Uh oh!

drupol commented Aug 12, 2024

Uh oh!

NattyNarwhal commented Aug 12, 2024

Uh oh!

drupol commented Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NattyNarwhal commented Aug 13, 2024

Uh oh!

Uh oh!

Uh oh!

petk commented Jun 25, 2024 •

edited

Loading

drupol commented Aug 12, 2024 •

edited

Loading