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:

https://wiki.php.net/rfc/expectations#production_time

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
--
devel mailing list
devel@lists.fedoraproject.org
https://lists.fedoraproject.org/admin/lists/devel@lists.fedoraproject.org

Reply via email to