Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-2388

Zend Optimizer Plus/opcache have overlap in config settings, annotations reader doesn't identify these properly

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.3
    • Fix Version/s: 2.4
    • Component/s: Mapping Drivers
    • Labels:
      None

      Description

      Doctrine\Common\Annotations\AnnotationReader.php

      A change was recently authored in the AnnotationReader constructor that checked for a properly configured opcode cache (namely saving comments so that Doctrine can parse annotations).

      The current open source version of Zend Optimizer+ hosted at GitHub (https://github.com/zend-dev/ZendOptimizerPlus) when loaded registers itself as a module with the name "Zend Optimizer+" but takes configuration directives in the form of "opcache.<parameter>".

      The change to the constructor requires that if the module name is Zend Optimizer+, then the configuration directives must be prefixed with "zend_optimizerplus.<parameter>". This is clearly incorrect.

      The most recent update now complains of an AnnotationException as a consequence of this change when running Symfony2 (or Doctrine) with the latest opcache because configuration directives aren't detected properly.

      I am happy to fix this but could not find a way to report an issue to initiate the process.

      I would suggest something like this in the constructor:

      if ((extension_loaded('Zend Optimizer+') || extension_loaded('opcache')) && !(ini_get('opcache.save_comments') == 0 XOR ini_get('zend_optimizerplus.save_comments') == 0)) {

      What this means is that if either extension is loaded, one of those directives must be true and configured. This is a compromise. The alternative would be to keep the original source and do the following:

      if (extension_loaded('Zend Optimizer+') && ini_get('zend_optimizerplus.save_comments') == 0) {
      if (ini_get('opcache.save_comments') == 0)

      { throw AnnotationException::optimizerPlusSaveComments(); }

      }

      if (extension_loaded('opcache') && ini_get('opcache.save_comments') == 0)

      { throw AnnotationException::optimizerPlusSaveComments(); }

      I have no idea about the "opcache" module and what it actually equates to or whether it has alternative formats for its configuration directives. I feel like the second suggestion is even smellier than my original suggestion despite probably handling the edge case better.

      If you want a pull request, I will send one, just let me know how you want to handle it.

      If my PHP is horrible I apologise.

        Activity

        Hide
        Marco Pivetta added a comment -

        Ross Cousens is this already enstabilished? It's not worth changing it until the ZO+ stuff is stable.

        Show
        Marco Pivetta added a comment - Ross Cousens is this already enstabilished? It's not worth changing it until the ZO+ stuff is stable.
        Hide
        Ross Cousens added a comment - - edited

        Marco Pivetta Benjamin Eberlei

        Seems stable to me.

        I guess whether this is a priority or not depends on your subjective weighting of its importance and whether or not you use ZO+. I've ditched APC, and moved 20+ web servers over to Apache+ZO+. I currently deploy my own version of DBAL commons because it is not possible to work around it.

        https://github.com/zend-dev/ZendOptimizerPlus

        Please see this reference, why would they suddenly change the name of all the configuration directives especially in light of the fact that this is being integrated into PHP 5.5 as the standard generic "opcache" solution? I think this is where the confusion comes from. It might be branded as ZO+ on Github, but that becomes irrelevant when it's part of core.

        I've tried contacting Beberlei directly (the author of the change), on Twitter and via e-mail, but no response. Not getting any response is quite annoying as it's been a month since this change was made and whenever I update an existing site or publish a new site I have to deal with this.

        I need to invest some time in finding out how I can maintain my own fork of OSS projects that pull changes in from the branched master but allow me to keep 1 or 2 files totally different. The other thing that drives me insane is the fact that the postgresql platform drivers can't implement GUID generators because it requires an extension be enabled on the server (seriously, solve with documentation, it's easy to detect programmatically as well) plus the choosing of a function, when there is obviously only one sane choice.

        I am so grateful for the opportunity to develop with OSS, but the bureaucratic decision-by-committee stuff is so frustrating when you're an end-user and can't find anyone who gives half a crap about stuff that is just plain stupid but overlooked due to resources already spread thin or whatever may be the case. I would be happy to develop fixes as well, but when they languish because you can't find a maintainer that is familiar with the issue, interested in finding a fix, and has the time available to act as custodian of a pull request, it is incredibly demoralizing.

        Show
        Ross Cousens added a comment - - edited Marco Pivetta Benjamin Eberlei Seems stable to me. I guess whether this is a priority or not depends on your subjective weighting of its importance and whether or not you use ZO+. I've ditched APC, and moved 20+ web servers over to Apache+ZO+. I currently deploy my own version of DBAL commons because it is not possible to work around it. https://github.com/zend-dev/ZendOptimizerPlus Please see this reference, why would they suddenly change the name of all the configuration directives especially in light of the fact that this is being integrated into PHP 5.5 as the standard generic "opcache" solution? I think this is where the confusion comes from. It might be branded as ZO+ on Github, but that becomes irrelevant when it's part of core. I've tried contacting Beberlei directly (the author of the change), on Twitter and via e-mail, but no response. Not getting any response is quite annoying as it's been a month since this change was made and whenever I update an existing site or publish a new site I have to deal with this. I need to invest some time in finding out how I can maintain my own fork of OSS projects that pull changes in from the branched master but allow me to keep 1 or 2 files totally different. The other thing that drives me insane is the fact that the postgresql platform drivers can't implement GUID generators because it requires an extension be enabled on the server (seriously, solve with documentation, it's easy to detect programmatically as well) plus the choosing of a function, when there is obviously only one sane choice. I am so grateful for the opportunity to develop with OSS, but the bureaucratic decision-by-committee stuff is so frustrating when you're an end-user and can't find anyone who gives half a crap about stuff that is just plain stupid but overlooked due to resources already spread thin or whatever may be the case. I would be happy to develop fixes as well, but when they languish because you can't find a maintainer that is familiar with the issue, interested in finding a fix, and has the time available to act as custodian of a pull request, it is incredibly demoralizing.
        Hide
        Benjamin Eberlei added a comment -

        Fixed and released a Collections 1.1.1 version

        Show
        Benjamin Eberlei added a comment - Fixed and released a Collections 1.1.1 version
        Hide
        Ross Cousens added a comment -

        Thanks Benjamin, I am grateful for the fix! Happy coding!

        Show
        Ross Cousens added a comment - Thanks Benjamin, I am grateful for the fix! Happy coding!

          People

          • Assignee:
            Benjamin Eberlei
            Reporter:
            Ross Cousens
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: