Skip to content
Snippets Groups Projects
New look: Off

PHP 8.1

    • Closed Issue created by JoeMurray @JoeMurray

    We should support PHP 8.1.

    The deprecations and breaking changes are listed in the release notes at https://www.php.net/releases/8.1/en.php#deprecations_and_bc_breaks

    Here are the backward compatibility issues:

    1. Tentative return types in PHP built-in class methods
    2. HTML entity en/decode functions process single quotes and substitute by default
    3. $GLOBALS variable restrictions.
    4. MySQLi: Default error mode set to exceptions

    I am most worried about HTML encode/decode.

    [ ] create edge test setup for PHP 8.1

    0 of 4 checklist items completed
    To upload designs, you'll need to enable LFS and have an admin enable hashed storage. More information

    Child items ...

    • Activity

    • JoeMurray added Concept approved git:civicrm-core improvement needs:planning needs:testing sig:compatibility sig:maintainability labels

      added Concept approved git:civicrm-core improvement needs:planning needs:testing sig:compatibility sig:maintainability labels

    • JoeMurray mentioned in issue drupal#176 (closed)

      mentioned in issue drupal#176 (closed)

    • seamuslee
      seamuslee @seamuslee ·
      Owner

      So one issue I am hitting at the moment is that this is generating bunch of deprecation warnings when trying to run unit tests

      Deprecated: Symfony\Component\Config\Resource\ReflectionClassResource implements the Serializable interface, which is deprecated. Implement __serialize() and __unserialize() instead (or in addition, if support for old PHP versions is necessary) in /home/jenkins/bknix-edge/build/build-1/web/sites/all/modules/civicrm/vendor/symfony/config/Resource/ReflectionClassResource.php on line 20

      @totten @DaveD I am wondering if we need to bite the bullet so to speak and update Symfony sooner rather than later as 3.x Symfony won't help here unless there is a way of patching to make this work but that makes me feel concerned as well.

    • DaveD
      DaveD @DaveD ·
      Maintainer

      I don't personally have any objections to that. For what it's worth I tried looking at universe for symfony usage, but wasn't able to get a good handle on it, and that's maybe not a useful way to approach it anyway.

      • seamuslee
        seamuslee @seamuslee ·
        Owner

        So one thing I have also hit with 8.1 is that we might have some issues with the restriction on modifying Globals https://www.php.net/manual/en/migration81.incompatible.php#migration81.incompatible.core.globals-access this code seems most problematic https://github.com/civicrm/civicrm-core/blob/5db0bc3c1f54eaca4307f103a73bda596ae914d6/Civi/Core/Resolver.php#L280

      • totten
        totten @totten ·
        Owner

        I'm not sure if that piece of code is actually problematic -- the underlying implementation of pathGet() and pathSet() should all be fairly direct access (eg $GLOBALS[$part]) rather than batchy/loopy access (eg array_merge($GLOBALS...)). But I may not have a proper test environment.

        If you have a way to produce a problem, then try this fix: https://github.com/civicrm/civicrm-core/pull/23947

      • totten
        totten @totten ·
        Owner

        For example, I tried this:

        $ cv ev 'return 123;'
123

$ cv ev '$getter = Civi\Core\Resolver::singleton()->get("global://civicrm_root?getter"); return $getter();'
"/Users/me/bknix/build/dmaster/web/sites/all/modules/civicrm/"

        On my local php80+php81, both commands return the expected output. Of course, on php81, both commands produce a ton of unrelated warnings - but I don't think any of them relate to GLOBALS.

      • DaveD
        DaveD @DaveD ·
        Maintainer

        For what it's worth in my drupal 10 install it didn't come up, but I also don't know what path (pun intended) to take to reproduce.

      • totten
        totten @totten ·
        Owner

        Yeah, the mechanism is pretty rare. The only thing I can figure (from reading/speculating) is to send a mail-blast with a language-targeted mailing.

        CRM/Mailing/BAO/MailingJob.php:      $swapLang = CRM_Utils_AutoClean::swap('global://dbLocale?getter', 'call://i18n/setLocale', $mailing->language);

        FWIW, Mailing-BAO and Flexmailer both do this, but it's just slightly different:

        ext/flexmailer/src/FlexMailer.php:  $swapLang = \CRM_Utils_AutoClean::swap('call://i18n/getLocale', 'call://i18n/setLocale', $mailing->language);

        The Flexmailer snippet looks more logical, but the differences (tsLocale vs dbLocale) are fairly edgy, and I can't honestly say which is better. Arguably, it becomes moot whenever we make Flexmailer mandatory...

      • Please register or sign in to reply
    • totten mentioned in commit 6b4be82d

      mentioned in commit 6b4be82d

    • totten mentioned in commit 4ade7791

      mentioned in commit 4ade7791

    • yodatak
      yodatak @yodatak ·

      Hi thanks for you awsome work some error i got on a PHP 8.1 on wordpress this page don't show too https://civicrm/wp-admin/admin.php?page=CiviCRM&q=civicrm%2Fa%2F#/status

      Deprecated: str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in /var/www/html/civicrm/wp-content/plugins/civicrm/civicrm/CRM/Utils/System/WordPress.php on line 296

