weixin_39784774
weixin_39784774
2020-12-02 18:42

EZP-15983 - Only include ezp_override when present

When generating autoloads on a fresh ezpublish-legacy install through Symfony, the missing file raises a notice which is caught by the Symfony exception handler - This means that the autoloads can't be generated through Symfony, unless they have previously been generated by legacy.

This change is trivial and mitigates the issue.

Duplicate/response to https://jira.ez.no/browse/EZP-15983 - Please don't hate me for using a file_exists !

该提问来源于开源项目:ezsystems/ezpublish-legacy

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

16条回答

  • weixin_39784774 weixin_39784774 4月前

    Running directly works, but is unreliable as it doesn't run through the Symfony console. Meaning it doesn't pick up Symfony includes (atm we run in prod mode to avoid the exception, but that is super hacky!)

    Generating an empty file doesn't work, as the autoload generator immediately attempts to use the included file as an array!

    点赞 评论 复制链接分享
  • weixin_39693437 weixin_39693437 4月前

    Generating an empty file doesn't work, as the autoload generator immediately attempts to use the included file as an array!

    So generate a file with an empty array? :)

    点赞 评论 复制链接分享
  • weixin_39984952 weixin_39984952 4月前

    I'm not sure but including a file with an empty array is likely to be slower than a file_exists.

    I did a simple test: I think I'm wrong: an include of an empty array CAN to be faster than a file_exists - if the FS is slow. On my SSD drive the file_exists is significantly faster.

    点赞 评论 复制链接分享
  • weixin_39784774 weixin_39784774 4月前

    Creating an empty file feels like a band-aid over what feels like a real problem - Given the direction of the LegacyBridge to remove deprecation warnings this feels like an apt PR.

    That being said, I am very open to hearing other methods of permanently solving this issue. 🙂

    点赞 评论 复制链接分享
  • weixin_39984952 weixin_39984952 4月前

    The code in question is wrapped in:

    if ( defined( 'EZP_AUTOLOAD_ALLOW_KERNEL_OVERRIDE' ) and EZP_AUTOLOAD_ALLOW_KERNEL_OVERRIDE )

    Why don't you set EZP_AUTOLOAD_ALLOW_KERNEL_OVERRIDE to false?

    点赞 评论 复制链接分享
  • weixin_39784774 weixin_39784774 4月前

    In this particular case, I actually attempt to generate kernel override autoloads through ezpgenerateautoloads.php which in turn invokes this autoloader.

    点赞 评论 复制链接分享
  • weixin_39879674 weixin_39879674 4月前

    In our use case we have to use kernel overrides, so disabling them altogether isn't really an option for us. 's conditional is within the check that you mentioned so if they are disabled then this check is obviously never performed.

    Also looking at the autoload.php file don't we already have a similar check on the ezp_extension.php file? To make sure that it exists before loading it. Presumably to correct the same problem for the extension autoload file?

    点赞 评论 复制链接分享
  • weixin_39693437 weixin_39693437 4月前

    All this talk and I still haven't been able to reproduce the issue with ezpublish:legacy:script :) No warnings whatsoever in logs.

    点赞 评论 复制链接分享
  • weixin_39784774 weixin_39784774 4月前

    I'm able to reliably reproduce on PHP 5.6 by using a fresh installation of eZ Publish Platform which through composer populates the ezpublish_legacy directory. var/autoload/ezp_override.php is not distributed with eZ Publish and so does not exist when this command is run (I've hooked it into the post autoload dump script run by composer, however this makes little difference as I can reproduce without), which is why an E_WARNING is raised.

    Applying this fix manually by running a patch, then running this, solves the problem consistently.

    点赞 评论 复制链接分享
  • weixin_39793813 weixin_39793813 4月前

    I have reproduced that couple of times too.

    点赞 评论 复制链接分享
  • weixin_39984952 weixin_39984952 4月前

    Performance doesn't seem to be a problem and given that the code already contains a few file_exists:

    +1

    点赞 评论 复制链接分享
  • weixin_39784774 weixin_39784774 4月前

    Bump? 😭

    点赞 评论 复制链接分享
  • weixin_39693437 weixin_39693437 4月前

    When generating autoloads on a fresh ezpublish-legacy install through Symfony

    I think I never ran into this issue so I'm curious about what exactly do you mean by "through Symfony" :)

    点赞 评论 复制链接分享
  • weixin_39784774 weixin_39784774 4月前

    Hey, I'm talking about an install through php app/console ezpublish:legacy:script bin/php/ezpgenerateautoloads.php from a fresh install (I.E. after dumping autoloads in composer)

    点赞 评论 复制链接分享
  • weixin_39693437 weixin_39693437 4月前

    Hm.. Okay! I still don't remember experiencing the issue :)

    How about generating an empty file rather than using a file_exists ?

    点赞 评论 复制链接分享
  • weixin_39953845 weixin_39953845 4月前

    Cant you run ezpgenerateautoloads without going thru the sf console? That's how I generally do it...

    点赞 评论 复制链接分享