Hi all,
One of the changes in PHP7 (which is of course a Fedora 25 Feature Change) is an alteration to the assert() behaviour.
The default default behaviour (ie no ini file changes) is to have assert() work as it did in PHP5.
The recommended php.ini for production is to have zend.assertions=-1 so that assert() code is not even compiled, this is of course a breaking change for any application or library that uses assert() to verify something before acting on it.
This became apparent to me in trying to get php-onelogin-php-saml packaged: https://bugzilla.redhat.com/show_bug.cgi?id=1356552
Note that this is something that can only go from completely disabled (-1) to partially or fully enabled (0 or 1) as part of php.ini ... mod_php settings or phpunit -d cannot change this runtime from -1.
The two essential questions for the PHP SIG to answer then are:
1) Should we maintain compatibility with PHP5 assert() and match the default settings used by tools such as Travis CI, or should we go with the php.ini-production suggestion as we've generally done in the past and set it to the value of zend.assertions=-1 which optimises out any assert() in compilation for performance reasons, and we recognise would potentially 'break' any code that uses assert() to check something and throw an exception if incorrect.
2) If we follow the suggested breaking production change with optimising out assert() in compiled code how do we raise awareness of the change and how do we want to change the PHP Packaging Guidelines to accommodate this?
My personal feeling is that we should set zend.assertions to 0 so that it gets compiled but jumped around to minimise the performance impact, but so that applications or libraries that are using assert() in their runtime (not test) code can toggle it on at runtime for the time being ... and then in F26 a separate change making it clear the default behaviour is altering.
Regardless of the outcome of (1) I think the following changes to PHP guidelines is sensible:
* In %check use the development values to enable assert() statements via:
- %{_bindir}/php -d zend.assertions=1 %{_bindir}/phpunit --verbose --debug --bootstrap etc
* Runtime (not test) PHP code SHOULD not use assert() for runtime checks
* Packaged runtime code SHOULD have a maintainer patch and upstream bug associated.
I feel this would be in keeping with the RFC that implemented the PHP7 Expectation changes:
As an example instead of just assert('is_bool($value))' as is the present case you'd go to:
if ! (is_bool($value)) {
throw exception;
}
Or similar such behaviour... note this would be specific to a runtime check before carrying out an action based on a variable type or similar as per the RFC with any assert() in an area that should not normally be reached being fine.
A quick grep -r 'assert(' of a codebase ought to find them easily enough ...
I would even be open to a MUST given the impact on the code path taken as a result of no assert() being run at all so no exceptions thrown to catch where they might be expected otherwise ...
Of course we should also document the significant change to behaviour in the Fedora 25 Release Notes.
Thoughts?
James
_______________________________________________ php-devel mailing list php-devel@xxxxxxxxxxxxxxxxxxxxxxx https://lists.fedoraproject.org/admin/lists/php-devel@xxxxxxxxxxxxxxxxxxxxxxx