Deprecated: trim(): Passing null to parameter #1 ($string) of type string is deprecated in /var/www/html/civicrm/wp-content/plugins/civicrm/civicrm/CRM/Report/Page/InstanceList.php on line 164

Deprecated: str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in /var/www/html/civicrm/wp-content/plugins/civicrm/civicrm/CRM/Utils/System/WordPress.php on line 296

Deprecated: str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in /var/www/html/civicrm/wp-content/plugins/civicrm/civicrm/CRM/Utils/System/WordPress.php on line 296

Deprecated: strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated in /var/www/html/civicrm/wp-content/plugins/civicrm/civicrm.php on line 1361

Deprecated: strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated in /var/www/html/civicrm/wp-content/plugins/civicrm/civicrm.php on line 1362

Deprecated: str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in /var/www/html/civicrm/wp-content/plugins/civicrm/civicrm/CRM/Utils/System/WordPress.php on line 296
      Edited by yodatak
    • yodatak
      yodatak @yodatak ·

      and some Warning: Undefined array key "group_by" in /var/www/html/civicrm/wp-content/uploads/civicrm/templates_c/fr_FR/%%F5/F54/F5478576%%Table.tpl.php on line 190

    • totten mentioned in commit wordpress@f28a6cb8

      mentioned in commit wordpress@f28a6cb8

    • totten mentioned in commit wordpress@4956cc5c

      mentioned in commit wordpress@4956cc5c

    • JoeMurray assigned to @seamuslee

      assigned to @seamuslee

    • JoeMurray
      JoeMurray @JoeMurray ·
      Author Developer

      I note that this is a D10 support dependency so deserves attention now that D10 is out.

    • yodatak
      yodatak @yodatak ·

      I cannot show reportings Warning: Undefined array key "group_by" in /var/www/html/XXXX/wp-content/uploads/civicrm/templates_c/fr_FR/%%F5/F54/F5478576%%Table.tpl.php on line 190

      Warning: Undefined array key "group_by" in /var/www/html/XXXX/wp-content/uploads/civicrm/templates_c/fr_FR/%%F5/F54/F5478576%%Table.tpl.php on line 190

      Warning: Undefined array key "group_by" in /var/www/html/XXXX/wp-content/uploads/civicrm/templates_c/fr_FR/%%F5/F54/F5478576%%Table.tpl.php on line 193 octobre 28th, 2011

    • RoWo-DS
      RoWo-DS @RoWo-DS ·

      The end of life date for Drupal 9 is in November 2023.

      The last comment here is six months ago. Do you need testing to proceed here?

    • JoeMurray
      JoeMurray @JoeMurray ·
      Author Developer

      I see that 'PHP 8.1 is supported from 5.55' : https://docs.civicrm.org/installation/en/latest/general/requirements/

      Should we mark all four tasks above as complete??

    • DaveD
      DaveD @DaveD ·
      Maintainer

      In terms of the scope of this ticket, I'd say it's complete in that you can install and run civi on php 8.1. What is still a problem is the enormous number of warnings/deprecations that flood logs and make writing new unit tests difficult, and that someday will be fatal errors. They mostly need to be handled individually so it suggests making new tickets for each one, and they're likely to get missed if all lumped into one ticket. It's just there's sooooo manyyyyy so it will be a lot of tickets and prone to duplicates. It's the kind of thing a sprint would be good for.

    • JoeMurray
      JoeMurray @JoeMurray ·
      Author Developer

      Nice doc on issues and approaches, @DaveD .

      So this issue didn't manage to marshal the focussed effort to deal with the bugs all at once, deliberately. :disappointed:

      Closing and letting people create the individual ones.

    • DaveD closed

      closed

    Please register or sign in to reply