Hm, was that removal intentional? If so, that should be noted in the migration guide.

Removed by @nikic, I hope so because a cropped error output (very often a long stacktrace) was very hard to use to identify real error source. Also already in PHP 8.0 releases ;-)

@cmb69 are the CI failures related to this PR?

Yes, at least partially. You need to adjust the number of expected array elements in parse_ini_file_variation3.phpt to 9.

Hm, was that removal intentional? If so, that should be noted in the migration guide.

Oops, it wasn't actually. So yeah, we should either re-introduce the functionality into PHP-8.0 (with an explicit truncation) or drop it entirely (as done here). Not sure which...

Drop, please, I am so happy that stack traces from fatal errors are no longer cropped :)

I hope a BC break is enought to reason this :D

So yeah, we should either re-introduce the functionality into PHP-8.0 (with an explicit truncation) or drop it entirely (as done here). Not sure which...

Apparently, nobody complained so far, so maybe the removal without prior deprecation is fine.

I started a thread on the mailing list in https://externals.io/message/114185 and there were no objections (by dint of there being no response). As such, I think we can go ahead with this. This will need a rebase as more tests using the setting have been added in the meantime.

rebased and all occurrences of log_errors_max_len removed

ext/oci8/tests/error3.phpt likely needs to be adapted (I don't think we're testing oci8 on CI). Anybody with a setup who could run the test? Maybe @cjbj?

The change to ext/oci8/tests/error3.phpt is good.

Labeling this as a bugfix since it was a bug that these vestiges remained (even if the real bug was that it was inadvertently removed in the first place 😉 ).

Should the PHP-8.0 upgrade guide be updated to mention that this no longer exists (works) in PHP 8?

@ramsey I think it deserves a line in PHP 8 upgrade as for ex. https://github.com/PrestaShop/PrestaShop/blob/1.7.8.x/install-dev/index_cli.php#L70 is in the code mainly for this - originally, without the echo statement, error was cropped, but now the complete exception/stacktrace is printed

This was merged into master. If it should go into PHP-8.0 it would need to be cherry picked from there. Note that we usually do it by applying to the lowest branch and then merge upwards till master (cf. https://wiki.php.net/vcs/gitworkflow).

@cmb69 I was asking whether it needed a mention in the PHP 8.0 upgrade guide because of this, which is already in PHP-8.0:

https://github.com/php/php-src/pull/5639/files#diff-2978fe1c2c45b4eca89dc476376ddc7193bc4e5e7fff0c7d1c465f057b35a5e6L1199

That said, maybe this should be cherry-picked into PHP-8.0, too. If so, I can revert it from master and follow the usual process. I could see this going either way, but I initially didn't think of it as a bugfix for PHP 8.0.

I reverted this in master (b5d5d06) and added it to PHP-8.0 (d2d227e) with entries for NEWS and UPGRADING (c2b5284)

/cc @carusogabriel @sgolemon

@nikic is the ABI break because of the changes to php_globals.h? Can the rest of this be applied without those changes?

@ramsey, ah, yes, would be helpful to tell users that the INI setting has no effect anymore.

@mvorisek Since I messed things up, this has been reverted from both master and PHP-8.0. Please feel free to open a new PR against the PHP-8.0 branch that applies the changes to the tests and ini files and also includes an update to UPGRADING to mention that log_errors_max_len no longer has any effect. It cannot be removed from main.c or php_globals.h since that would introduce breaking changes to the application binary interface.

Sorry for the churn.

@ramsey I see... Should we really change tests in PHP 8.0? I think we should merge this PR again (eg. cherrypick) into master and then add the UPGRADING/NEWS line into both versions. I prefer if some maintainer can commit this.

@mvorisek, this patch is still in master; it only has been reverted for PHP-8.0 (see b5c0e7e, which is an empty merge).

@cmb69 really? see master branch file tree and b5d5d06

I had reverted it in master and applied it to PHP-8.0 so that I could then merge PHP-8.0 up into master, but it was then reverted from PHP-8.0.

I created two branches:

• changes for PHP 8.0 - https://github.com/mvorisek/php-src/tree/remove_unused_log_maxlen_80
• merged changes for master - https://github.com/mvorisek/php-src/tree/remove_unused_log_maxlen_81

@cmb69 or @nikic please check and commit directly